Merge pull request #64525 from beccadax/checkmate-ii

Re-merge #63668: Improve @objcImplementation member checking

Author: swift-ci

include/swift/AST/Decl.h

@@ -1637,6 +1637,10 @@ class ExtensionDecl final : public GenericContext, public Decl,
   /// \c isObjCImplementation() returns \c true.
   Optional<Identifier> getCategoryNameForObjCImplementation() const;
 
+  /// If this extension represents an imported Objective-C category, returns the
+  /// category's name. Otherwise returns the empty identifier.
+  Identifier getObjCCategoryName() const;
+
   // Implement isa/cast/dyncast/etc.
   static bool classof(const Decl *D) {
     return D->getKind() == DeclKind::Extension;

include/swift/AST/DiagnosticsSema.def

@@ -1602,14 +1602,56 @@ ERROR(member_of_objc_implementation_not_objc_or_final,none,
       "%0 %1 does not match any %0 declared in the headers for %2; did you use "
       "the %0's Swift name?",
       (DescriptiveDeclKind, ValueDecl *, ValueDecl *))
-NOTE(fixit_add_objc_for_objc_implementation,none,
-     "add '@objc' to define an Objective-C-compatible %0 not declared in "
-     "the header",
+NOTE(fixit_add_private_for_objc_implementation,none,
+     "add 'private' or 'fileprivate' to define an Objective-C-compatible %0 "
+     "not declared in the header",
      (DescriptiveDeclKind))
 NOTE(fixit_add_final_for_objc_implementation,none,
      "add 'final' to define a Swift %0 that cannot be overridden",
      (DescriptiveDeclKind))
 
+ERROR(objc_implementation_wrong_category,none,
+      "%0 %1 should be implemented in extension for "
+      "%select{main class interface|category %2}2, not "
+      "%select{main class interface|category %3}3",
+      (DescriptiveDeclKind, ValueDecl *, Identifier, Identifier))
+
+ERROR(objc_implementation_wrong_objc_name,none,
+      "selector %0 for %1 %2 not found in header; did you mean %3?",
+      (ObjCSelector, DescriptiveDeclKind, ValueDecl *, ObjCSelector))
+
+ERROR(objc_implementation_wrong_swift_name,none,
+      "selector %0 used in header by an %1 with a different name; did you "
+      "mean %2?",
+      (ObjCSelector, DescriptiveDeclKind, DeclName))
+
+ERROR(objc_implementation_missing_impl,none,
+      "extension for %select{main class interface|category %0}0 should "
+      "provide implementation for %1 %2",
+      (Identifier, DescriptiveDeclKind, ValueDecl *))
+
+ERROR(objc_implementation_class_or_instance_mismatch,none,
+      "%0 %1 does not match %2 declared in header",
+      (DescriptiveDeclKind, ValueDecl *, DescriptiveDeclKind))
+
+ERROR(objc_implementation_multiple_matching_candidates,none,
+      "found multiple implementations that could match %0 %1 with selector %2",
+      (DescriptiveDeclKind, ValueDecl *, ObjCSelector))
+NOTE(objc_implementation_candidate_impl_here,none,
+     "%0 %1 is a potential match%select{|; insert '@objc(%3)' to use it}2",
+     (DescriptiveDeclKind, ValueDecl *, bool, StringRef))
+NOTE(objc_implementation_requirement_here,none,
+     "%0 %1 declared in header here",
+     (DescriptiveDeclKind, ValueDecl *))
+
+ERROR(objc_implementation_multiple_matching_requirements,none,
+      "%0 %1 could match several different members declared in the header",
+      (DescriptiveDeclKind, ValueDecl *))
+NOTE(objc_implementation_one_matched_requirement,none,
+     "%0 %1 (with selector %2) is a potential match%select{|; insert "
+     "'@objc(%4)' to use it}3",
+     (DescriptiveDeclKind, ValueDecl *, ObjCSelector, bool, StringRef))
+
 ERROR(cdecl_not_at_top_level,none,
       "@_cdecl can only be applied to global functions", ())
 ERROR(cdecl_empty_name,none,

include/swift/AST/TypeCheckRequests.h

@@ -4166,6 +4166,28 @@ class LocalDiscriminatorsRequest
   bool isCached() const { return true; }
 };
 
+/// Checks that all of a class's \c \@objcImplementation extensions provide
+/// complete and correct implementations for their corresponding interfaces.
+/// This is done on all of a class's implementations at once to improve diagnostics.
+class TypeCheckObjCImplementationRequest
+    : public SimpleRequest<TypeCheckObjCImplementationRequest,
+                           evaluator::SideEffect(ExtensionDecl *),
+                           RequestFlags::Cached> {
+public:
+  using SimpleRequest::SimpleRequest;
+
+private:
+  friend SimpleRequest;
+
+  // Evaluation.
+  evaluator::SideEffect
+  evaluate(Evaluator &evaluator, ExtensionDecl *ED) const;
+
+public:
+  // Separate caching.
+  bool isCached() const { return true; }
+};
+
 void simple_display(llvm::raw_ostream &out, ASTNode node);
 void simple_display(llvm::raw_ostream &out, Type value);
 void simple_display(llvm::raw_ostream &out, const TypeRepr *TyR);

include/swift/AST/TypeCheckerTypeIDZone.def

@@ -469,3 +469,6 @@ SWIFT_REQUEST(TypeChecker, LocalDiscriminatorsRequest,
 SWIFT_REQUEST(TypeChecker, IsNonUserModuleRequest,
               bool(ModuleDecl *),
               Cached, NoLocationInfo)
+SWIFT_REQUEST(TypeChecker, TypeCheckObjCImplementationRequest,
+              unsigned(ExtensionDecl *),
+              Cached, NoLocationInfo)

lib/AST/ASTPrinter.cpp

@@ -137,6 +137,11 @@ static bool contributesToParentTypeStorage(const AbstractStorageDecl *ASD) {
   return !ND->isResilient() && ASD->hasStorage() && !ASD->isStatic();
 }
 
+static bool isInObjCImpl(const ValueDecl *VD) {
+  auto *ED = dyn_cast<ExtensionDecl>(VD->getDeclContext());
+  return ED && ED->isObjCImplementation();
+}
+
 PrintOptions PrintOptions::printSwiftInterfaceFile(ModuleDecl *ModuleToPrint,
                                                    bool preferTypeRepr,
                                                    bool printFullConvention,
@@ -208,9 +213,9 @@ PrintOptions PrintOptions::printSwiftInterfaceFile(ModuleDecl *ModuleToPrint,
       if (!options.PrintSPIs && D->isSPI())
         return false;
 
-      // Skip anything that isn't 'public' or '@usableFromInline' or has a
-      // _specialize attribute with a targetFunction parameter.
       if (auto *VD = dyn_cast<ValueDecl>(D)) {
+        // Skip anything that isn't 'public' or '@usableFromInline' or has a
+        // _specialize attribute with a targetFunction parameter.
         if (!isPublicOrUsableFromInline(VD) &&
             !isPrespecilizationDeclWithTarget(VD)) {
           // We do want to print private stored properties, without their
@@ -221,6 +226,13 @@ PrintOptions PrintOptions::printSwiftInterfaceFile(ModuleDecl *ModuleToPrint,
 
           return false;
         }
+
+        // Skip member implementations and @objc overrides in @objcImpl
+        // extensions.
+        if (VD->isObjCMemberImplementation()
+            || (isInObjCImpl(VD) && VD->getOverriddenDecl() && VD->isObjC())) {
+          return false;
+        }
       }
 
       // Skip extensions that extend things we wouldn't print.
@@ -312,6 +324,7 @@ PrintOptions PrintOptions::printSwiftInterfaceFile(ModuleDecl *ModuleToPrint,
     DAK_AccessControl,
     DAK_SetterAccess,
     DAK_Lazy,
+    DAK_ObjCImplementation,
     DAK_StaticInitializeObjCMetadata,
     DAK_RestatedObjCConformance,
     DAK_NonSendable,
@@ -1125,8 +1138,10 @@ void PrintAST::printAttributes(const Decl *D) {
 
     if (auto vd = dyn_cast<VarDecl>(D)) {
       // Don't print @_hasInitialValue if we're printing an initializer
-      // expression or if the storage is resilient.
-      if (vd->isInitExposedToClients() || vd->isResilient())
+      // expression, if the storage is resilient, or if it's in an
+      // @objcImplementation extension (where final properties should appear
+      // computed).
+      if (vd->isInitExposedToClients() || vd->isResilient() || isInObjCImpl(vd))
         Options.ExcludeAttrList.push_back(DAK_HasInitialValue);
 
       if (!Options.PrintForSIL) {
@@ -2105,8 +2120,9 @@ void PrintAST::printAccessors(const AbstractStorageDecl *ASD) {
   // Don't print accessors for trivially stored properties...
   if (impl.isSimpleStored()) {
     // ...unless we're printing for SIL, which expects a { get set? } on
-    //    trivial properties
-    if (Options.PrintForSIL) {
+    //    trivial properties, or in an @objcImpl extension, which treats
+    //    final stored properties as computed.
+    if (Options.PrintForSIL || isInObjCImpl(ASD)) {
       Printer << " { get " << (impl.supportsMutation() ? "set }" : "}");
     }
     // ...or you're private/internal(set), at which point we'll print

lib/AST/Decl.cpp

@@ -5350,8 +5350,10 @@ synthesizeEmptyFunctionBody(AbstractFunctionDecl *afd, void *context) {
 
 DestructorDecl *
 GetDestructorRequest::evaluate(Evaluator &evaluator, ClassDecl *CD) const {
+  auto dc = CD->getImplementationContext();
+
   auto &ctx = CD->getASTContext();
-  auto *DD = new (ctx) DestructorDecl(CD->getLoc(), CD);
+  auto *DD = new (ctx) DestructorDecl(CD->getLoc(), dc->getAsGenericContext());
 
   DD->setImplicit();
 

lib/AST/NameLookupRequests.cpp

@@ -219,7 +219,7 @@ Optional<DestructorDecl *> GetDestructorRequest::getCachedResult() const {
 
 void GetDestructorRequest::cacheResult(DestructorDecl *value) const {
   auto *classDecl = std::get<0>(getStorage());
-  classDecl->addMember(value);
+  classDecl->getImplementationContext()->addMember(value);
 }
 
 //----------------------------------------------------------------------------//

lib/ClangImporter/ClangImporter.cpp

@@ -5183,20 +5183,20 @@ findImplsGivenInterface(ClassDecl *classDecl, Identifier categoryName) {
   return impls;
 }
 
-static Identifier getCategoryName(ExtensionDecl *ext) {
+Identifier ExtensionDecl::getObjCCategoryName() const {
   // Could it be an imported category?
-  if (!ext || !ext->hasClangNode())
+  if (!hasClangNode())
     // Nope, not imported.
     return Identifier();
 
-  auto category = dyn_cast<clang::ObjCCategoryDecl>(ext->getClangDecl());
+  auto category = dyn_cast<clang::ObjCCategoryDecl>(getClangDecl());
   if (!category)
     // Nope, not a category.
     return Identifier();
 
   // We'll look for an implementation with this category name.
   auto clangCategoryName = category->getName();
-  return ext->getASTContext().getIdentifier(clangCategoryName);
+  return getASTContext().getIdentifier(clangCategoryName);
 }
 
 static IterableDeclContext *
@@ -5246,7 +5246,7 @@ findContextInterfaceAndImplementation(DeclContext *dc) {
 
     // Is this an imported category? If so, extract its name so we can look for
     // implementations of that category.
-    categoryName = getCategoryName(ext);
+    categoryName = ext->getObjCCategoryName();
     if (categoryName.empty())
       return {};
 

lib/Sema/CodeSynthesis.cpp

@@ -937,6 +937,29 @@ static bool canInheritDesignatedInits(Evaluator &eval, ClassDecl *decl) {
          areAllStoredPropertiesDefaultInitializable(eval, decl);
 }
 
+static ValueDecl *findImplementedObjCDecl(ValueDecl *VD) {
+  // If VD has an ObjC name...
+  if (auto vdSelector = VD->getObjCRuntimeName()) {
+    // and it's in an extension...
+    if (auto implED = dyn_cast<ExtensionDecl>(VD->getDeclContext())) {
+      // and that extension is the @objcImplementation of a class's main body...
+      if (auto interfaceCD =
+              dyn_cast_or_null<ClassDecl>(implED->getImplementedObjCDecl())) {
+        // Find the initializer in the class's main body that matches VD.
+        for (auto interfaceVD : interfaceCD->getAllMembers()) {
+          if (auto interfaceCtor = dyn_cast<ConstructorDecl>(interfaceVD)) {
+            if (vdSelector == interfaceCtor->getObjCRuntimeName()) {
+              return interfaceCtor;
+            }
+          }
+        }
+      }
+    }
+  }
+
+  return VD;
+}
+
 static void collectNonOveriddenSuperclassInits(
     ClassDecl *subclass, SmallVectorImpl<ConstructorDecl *> &results) {
   auto *superclassDecl = subclass->getSuperclassDecl();
@@ -968,6 +991,15 @@ static void collectNonOveriddenSuperclassInits(
       subOptions, lookupResults);
 
   for (auto decl : lookupResults) {
+    // HACK: If an @objcImplementation extension declares an initializer, its
+    // interface usually also has a declaration. We need the interface decl for
+    // access control computations, but the name lookup returns the
+    // implementation decl because it's in the Swift module. Go find the
+    // matching interface decl.
+    // (Note that this is necessary for both newly-declared inits and overrides,
+    // even implicit ones.)
+    decl = findImplementedObjCDecl(decl);
+
     auto superclassCtor = cast<ConstructorDecl>(decl);
 
     // Skip invalid superclass initializers.