From 0c39ec02fa0b5f649e872dead92db46ab07d9248 Mon Sep 17 00:00:00 2001 From: John McCall Date: Thu, 13 Aug 2015 21:49:14 +0000 Subject: [PATCH] Don't use materializeForSet when accessing properties or subscripts defined in protocol extensions. The right condition for this is really direct uses of the default implementation; right now, that just means all direct uses of something from a protocol extension. Fixes rdar://22109071. Swift SVN r31228 --- lib/SILGen/SILGenApply.cpp | 5 +++++ lib/SILGen/SILGenLValue.cpp | 9 +++++--- test/SILGen/materializeForSet.swift | 34 +++++++++++++++++++++++++++++ 3 files changed, 45 insertions(+), 3 deletions(-) diff --git a/lib/SILGen/SILGenApply.cpp b/lib/SILGen/SILGenApply.cpp index 1539e91be64a5..49f25852944b2 100644 --- a/lib/SILGen/SILGenApply.cpp +++ b/lib/SILGen/SILGenApply.cpp @@ -3821,6 +3821,11 @@ emitMaterializeForSetAccessor(SILLocation loc, SILDeclRef materializeForSet, // Scope any further writeback just within this operation. WritebackScope writebackScope(*this); + assert(!materializeForSet.getDecl() + ->getDeclContext()->isProtocolExtensionContext() && + "direct use of materializeForSet from a protocol extension is" + " probably a miscompile"); + Callee callee = emitSpecializedAccessorFunctionRef(*this, loc, materializeForSet, substitutions, selfValue, diff --git a/lib/SILGen/SILGenLValue.cpp b/lib/SILGen/SILGenLValue.cpp index 516b9fe191588..09b37c1c5f035 100644 --- a/lib/SILGen/SILGenLValue.cpp +++ b/lib/SILGen/SILGenLValue.cpp @@ -809,11 +809,14 @@ namespace { ManagedValue base, AccessKind accessKind) && override { // If this is just for a read, or the property is dynamic, or if - // it doesn't have a materializeForSet, just materialize to a - // temporary. + // it doesn't have a materializeForSet, or if this is a direct + // use of something defined in a protocol extension (see + // maybeEmitMaterializeForSetThunk), just materialize to a + // temporary directly. if (accessKind == AccessKind::Read || decl->getAttrs().hasAttribute() || - !decl->getMaterializeForSetFunc()) { + !decl->getMaterializeForSetFunc() || + decl->getDeclContext()->isProtocolExtensionContext()) { return std::move(*this).LogicalPathComponent::getMaterialized(gen, loc, base, accessKind); } diff --git a/test/SILGen/materializeForSet.swift b/test/SILGen/materializeForSet.swift index 6a0c65f06e417..f3718e83f9ef6 100644 --- a/test/SILGen/materializeForSet.swift +++ b/test/SILGen/materializeForSet.swift @@ -225,6 +225,40 @@ class HasWeak { // CHECK: return [[T4]] : $(Builtin.RawPointer, Optional<@convention(thin) (Builtin.RawPointer, inout Builtin.UnsafeValueBuffer, inout HasWeak, @thick HasWeak.Type) -> ()>) // CHECK: } +// rdar://22109071 +// Test that we don't use materializeForSet from a protocol extension. +protocol Magic {} +extension Magic { + var hocus: Int { + get { return 0 } + set {} + } +} +struct Wizard : Magic {} +func improve(inout x: Int) {} +func improveWizard(inout wizard: Wizard) { + improve(&wizard.hocus) +} +// SILGEN-LABEL: sil hidden @_TF17materializeForSet13improveWizardFRVS_6WizardT_ +// SILGEN: [[IMPROVE:%.*]] = function_ref @_TF17materializeForSet7improveFRSiT_ : +// SILGEN-NEXT: [[TEMP:%.*]] = alloc_stack $Int +// Call the getter and materialize the result in the temporary. +// SILGEN-NEXT: [[T0:%.*]] = load [[WIZARD:.*]] : $*Wizard +// SILGEN-NEXT: function_ref +// SILGEN-NEXT: [[GETTER:%.*]] = function_ref @_TFeRq_17materializeForSet5Magic_S_S0_g5hocusSi +// SILGEN-NEXT: [[WTEMP:%.*]] = alloc_stack $Wizard +// SILGEN-NEXT: store [[T0]] to [[WTEMP]]#1 +// SILGEN-NEXT: [[T0:%.*]] = apply [[GETTER]]([[WTEMP]]#1) +// SILGEN-NEXT: store [[T0]] to [[TEMP]]#1 +// Call improve. +// SILGEN-NEXT: apply [[IMPROVE]]([[TEMP]]#1) +// SILGEN-NEXT: [[T0:%.*]] = load [[TEMP]]#1 +// SILGEN-NEXT: function_ref +// SILGEN-NEXT: [[SETTER:%.*]] = function_ref @_TFeRq_17materializeForSet5Magic_S_S0_s5hocusSi +// SILGEN-NEXT: apply [[SETTER]]([[T0]], [[WIZARD]]) +// SILGEN-NEXT: dealloc_stack [[WTEMP]]#0 +// SILGEN-NEXT: dealloc_stack [[TEMP]]#0 + protocol Totalled { var total: Int { get set } }