[Typechecker] Reapply fix for SR-11298

CHANGELOG.md

@@ -58,6 +58,29 @@ Swift 5.2
   (foo as Magic)(5)
   ```
 
+* [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 }
+    }
+  }
+  ```
+
 * [SE-0253][]:
 
   Values of types that declare `func callAsFunction` methods can be called
@@ -83,7 +106,7 @@ Swift 5.2
 
 * [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:
 
   ```
@@ -7799,4 +7822,5 @@ 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>
 [SR-11429]: <https://bugs.swift.org/browse/SR-11429>

include/swift/AST/DeclContext.h

@@ -261,7 +261,13 @@ 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;
+
+  /// Returns whether this context is an extension constrained to a class type.
+  bool isClassConstrainedProtocolExtension() 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

@@ -5628,7 +5628,7 @@ 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;

lib/AST/DeclContext.cpp

@@ -1035,6 +1035,20 @@ DeclContextKind DeclContext::getContextKind() const {
   llvm_unreachable("Unhandled DeclContext ASTHierarchy");
 }
 
+bool DeclContext::hasValueSemantics() const {
+  if (getExtendedProtocolDecl())
+    return !isClassConstrainedProtocolExtension();
+  return !getDeclaredInterfaceType()->hasReferenceSemantics();
+}
+
+bool DeclContext::isClassConstrainedProtocolExtension() const {
+  if (getExtendedProtocolDecl()) {
+    auto ED = cast<ExtensionDecl>(this);
+    return ED->getGenericSignature()->requiresClass(ED->getSelfInterfaceType());
+  }
+  return false;
+}
+
 SourceLoc swift::extractNearestSourceLoc(const DeclContext *dc) {
   switch (dc->getContextKind()) {
   case DeclContextKind::AbstractFunctionDecl:

lib/Sema/TypeCheckDecl.cpp

@@ -2087,17 +2087,15 @@ OperatorPrecedenceGroupRequest::evaluate(Evaluator &evaluator,
   return group;
 }
 
-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()) {
+      // If this decl is on a class-constrained protocol extension, then
+      // respect the explicit mutatingness. Otherwise, we would throw an
+      // error.
+      if (FD->getDeclContext()->isClassConstrainedProtocolExtension())
+        return SelfAccessKind::Mutating;
       return SelfAccessKind::NonMutating;
     }
     return SelfAccessKind::Mutating;
@@ -2119,8 +2117,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

@@ -270,8 +270,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
@@ -299,7 +300,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()) {
@@ -325,8 +326,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,31 +126,202 @@ struct WrapperContext {
 }
 
 // Class-constrained extension where protocol does not impose class requirement
+// SR-11298
 
 protocol DoesNotImposeClassReq_1 {}
-	
+
 class JustAClass: DoesNotImposeClassReq_1 {
   var property: String = ""
 }
 
 extension DoesNotImposeClassReq_1 where Self: JustAClass {
-  var wrappingProperty: String {
+  var wrappingProperty1: String {
+    get { return property }
+    set { property = newValue } // Okay
+  }
+
+  var wrappingProperty2: String {
     get { return property }
-    set { property = newValue }
+    nonmutating set { property = newValue } // Okay
+  }
+
+  var wrappingProperty3: String {
+    get { return property }
+    mutating set { property = newValue } // Okay
+  }
+
+  mutating func foo() {
+    property = "" // Okay
+    wrappingProperty1 = "" // Okay
+    wrappingProperty2 = "" // Okay
+    wrappingProperty3 = "" // Okay
+  }
+
+  func bar() { // expected-note {{mark method 'mutating' to make 'self' mutable}}{{3-3=mutating }}
+    property = "" // Okay
+    wrappingProperty1 = "" // Okay
+    wrappingProperty2 = "" // Okay
+    wrappingProperty3 = "" // expected-error {{cannot assign to property: 'self' is immutable}}
+  }
+
+  nonmutating func baz() { // expected-note {{mark method 'mutating' to make 'self' mutable}}{{3-14=mutating}}
+    property = "" // Okay
+    wrappingProperty1 = "" // Okay
+    wrappingProperty2 = "" // Okay
+    wrappingProperty3 = "" // expected-error {{cannot assign to property: 'self' is immutable}}
   }
 }
-
-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 instanceOfJustAClass1 = JustAClass() // expected-note 2{{change 'let' to 'var' to make it mutable}}
+instanceOfJustAClass1.wrappingProperty1 = "" // Okay
+instanceOfJustAClass1.wrappingProperty2 = "" // Okay
+instanceOfJustAClass1.wrappingProperty3 = "" // expected-error {{cannot assign to property: 'instanceOfJustAClass1' is a 'let' constant}}
+instanceOfJustAClass1.foo() // expected-error {{cannot use mutating member on immutable value: 'instanceOfJustAClass1' is a 'let' constant}}
+instanceOfJustAClass1.bar() // Okay
+instanceOfJustAClass1.baz() // Okay
+
+var instanceOfJustAClass2 = JustAClass()
+instanceOfJustAClass2.foo() // Okay
 
 protocol DoesNotImposeClassReq_2 {
   var property: String { get set }
 }
 
 extension DoesNotImposeClassReq_2 where Self : AnyObject {
-  var wrappingProperty: String {
+  var wrappingProperty1: String {
     get { property }
-    set { property = newValue } // Okay
+    set { property = newValue } // expected-error {{cannot assign to property: 'self' is immutable}}
+    // expected-note@-1 {{mark accessor 'mutating' to make 'self' mutable}}{{5-5=mutating }}
+  }
+
+  var wrappingProperty2: String {
+    get { property }
+    nonmutating set { property = newValue } // expected-error {{cannot assign to property: 'self' is immutable}}
+    // expected-note@-1 {{mark accessor 'mutating' to make 'self' mutable}}{{5-16=mutating}}
+  }
+
+  var wrappingProperty3: String {
+    get { property }
+    mutating set { property = newValue } // Okay
+  }
+
+  mutating func foo() {
+    property = "" // Okay
+    wrappingProperty1 = "" // Okay (the error is on the setter declaration above)
+    wrappingProperty2 = "" // Okay (the error is on the setter declaration above)
+    wrappingProperty3 = "" // Okay
+  }
+
+  func bar() { // expected-note 2{{mark method 'mutating' to make 'self' mutable}}{{3-3=mutating }}
+    property = "" // expected-error {{cannot assign to property: 'self' is immutable}}
+    wrappingProperty1 = "" // Okay (the error is on the setter declaration above)
+    wrappingProperty2 = "" // Okay (the error is on the setter declaration above)
+    wrappingProperty3 = "" // expected-error {{cannot assign to property: 'self' is immutable}}
+  }
+
+  nonmutating func baz() { // expected-note 2{{mark method 'mutating' to make 'self' mutable}}{{3-14=mutating}}
+    property = "" // expected-error {{cannot assign to property: 'self' is immutable}}
+    wrappingProperty1 = "" // Okay (the error is on the setter declaration above)
+    wrappingProperty2 = "" // Okay (the error is on the setter declaration above)
+    wrappingProperty3 = "" // expected-error {{cannot assign to property: 'self' is immutable}}
+  }
+}
+
+protocol DoesNotImposeClassReq_3 {
+  var someProperty: Int { get set }
+}
+
+class JustAClass1: DoesNotImposeClassReq_3 {
+  var someProperty = 0
+}
+
+extension DoesNotImposeClassReq_3 where Self: JustAClass1 {
+  var anotherProperty1: Int {
+    get { return someProperty }
+    set { someProperty = newValue } // Okay
+  }
+
+  var anotherProperty2: Int {
+    get { return someProperty }
+    mutating set { someProperty = newValue } // Okay
+  }
+}
+
+let justAClass1 = JustAClass1() // expected-note {{change 'let' to 'var' to make it mutable}}
+justAClass1.anotherProperty1 = 1234 // Okay
+justAClass1.anotherProperty2 = 4321 // expected-error {{cannot assign to property: 'justAClass1' is a 'let' constant}}
+
+protocol ImposeClassReq1: AnyObject {
+  var someProperty: Int { get set }
+}
+
+class JustAClass2: ImposeClassReq1 {
+  var someProperty = 0
+}
+
+extension ImposeClassReq1 where Self: AnyObject {
+  var wrappingProperty1: Int {
+    get { return someProperty }
+    set { someProperty = newValue }
+  }
+
+  var wrappingProperty2: Int {
+    get { return someProperty }
+    mutating set { someProperty = newValue } // expected-error {{'mutating' isn't valid on methods in classes or class-bound protocols}}
+  }
+
+  mutating func foo() { // expected-error {{mutating' isn't valid on methods in classes or class-bound protocols}}
+    someProperty = 1
+  }
+
+  nonmutating func bar() { // expected-error {{'nonmutating' isn't valid on methods in classes or class-bound protocols}}
+    someProperty = 2
+  }
+
+  func baz() { // Okay
+    someProperty = 3
+  }
+}
+
+extension ImposeClassReq1 {
+  var wrappingProperty3: Int {
+    get { return someProperty }
+    set { someProperty = newValue }
+  }
+}
+
+let justAClass2 = JustAClass2() // expected-note {{change 'let' to 'var' to make it mutable}}
+justAClass2.wrappingProperty1 = 9876 // Okay
+justAClass2.wrappingProperty3 = 0987 // Okay
+justAClass2.foo() // expected-error {{cannot use mutating member on immutable value: 'justAClass2' is a 'let' constant}}
+justAClass2.bar() // Okay as well (complains about explicit nonmutating on decl)
+justAClass2.baz() // Okay
+
+protocol ImposeClassReq2: AnyObject {
+  var someProperty: Int { get set }
+}
+
+extension ImposeClassReq2 {
+  var wrappingProperty1: Int {
+    get { return someProperty }
+    set { someProperty = newValue }
+  }
+
+  var wrappingProperty2: Int {
+    get { return someProperty }
+    mutating set { someProperty = newValue } // expected-error {{'mutating' isn't valid on methods in classes or class-bound protocols}}
+  }
+
+  mutating func foo() { // expected-error {{mutating' isn't valid on methods in classes or class-bound protocols}}
+    someProperty = 1
+  }
+
+  nonmutating func bar() { // expected-error {{'nonmutating' isn't valid on methods in classes or class-bound protocols}}
+    someProperty = 2
+  }
+
+  func baz() { // Okay
+    someProperty = 3
   }
 }