Infer selectors from protocols for property accessors too. (#6634)

Most property accessors have selectors matching their protocols, but
not all. Don't force the user to write '@objc' explicitly on an
accessor, which isn't even possible for stored properties.

More groundwork for rdar://problem/28543037.

Author: jrose-apple

lib/Sema/TypeCheckProtocol.cpp

@@ -5357,10 +5357,18 @@ TypeChecker::findWitnessedObjCRequirements(const ValueDecl *witness,
   if (isa<TypeDecl>(witness)) return result;
 
   auto dc = witness->getDeclContext();
-  auto name = witness->getFullName();
   auto nominal = dc->getAsNominalTypeOrNominalTypeExtensionContext();
   if (!nominal) return result;
 
+  DeclName name = witness->getFullName();
+  auto accessorKind = AccessorKind::NotAccessor;
+  if (auto *fn = dyn_cast<FuncDecl>(witness)) {
+    accessorKind = fn->getAccessorKind();
+    if (accessorKind != AccessorKind::NotAccessor) {
+      name = fn->getAccessorStorageDecl()->getFullName();
+    }
+  }
+
   for (auto proto : nominal->getAllProtocols()) {
     // We only care about Objective-C protocols.
     if (!proto->isObjC()) continue;
@@ -5385,36 +5393,68 @@ TypeChecker::findWitnessedObjCRequirements(const ValueDecl *witness,
       }
       if (!*conformance) continue;
 
+      const Decl *found = (*conformance)->getWitness(req, this).getDecl();
+
+      if (!found) {
+        // If we have an optional requirement in an inherited conformance,
+        // check whether the potential witness matches the requirement.
+        // FIXME: for now, don't even try this with generics involved. We
+        // should be tracking how subclasses implement optional requirements,
+        // in which case the getWitness() check above would suffice.
+        if (!req->getAttrs().hasAttribute<OptionalAttr>() ||
+            !isa<InheritedProtocolConformance>(*conformance)) {
+          continue;
+        }
+
+        auto normal = (*conformance)->getRootNormalConformance();
+        auto dc = (*conformance)->getDeclContext();
+        if (dc->getGenericSignatureOfContext() ||
+            normal->getDeclContext()->getGenericSignatureOfContext()) {
+          continue;
+        }
+
+        const ValueDecl *witnessToMatch = witness;
+        if (accessorKind != AccessorKind::NotAccessor)
+          witnessToMatch = cast<FuncDecl>(witness)->getAccessorStorageDecl();
+
+        RequirementEnvironment reqEnvironment(*this, dc, req, *conformance);
+        if (matchWitness(*this, proto, *conformance,
+                         witnessToMatch->getDeclContext(), req,
+                         const_cast<ValueDecl *>(witnessToMatch),
+                         reqEnvironment).Kind == MatchKind::ExactMatch) {
+          if (accessorKind != AccessorKind::NotAccessor) {
+            auto *storageReq = dyn_cast<AbstractStorageDecl>(req);
+            if (!storageReq)
+              continue;
+            req = storageReq->getAccessorFunction(accessorKind);
+          }
+          result.push_back(req);
+          if (anySingleRequirement) return result;
+          continue;
+        }
+
+        continue;
+      }
+
+      // Dig out the appropriate accessor, if necessary.
+      if (accessorKind != AccessorKind::NotAccessor) {
+        auto *storageReq = dyn_cast<AbstractStorageDecl>(req);
+        auto *storageFound = dyn_cast_or_null<AbstractStorageDecl>(found);
+        if (!storageReq || !storageFound)
+          continue;
+        req = storageReq->getAccessorFunction(accessorKind);
+        if (!req)
+          continue;
+        found = storageFound->getAccessorFunction(accessorKind);
+      }
+
       // Determine whether the witness for this conformance is in fact
       // our witness.
-      if ((*conformance)->getWitness(req, this).getDecl() == witness) {
+      if (found == witness) {
         result.push_back(req);
         if (anySingleRequirement) return result;
         continue;
       }
-
-      // If we have an inherited conformance, check whether the potential
-      // witness matches the requirement.
-      // FIXME: for now, don't even try this with generics involved. We
-      // should be tracking how subclasses implement optional requirements,
-      // in which case the getWitness() check above would suffice.
-      if (req->getAttrs().hasAttribute<OptionalAttr>() &&
-          isa<InheritedProtocolConformance>(*conformance)) {
-        auto normal = (*conformance)->getRootNormalConformance();
-        if (!(*conformance)->getDeclContext()->getGenericSignatureOfContext() &&
-            !normal->getDeclContext()->getGenericSignatureOfContext()) {
-          auto dc = (*conformance)->getDeclContext();
-          RequirementEnvironment reqEnvironment(*this, dc, req, *conformance);
-          if (matchWitness(*this, proto, *conformance, witness->getDeclContext(),
-                           req, const_cast<ValueDecl *>(witness),
-                           reqEnvironment)
-                .Kind == MatchKind::ExactMatch) {
-            result.push_back(req);
-            if (anySingleRequirement) return result;
-            continue;
-          }
-        }
-      }
     }
   }
 

lib/Sema/TypeCheckType.cpp

@@ -3205,7 +3205,8 @@ bool TypeChecker::isRepresentableInObjC(
       // Global computed properties may however @_cdecl their accessors.
       auto storage = FD->getAccessorStorageDecl();
       validateDecl(storage);
-      if (!storage->isObjC() && Reason != ObjCReason::ExplicitlyCDecl) {
+      if (!storage->isObjC() && Reason != ObjCReason::ExplicitlyCDecl &&
+          Reason != ObjCReason::WitnessToObjC) {
         if (Diagnose) {
           auto error = FD->isGetter()
                     ? (isa<VarDecl>(storage) 
@@ -3220,25 +3221,27 @@ bool TypeChecker::isRepresentableInObjC(
         }
         return false;
       }
-    } else {
-      unsigned ExpectedParamPatterns = 1;
-      if (FD->getImplicitSelfDecl())
-        ExpectedParamPatterns++;
-      if (FD->getParameterLists().size() != ExpectedParamPatterns) {
+
+      // willSet/didSet implementations are never exposed to objc, they are
+      // always directly dispatched from the synthesized setter.
+      if (FD->isObservingAccessor()) {
         if (Diagnose) {
-          diagnose(AFD->getLoc(), diag::objc_invalid_on_func_curried,
-                   getObjCDiagnosticAttrKind(Reason));
+          diagnose(AFD->getLoc(), diag::objc_observing_accessor);
           describeObjCReason(*this, AFD, Reason);
         }
         return false;
       }
+      assert(FD->isGetterOrSetter() && "missing diags for other accessors");
+      return true;
     }
 
-    // willSet/didSet implementations are never exposed to objc, they are always
-    // directly dispatched from the synthesized setter.
-    if (FD->isObservingAccessor()) {
+    unsigned ExpectedParamPatterns = 1;
+    if (FD->getImplicitSelfDecl())
+      ExpectedParamPatterns++;
+    if (FD->getParameterLists().size() != ExpectedParamPatterns) {
       if (Diagnose) {
-        diagnose(AFD->getLoc(), diag::objc_observing_accessor);
+        diagnose(AFD->getLoc(), diag::objc_invalid_on_func_curried,
+                 getObjCDiagnosticAttrKind(Reason));
         describeObjCReason(*this, AFD, Reason);
       }
       return false;

test/decl/protocol/objc.swift

@@ -45,15 +45,15 @@ class C2a : P2 {
   func method(_: Int, class: ObjCClass) { }
 
   var empty: Bool {
-    get { } // expected-error{{Objective-C method 'empty' provided by getter for 'empty' does not match the requirement's selector ('checkIfEmpty')}}
+    get { }
   }
 }
 
 class C2b : P2 {
   @objc func method(_: Int, class: ObjCClass) { }
 
   @objc var empty: Bool {
-    @objc get { } // expected-error{{Objective-C method 'empty' provided by getter for 'empty' does not match the requirement's selector ('checkIfEmpty')}}{{10-10=(checkIfEmpty)}}
+    @objc get { }
   }
 }
 
@@ -146,3 +146,90 @@ class C4_5a : P4, P5 {
 class C6a : P6 {
   func doSomething(sender: AnyObject?) { } // expected-error{{method 'doSomething(sender:)' has different argument names from those required by protocol 'P6' ('doSomething')}}{{20-20=_ }}{{none}}
 }
+
+
+
+@objc protocol P7 {
+  var prop: Int {
+    @objc(getTheProp) get
+    @objc(setTheProp:) set
+  }
+}
+
+class C7 : P7 {
+  var prop: Int {
+    get { return 0 }
+    set {}
+  }
+}
+
+class C7a : P7 {
+  @objc var prop: Int {
+    get { return 0 }
+    set {}
+  }
+}
+
+class C7b : P7 {
+  @objc var prop: Int {
+    @objc(getTheProp) get { return 0 }
+    @objc(setTheProp:) set {}
+  }
+}
+
+class C7c : P7 {
+  var prop: Int = 0
+}
+
+class C7d : P7 {
+  @objc var prop: Int = 0
+}
+
+class C7e : P7 {
+  // FIXME: This should probably still complain.
+  @objc(notProp) var prop: Int {
+    get { return 0 }
+    set {}
+  }
+}
+
+class C7f : P7 {
+  var prop: Int {
+    @objc(getProp) get { return 0 } // expected-error {{Objective-C method 'getProp' provided by getter for 'prop' does not match the requirement's selector ('getTheProp')}} {{11-18=getTheProp}}
+    set {}
+  }
+}
+
+class C7g : P7 {
+  var prop: Int {
+    get { return 0 }
+    @objc(prop:) set {} // expected-error {{Objective-C method 'prop:' provided by setter for 'prop' does not match the requirement's selector ('setTheProp:')}} {{11-16=setTheProp:}}
+  }
+}
+
+@objc protocol P8 {
+  @objc optional var prop: Int {
+    @objc(getTheProp) get
+  }
+}
+
+class C8Base: P8 {}
+class C8Sub: C8Base {
+  var prop: Int { return 0 } // expected-note {{getter for 'prop' declared here}}
+
+  @objc(getTheProp) func collision() {} // expected-error {{method 'collision()' with Objective-C selector 'getTheProp' conflicts with getter for 'prop' with the same Objective-C selector}}
+}
+class C8SubA: C8Base {
+  var prop: Int {
+    @objc get { return 0 } // expected-note {{getter for 'prop' declared here}}
+  }
+
+  @objc(getTheProp) func collision() {} // expected-error {{method 'collision()' with Objective-C selector 'getTheProp' conflicts with getter for 'prop' with the same Objective-C selector}}
+}
+class C8SubB: C8Base {
+  var prop: Int {
+    @objc(getProp) get { return 0 }
+  }
+
+  @objc(getTheProp) func collision() {} // okay
+}