Merge pull request swiftlang#27057 from theblixguy/unrevert/SR-11298

Unrevert "[Sema] Setter has incorrect mutating-ness inside class-constrained protocol extension"

Author: theblixguy

CHANGELOG.md

@@ -26,6 +26,63 @@ CHANGELOG
 Swift Next
 ----------
 
+* [SR-11298][]:
+
+  A class-constrained protocol extension, where the extended protocol does
+  not impose a class constraint, will now infer the constraint implicitly.
+
+  ```swift
+  protocol Foo {}
+  class Bar: Foo {
+    var someProperty: Int = 0
+  }
+
+  // Even though 'Foo' does not impose a class constraint, it is automatically
+  // inferred due to the Self: Bar constraint.
+  extension Foo where Self: Bar {
+    var anotherProperty: Int {
+      get { return someProperty }
+      // As a result, the setter is now implicitly nonmutating, just like it would
+      // be if 'Foo' had a class constraint.
+      set { someProperty = newValue }
+    }
+  }
+  ```
+  
+  As a result, this could lead to code that currently compiles today to throw an error.
+  
+  ```swift
+  protocol Foo {
+    var someProperty: Int { get set }
+  }
+  
+  class Bar: Foo {
+    var someProperty = 0
+  }
+  
+  extension Foo where Self: Bar {
+    var anotherProperty1: Int {
+      get { return someProperty }
+      // This will now error, because the protocol requirement
+      // is implicitly mutating and the setter is implicitly 
+      // nonmutating.
+      set { someProperty = newValue } // Error
+    }
+  }
+  ```
+
+  **Workaround**: Define a new mutable variable inside the setter that has a reference to `self`:
+  
+  ```swift
+  var anotherProperty1: Int {
+    get { return someProperty }
+    set {
+      var mutableSelf = self
+      mutableSelf.someProperty = newValue // Okay
+    }
+  }
+```
+
 * [SE-0253][]:
 
   Values of types that declare `func callAsFunction` methods can be called
@@ -51,7 +108,7 @@ Swift Next
 
 * [SR-4206][]:
 
