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
dxc0829 · Aug 13, 2015 · 0c39ec0 · 0c39ec0
1 parent f493ab2
commit 0c39ec0
Show file tree

Hide file tree

Showing 3 changed files with 45 additions and 3 deletions.
diff --git 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
@@ -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<DynamicAttr>() ||
-          !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
@@ -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]]<Wizard>([[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]]<Wizard>([[T0]], [[WIZARD]])
+// SILGEN-NEXT:  dealloc_stack [[WTEMP]]#0
+// SILGEN-NEXT:  dealloc_stack [[TEMP]]#0
+
 protocol Totalled {
   var total: Int { get set }
 }