Merge pull request #63668 from beccadax/checkmate

Improve @objcImplementation member checking

Author: nkcsgexi

include/swift/AST/Decl.h

@@ -1633,6 +1633,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

@@ -1587,14 +1587,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

@@ -4065,6 +4065,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

@@ -460,3 +460,6 @@ SWIFT_REQUEST(TypeChecker, GetRuntimeDiscoverableAttributes,
 SWIFT_REQUEST(TypeChecker, LocalDiscriminatorsRequest,
               unsigned(DeclContext *),
               Cached, NoLocationInfo)
+SWIFT_REQUEST(TypeChecker, TypeCheckObjCImplementationRequest,
+              unsigned(ExtensionDecl *),
+              Cached, NoLocationInfo)

lib/AST/Decl.cpp

@@ -5269,8 +5269,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

@@ -5136,20 +5136,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 *
@@ -5199,7 +5199,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/Parse/ParseDecl.cpp

@@ -9047,13 +9047,14 @@ parseDeclDeinit(ParseDeclOptions Flags, DeclAttributes &Attributes) {
 
   DD->getAttrs() = Attributes;
 
-  // Reject 'destructor' functions outside of structs, enums, and classes.
+  // Reject 'destructor' functions outside of structs, enums, classes, and
+  // @objcImplementation extensions.
   //
   // Later in the type checker, we validate that structs/enums only do this if
-  // they are move only.
-  auto *nom = dyn_cast<NominalTypeDecl>(CurDeclContext);
-  if (!nom ||
-      (!isa<StructDecl>(nom) && !isa<EnumDecl>(nom) && !isa<ClassDecl>(nom))) {
+  // they are move only, and that @objcImplementations are main-body.
+  auto *ED = dyn_cast<ExtensionDecl>(CurDeclContext);
+  if (!(isa<StructDecl>(CurDeclContext) || isa<EnumDecl>(CurDeclContext) ||
+        isa<ClassDecl>(CurDeclContext) || (ED && ED->isObjCImplementation()))) {
     diagnose(DestructorLoc, diag::destructor_decl_outside_class);
 
     // Tell the type checker not to touch this destructor.