-  A method override is no longer allowed to have a generic signature with 
+  A method override is no longer allowed to have a generic signature with
   requirements not imposed by the base method. For example:
 
   ```
@@ -7765,3 +7822,4 @@ Swift 1.0
 [SR-8974]: <https://bugs.swift.org/browse/SR-8974>
 [SR-9043]: <https://bugs.swift.org/browse/SR-9043>
 [SR-9827]: <https://bugs.swift.org/browse/SR-9827>
+[SR-11298]: <https://bugs.swift.org/browse/SR-11298>

include/swift/AST/DeclContext.h

@@ -261,7 +261,10 @@ class alignas(1 << DeclContextAlignInBits) DeclContext {
 
   /// Returns the kind of context this is.
   DeclContextKind getContextKind() const;
-  
+
+  /// Returns whether this context has value semantics.
+  bool hasValueSemantics() const;
+
   /// Determines whether this context is itself a local scope in a
   /// code block.  A context that appears in such a scope, like a
   /// local type declaration, does not itself become a local context.

lib/AST/Decl.cpp

@@ -5634,12 +5634,18 @@ void VarDecl::emitLetToVarNoteIfSimple(DeclContext *UseDC) const {
     if (FD && !FD->isMutating() && !FD->isImplicit() && FD->isInstanceMember()&&
         !FD->getDeclContext()->getDeclaredInterfaceType()
                  ->hasReferenceSemantics()) {
-      // Do not suggest the fix it in implicit getters
+      // Do not suggest the fix-it in implicit getters
       if (auto AD = dyn_cast<AccessorDecl>(FD)) {
         if (AD->isGetter() && !AD->getAccessorKeywordLoc().isValid())
           return;
+
+        auto accessorDC = AD->getDeclContext();
+        // Do not suggest the fix-it if `Self` is a class type.
+        if (accessorDC->isTypeContext() && !accessorDC->hasValueSemantics()) {
+          return;
+        }
       }
-                   
+
       auto &d = getASTContext().Diags;
       d.diagnose(FD->getFuncLoc(), diag::change_to_mutating,
                  isa<AccessorDecl>(FD))

lib/AST/DeclContext.cpp

@@ -1018,6 +1018,14 @@ DeclContextKind DeclContext::getContextKind() const {
   llvm_unreachable("Unhandled DeclContext ASTHierarchy");
 }
 
+bool DeclContext::hasValueSemantics() const {
+  if (auto contextTy = getSelfTypeInContext()) {
+    return !contextTy->hasReferenceSemantics();
+  }
+
+  return false;
+}
+
 SourceLoc swift::extractNearestSourceLoc(const DeclContext *dc) {
   switch (dc->getContextKind()) {
   case DeclContextKind::AbstractFunctionDecl:

lib/Sema/TypeCheckDecl.cpp

@@ -1953,17 +1953,10 @@ void TypeChecker::validateDecl(OperatorDecl *OD) {
   }
 }
 
-bool swift::doesContextHaveValueSemantics(DeclContext *dc) {
-  if (Type contextTy = dc->getDeclaredInterfaceType())
-    return !contextTy->hasReferenceSemantics();
-  return false;
-}
-
 llvm::Expected<SelfAccessKind>
 SelfAccessKindRequest::evaluate(Evaluator &evaluator, FuncDecl *FD) const {
   if (FD->getAttrs().getAttribute<MutatingAttr>(true)) {
-    if (!FD->isInstanceMember() ||
-        !doesContextHaveValueSemantics(FD->getDeclContext())) {
+    if (!FD->isInstanceMember() || !FD->getDeclContext()->hasValueSemantics()) {
       return SelfAccessKind::NonMutating;
     }
     return SelfAccessKind::Mutating;
@@ -1985,8 +1978,7 @@ SelfAccessKindRequest::evaluate(Evaluator &evaluator, FuncDecl *FD) const {
     case AccessorKind::MutableAddress:
     case AccessorKind::Set:
     case AccessorKind::Modify:
-      if (AD->isInstanceMember() &&
-          doesContextHaveValueSemantics(AD->getDeclContext()))
+      if (AD->isInstanceMember() && AD->getDeclContext()->hasValueSemantics())
         return SelfAccessKind::Mutating;
       break;
 

lib/Sema/TypeCheckDecl.h

@@ -25,8 +25,6 @@ class DeclContext;
 class ValueDecl;
 class Pattern;
 
-bool doesContextHaveValueSemantics(DeclContext *dc);
-
 /// Walks up the override chain for \p CD until it finds an initializer that is
 /// required and non-implicit. If no such initializer exists, returns the
 /// declaration where \c required was introduced (i.e. closest to the root

lib/Sema/TypeCheckStorage.cpp

@@ -268,8 +268,9 @@ void swift::validatePatternBindingEntries(TypeChecker &tc,
 llvm::Expected<bool>
 IsGetterMutatingRequest::evaluate(Evaluator &evaluator,
                                   AbstractStorageDecl *storage) const {
-  bool result = (!storage->isStatic() &&
-                 doesContextHaveValueSemantics(storage->getDeclContext()));
+  auto storageDC = storage->getDeclContext();
+  bool result = (!storage->isStatic() && storageDC->isTypeContext() &&
+                 storageDC->hasValueSemantics());
 
   // 'lazy' overrides the normal accessor-based rules and heavily
   // restricts what accessors can be used.  The getter is considered
@@ -297,7 +298,7 @@ IsGetterMutatingRequest::evaluate(Evaluator &evaluator,
 
   // Protocol requirements are always written as '{ get }' or '{ get set }';
   // the @_borrowed attribute determines if getReadImpl() becomes Get or Read.
-  if (isa<ProtocolDecl>(storage->getDeclContext()))
+  if (isa<ProtocolDecl>(storageDC))
     return checkMutability(AccessorKind::Get);
 
   switch (storage->getReadImpl()) {
@@ -323,8 +324,9 @@ IsSetterMutatingRequest::evaluate(Evaluator &evaluator,
                                   AbstractStorageDecl *storage) const {
   // By default, the setter is mutating if we have an instance member of a
   // value type, but this can be overridden below.
-  bool result = (!storage->isStatic() &&
-                 doesContextHaveValueSemantics(storage->getDeclContext()));
+  auto storageDC = storage->getDeclContext();
+  bool result = (!storage->isStatic() && storageDC->isTypeContext() &&
+                 storageDC->hasValueSemantics());
 
   // If we have an attached property wrapper, the setter is mutating
   // or not based on the composition of the wrappers.

test/decl/ext/extensions.swift

@@ -126,7 +126,7 @@ struct WrapperContext {
 }
 
 // Class-constrained extension where protocol does not impose class requirement
-
+// SR-11298
 protocol DoesNotImposeClassReq_1 {}
 
 class JustAClass: DoesNotImposeClassReq_1 {
@@ -140,8 +140,8 @@ extension DoesNotImposeClassReq_1 where Self: JustAClass {
   }
 }
 
-let instanceOfJustAClass = JustAClass() // expected-note {{change 'let' to 'var' to make it mutable}}
-instanceOfJustAClass.wrappingProperty = "" // expected-error {{cannot assign to property: 'instanceOfJustAClass' is a 'let' constant}}
+let instanceOfJustAClass = JustAClass()
+instanceOfJustAClass.wrappingProperty = "" // Okay
 
 protocol DoesNotImposeClassReq_2 {
   var property: String { get set }
@@ -150,7 +150,7 @@ protocol DoesNotImposeClassReq_2 {
 extension DoesNotImposeClassReq_2 where Self : AnyObject {
   var wrappingProperty: String {
     get { property }
-    set { property = newValue } // Okay
+    set { property = newValue } // expected-error {{cannot assign to property: 'self' is immutable}}
   }
 }