Merge pull request #11744 from gspiers/bugfix/SR-479-ownership-in-protocols

[SE implementation] Ownership keyword removal in protocols

Author: slavapestov

include/swift/AST/DiagnosticsSema.def

@@ -3120,6 +3120,13 @@ ERROR(invalid_weak_ownership_not_optional,none,
       "'weak' variable should have optional type %0", (Type))
 ERROR(invalid_weak_let,none,
       "'weak' must be a mutable variable, because it may change at runtime", ())
+ERROR(ownership_invalid_in_protocols,none,
+      "'%select{strong|weak|unowned|unowned}0' cannot be applied to a property declaration in a protocol",
+      (/*Ownership*/unsigned))
+WARNING(ownership_invalid_in_protocols_compat_warning,none,
+      "'%select{strong|weak|unowned|unowned}0' should not be applied to a property declaration "
+      "in a protocol and will be disallowed in future versions",
+      (/*Ownership*/unsigned))
 
 // required
 ERROR(required_initializer_nonclass,none,

lib/Sema/TypeCheckAttr.cpp

@@ -2006,6 +2006,10 @@ void TypeChecker::checkTypeModifyingDeclAttributes(VarDecl *var) {
 }
 
 void TypeChecker::checkOwnershipAttr(VarDecl *var, OwnershipAttr *attr) {
+  // Don't check ownership attribute if the declaration is already marked invalid.
+  if (var->isInvalid())
+    return;
+
   Type type = var->getType();
   Type interfaceType = var->getInterfaceType();
 
@@ -2053,6 +2057,20 @@ void TypeChecker::checkOwnershipAttr(VarDecl *var, OwnershipAttr *attr) {
 
     diagnose(var->getStartLoc(), D, (unsigned) ownershipKind, underlyingType);
     attr->setInvalid();
+  } else if (dyn_cast<ProtocolDecl>(var->getDeclContext())) {
+    // Ownership does not make sense in protocols.
+    if (Context.isSwiftVersionAtLeast(5))
+      diagnose(attr->getLocation(),
+        diag::ownership_invalid_in_protocols,
+        (unsigned) ownershipKind)
+        .fixItRemove(attr->getRange());
+    else
+      diagnose(attr->getLocation(),
+        diag::ownership_invalid_in_protocols_compat_warning,
+        (unsigned) ownershipKind)
+        .fixItRemove(attr->getRange());
+
+    attr->setInvalid();
   }
 
   if (attr->isInvalid())

test/Compatibility/ownership_protocol.swift

@@ -0,0 +1,12 @@
+// RUN: %target-typecheck-verify-swift -swift-version 4
+// Generate a swift 4 compatible warning if ownership is specified in a protocol
+
+class SomeClass {}
+
+protocol P {
+  weak var foo: SomeClass? { get set } // expected-warning {{'weak' should not be applied to a property declaration in a protocol and will be disallowed in future versions}}
+  unowned var foo2: SomeClass { get set } // expected-warning {{'unowned' should not be applied to a property declaration in a protocol and will be disallowed in future versions}}
+  weak var foo3: Int? { get set } // expected-error {{'weak' may only be applied to class and class-bound protocol types, not 'Int'}}
+  unowned var foo4: Int { get set } // expected-error {{'unowned' may only be applied to class and class-bound protocol types, not 'Int'}}
+}
+

test/SILGen/Inputs/weak_other.swift

@@ -11,7 +11,7 @@ extension Router {
 }
 
 public protocol Environment : class {
-  unowned var router: Router { get }
+  var router: Router { get }
 }
 
 open class UI {

test/SILGen/properties.swift

@@ -850,7 +850,7 @@ struct ObservingPropertiesWithOwnershipTypesInferred {
 
 // <rdar://problem/16554876> property accessor synthesization of weak variables doesn't work
 protocol WeakPropertyProtocol {
- weak var maybePresent : Ref? { get set }
+ var maybePresent : Ref? { get set }
 }
 
 struct WeakPropertyStruct : WeakPropertyProtocol {

test/decl/protocol/ownership_protocol.swift

@@ -0,0 +1,29 @@
+// RUN: %target-typecheck-verify-swift -swift-version 5
+
+class SomeClass {}
+
+protocol P {
+  weak var foo: SomeClass? { get set } // expected-error {{'weak' cannot be applied to a property declaration in a protocol}}
+  unowned var foo2: SomeClass { get set } // expected-error {{'unowned' cannot be applied to a property declaration in a protocol}}
+  weak var foo3: Int? { get set } // expected-error {{'weak' may only be applied to class and class-bound protocol types, not 'Int'}}
+  unowned var foo4: Int { get set } // expected-error {{'unowned' may only be applied to class and class-bound protocol types, not 'Int'}}
+  var foo9: SomeClass? { get }
+}
+
+extension P {
+  weak var foo5: SomeClass? // expected-error {{extensions must not contain stored properties}}
+  unowned var foo6: SomeClass // expected-error {{extensions must not contain stored properties}}
+  
+  weak var foo7: SomeClass? { // Okay
+    return SomeClass()
+  }
+
+  unowned var foo8: SomeClass { // Okay
+    return SomeClass()
+  }
+
+  weak var foo9: SomeClass? { // Okay
+    return nil
+  }
+}
+

test/stdlib/WeakMirror.swift

@@ -103,7 +103,7 @@ mirrors.test("struct/StructHasNativeWeakReference") {
 import Foundation
 
 @objc protocol ObjCClassExistential : class {
-  weak var weakProperty: AnyObject? { get set }
+  var weakProperty: AnyObject? { get set }
   var x: Int { get }
 }