Make sure we infer selectors for accessors when checking a conformance (swiftlang#14794)

Usually this happens directly, through some use of the class and its
conformance. However, if a conformance is /only/ used to satisfy an
associated type, we seem to bypass the step that actually infers
selector names for accessors, even though we do it successfully for
methods. Fix this by making sure the accessor decls are validated when
a property is, something that normal uses of a property probably have
to do anyway.

Also, simplify inferObjCName by assuming/asserting that it is only
used on things that are already marked @objc.

https://bugs.swift.org/browse/SR-6944

Author: jrose-apple

lib/Sema/TypeCheckDecl.cpp

@@ -2659,6 +2659,9 @@ static void inferObjCName(TypeChecker &tc, ValueDecl *decl) {
   if (isa<DestructorDecl>(decl))
     return;
 
+  auto attr = decl->getAttrs().getAttribute<ObjCAttr>();
+  assert(attr && "should only be called on decls already marked @objc");
+
   // If this declaration overrides an @objc declaration, use its name.
   if (auto overridden = decl->getOverriddenDecl()) {
     if (overridden->isObjC()) {
@@ -2667,17 +2670,6 @@ static void inferObjCName(TypeChecker &tc, ValueDecl *decl) {
         // Determine the selector of the overridden method.
         ObjCSelector overriddenSelector = overriddenFunc->getObjCSelector();
 
-        // Dig out the @objc attribute on the method, if it exists.
-        auto attr = decl->getAttrs().getAttribute<ObjCAttr>();
-        if (!attr) {
-          // There was no @objc attribute; add one with the
-          // appropriate name.
-          decl->getAttrs().add(ObjCAttr::create(tc.Context,
-                                                overriddenSelector,
-                                                true));
-          return;
-        }
-
         // Determine whether there is a name conflict.
         bool shouldFixName = !attr->hasName();
         if (attr->hasName() && *attr->getName() != overriddenSelector) {
@@ -2711,18 +2703,6 @@ static void inferObjCName(TypeChecker &tc, ValueDecl *decl) {
         Identifier overriddenName = overriddenProp->getObjCPropertyName();
         ObjCSelector overriddenNameAsSel(tc.Context, 0, overriddenName);
 
-        // Dig out the @objc attribute, if specified.
-        auto attr = decl->getAttrs().getAttribute<ObjCAttr>();
-        if (!attr) {
-          // There was no @objc attribute; add one with the
-          // appropriate name.
-          decl->getAttrs().add(
-            ObjCAttr::createNullary(tc.Context,
-                                    overriddenName,
-                                    /*isNameImplicit=*/true));
-          return;
-        }
-
         // Determine whether there is a name conflict.
         bool shouldFixName = !attr->hasName();
         if (attr->hasName() && *attr->getName() != overriddenNameAsSel) {
@@ -2751,11 +2731,9 @@ static void inferObjCName(TypeChecker &tc, ValueDecl *decl) {
     }
   }
 
-  // Dig out the @objc attribute. If it already has a name, do
-  // nothing; the protocol conformance checker will handle any
-  // mismatches.
-  auto attr = decl->getAttrs().getAttribute<ObjCAttr>();
-  if (attr && attr->hasName()) return;
+  // If the decl already has a name, do nothing; the protocol conformance
+  // checker will handle any mismatches.
+  if (attr->hasName()) return;
 
   // When no override determined the Objective-C name, look for
   // requirements for which this declaration is a witness.
@@ -2799,13 +2777,8 @@ static void inferObjCName(TypeChecker &tc, ValueDecl *decl) {
 
   // If we have a name, install it via an @objc attribute.
   if (requirementObjCName) {
-    if (attr)
-      const_cast<ObjCAttr *>(attr)->setName(*requirementObjCName,
-                                            /*implicit=*/true);
-    else
-      decl->getAttrs().add(
-        ObjCAttr::create(tc.Context, *requirementObjCName,
-                         /*implicitName=*/true));
+    const_cast<ObjCAttr *>(attr)->setName(*requirementObjCName,
+                                          /*implicit=*/true);
   }
 }
 
@@ -3943,6 +3916,16 @@ static void validateAbstractStorageDecl(TypeChecker &TC,
   storage->setIsGetterMutating(computeIsGetterMutating(TC, storage));
   storage->setIsSetterMutating(computeIsSetterMutating(TC, storage));
 
+  // We can't delay validation of getters and setters on @objc properties,
+  // because if they never get validated at all then conformance checkers
+  // will complain about selector mismatches.
+  if (storage->isObjC()) {
+    if (auto *getter = storage->getGetter())
+      TC.validateDecl(getter);
+    if (auto *setter = storage->getSetter())
+      TC.validateDecl(setter);
+  }
+
   // Create a materializeForSet function if necessary.  This needs to
   // happen immediately so that subclass materializeForSet functions
   // will be properly marked as overriding it.

validation-test/Sema/Inputs/sr9644-helper.swift

@@ -0,0 +1,7 @@
+class Test: TestProto {
+  var swiftFoo: Int32 { return 0 }
+  var swiftBar: Int32 {
+    get { return 0 }
+    set {}
+  }
+}

validation-test/Sema/Inputs/sr9644.h

@@ -0,0 +1,4 @@
+@protocol TestProto
+@property (nonatomic, readonly, getter=getFoo) int foo __attribute__((swift_name("swiftFoo")));
+@property (nonatomic, getter=getTheBar, setter=setTheBar:) int bar __attribute__((swift_name("swiftBar")));
+@end

validation-test/Sema/sr9644.swift

@@ -0,0 +1,14 @@
+// RUN: %target-swift-frontend -typecheck -primary-file %s %S/Inputs/sr9644-helper.swift -import-objc-header %S/Inputs/sr9644.h -verify
+
+// REQUIRES: objc_interop
+// expected-no-warning
+
+protocol HasAssoc {
+  associatedtype Assoc: TestProto
+}
+
+struct Impl: HasAssoc {
+  // This used to trigger validation of the Test: TestProto conformance, but
+  // /without/ propagating the correct selector onto Test.swiftFoo's getter.
+  typealias Assoc = Test
+}