Cache @_objcImpl pointers and diagnose dupes

Author: beccadax

include/swift/AST/Decl.h

@@ -723,6 +723,13 @@ class alignas(1 << DeclAlignInBits) Decl : public ASTAllocated<Decl> {
 private:
   llvm::PointerUnion<DeclContext *, ASTContext *> Context;
 
+  /// The imported Clang declaration representing the \c @_objcInterface for
+  /// this declaration (or vice versa), or \c nullptr if there is none.
+  ///
+  /// If \c this (an otherwise nonsensical value), the value has not yet been
+  /// computed.
+  Decl *CachedObjCImplementationDecl;
+
   Decl(const Decl&) = delete;
   void operator=(const Decl&) = delete;
   SourceLoc getLocFromSource() const;
@@ -740,7 +747,7 @@ class alignas(1 << DeclAlignInBits) Decl : public ASTAllocated<Decl> {
 protected:
 
   Decl(DeclKind kind, llvm::PointerUnion<DeclContext *, ASTContext *> context)
-    : Context(context) {
+    : Context(context), CachedObjCImplementationDecl(this) {
     Bits.OpaqueBits = 0;
     Bits.Decl.Kind = unsigned(kind);
     Bits.Decl.Invalid = false;
@@ -975,6 +982,32 @@ class alignas(1 << DeclAlignInBits) Decl : public ASTAllocated<Decl> {
     return getClangNodeImpl().getAsMacro();
   }
 
+  /// If this is the Swift implementation of a declaration imported from ObjC,
+  /// returns the imported declaration. Otherwise return \c nullptr.
+  ///
+  /// \seeAlso ExtensionDecl::isObjCInterface()
+  Decl *getImplementedObjCDecl() const;
+
+  /// If this is the ObjC interface of a declaration implemented in Swift,
+  /// returns the implementating declaration. Otherwise return \c nullptr.
+  ///
+  /// \seeAlso ExtensionDecl::isObjCInterface()
+  Decl *getObjCImplementationDecl() const;
+
+  Optional<Decl *> getCachedObjCImplementationDecl() const {
+    if (CachedObjCImplementationDecl == this)
+      return None;
+    return CachedObjCImplementationDecl;
+  }
+
+  void setCachedObjCImplementationDecl(Decl *decl) {
+    assert((CachedObjCImplementationDecl == this
+              || CachedObjCImplementationDecl == decl)
+               && "can't change CachedObjCInterfaceDecl once it's computed");
+    assert(decl != this && "can't form circular reference");
+    CachedObjCImplementationDecl = decl;
+  }
+
   /// Return the GenericContext if the Decl has one.
   LLVM_READONLY
   const GenericContext *getAsGenericContext() const;
@@ -1506,13 +1539,6 @@ class ExtensionDecl final : public GenericContext, public Decl,
   /// \c isObjCImplementation() returns \c true.
   Optional<Identifier> getCategoryNameForObjCImplementation() const;
 
-  /// Returns the \c clang::ObjCCategoryDecl or \c clang::ObjCInterfaceDecl
-  /// implemented by this extension.
-  ///
-  /// This can return \c nullptr if the category doesn't exist. Do not call it
-  /// unless \c isObjCImplementation() returns \c true.
-  const clang::ObjCContainerDecl *getInterfaceForObjCImplementation() const;
-
   // Implement isa/cast/dyncast/etc.
   static bool classof(const Decl *D) {
     return D->getKind() == DeclKind::Extension;
@@ -4380,7 +4406,7 @@ class ClassDecl final : public NominalTypeDecl {
   /// will always be an Objective-C-backed \c ExtensionDecl or, if \p name is
   /// empty, \c ClassDecl. Returns \c nullptr if the class was not imported from
   /// Objective-C or does not have an imported category by that name.
-  Decl *getImportedObjCCategory(Identifier name) const;
+  IterableDeclContext *getImportedObjCCategory(Identifier name) const;
 
   // Implement isa/cast/dyncast/etc.
   static bool classof(const Decl *D) {

include/swift/AST/DiagnosticsClangImporter.def

@@ -132,6 +132,13 @@ WARNING(api_pattern_attr_ignored, none,
         "'%0' swift attribute ignored on type '%1': type is not copyable or destructible",
         (StringRef, StringRef))
 
+ERROR(objc_implementation_two_impls, none,
+      "duplicate implementation of Objective-C %select{|category %0 on }0"
+      "class %1",
+      (Identifier, ValueDecl *))
+NOTE(previous_objc_implementation, none,
+     "previously implemented by extension here", ())
+
 NOTE(macro_not_imported_unsupported_operator, none, "operator not supported in macro arithmetic", ())
 NOTE(macro_not_imported_unsupported_named_operator, none, "operator '%0' not supported in macro arithmetic", (StringRef))
 NOTE(macro_not_imported_invalid_string_literal, none, "invalid string literal", ())

include/swift/ClangImporter/ClangImporterRequests.h

@@ -217,7 +217,7 @@ SourceLoc extractNearestSourceLoc(const ClangCategoryLookupDescriptor &desc);
 /// An empty/invalid \c categoryName requests the main interface for the class.
 class ClangCategoryLookupRequest
     : public SimpleRequest<ClangCategoryLookupRequest,
-                           Decl *(ClangCategoryLookupDescriptor),
+                           IterableDeclContext *(ClangCategoryLookupDescriptor),
                            RequestFlags::Uncached> {
 public:
   using SimpleRequest::SimpleRequest;
@@ -226,8 +226,78 @@ class ClangCategoryLookupRequest
   friend SimpleRequest;
 
   // Evaluation.
-  Decl *evaluate(Evaluator &evaluator,
-                 ClangCategoryLookupDescriptor desc) const;
+  IterableDeclContext *evaluate(Evaluator &evaluator,
+                                ClangCategoryLookupDescriptor desc) const;
+};
+
+/// Links an imported Clang decl to the native Swift decl(s) that implement it
+/// using \c \@_objcImplementation.
+struct ObjCInterfaceAndImplementation final {
+  Decl *interfaceDecl;
+  Decl *implementationDecl;
+
+  ObjCInterfaceAndImplementation(Decl *interfaceDecl,
+                                 Decl *implementationDecl)
+      : interfaceDecl(interfaceDecl), implementationDecl(implementationDecl)
+  {
+    assert(interfaceDecl && implementationDecl &&
+           "interface and implementation are both non-null");
+  }
+
+  ObjCInterfaceAndImplementation()
+      : interfaceDecl(nullptr), implementationDecl(nullptr) {}
+
+  operator bool() const {
+    return interfaceDecl;
+  }
+
+  friend llvm::hash_code
+  hash_value(const ObjCInterfaceAndImplementation &pair) {
+    return llvm::hash_combine(pair.interfaceDecl, pair.implementationDecl);
+  }
+
+  friend bool operator==(const ObjCInterfaceAndImplementation &lhs,
+                         const ObjCInterfaceAndImplementation &rhs) {
+    return lhs.interfaceDecl == rhs.interfaceDecl
+               && lhs.implementationDecl == rhs.implementationDecl;
+  }
+
+  friend bool operator!=(const ObjCInterfaceAndImplementation &lhs,
+                         const ObjCInterfaceAndImplementation &rhs) {
+    return !(lhs == rhs);
+  }
+};
+
+void simple_display(llvm::raw_ostream &out,
+                    const ObjCInterfaceAndImplementation &desc);
+SourceLoc extractNearestSourceLoc(const ObjCInterfaceAndImplementation &desc);
+
+/// Given a \c Decl whose declaration is imported from ObjC but whose
+/// implementation is provided by a Swift \c \@_objcImplementation
+/// \c extension , return both decls, with the imported interface first.
+/// Otherwise return \c {nullptr,nullptr} .
+///
+/// We retrieve both in a single request because we want to cache the
+/// relationship on both sides to avoid duplicating work.
+class ObjCInterfaceAndImplementationRequest
+    : public SimpleRequest<ObjCInterfaceAndImplementationRequest,
+                           ObjCInterfaceAndImplementation(Decl *),
+                           RequestFlags::SeparatelyCached> {
+public:
+  using SimpleRequest::SimpleRequest;
+
+private:
+  friend SimpleRequest;
+
+  // Evaluation.
+  ObjCInterfaceAndImplementation
+  evaluate(Evaluator &evaluator, Decl *decl) const;
+
+ public:
+   // Separate caching.
+   bool isCached() const { return true; }
+   Optional<ObjCInterfaceAndImplementation> getCachedResult() const;
+   void cacheResult(ObjCInterfaceAndImplementation value) const;
 };
 
 enum class CxxRecordSemanticsKind {

include/swift/ClangImporter/ClangImporterTypeIDZone.def

@@ -25,7 +25,10 @@ SWIFT_REQUEST(ClangImporter, ClangRecordMemberLookup,
               Decl *(ClangRecordMemberLookupDescriptor), Uncached,
               NoLocationInfo)
 SWIFT_REQUEST(ClangImporter, ClangCategoryLookupRequest,
-              Decl *(ClangCategoryLookupDescriptor), Uncached,
+              IterableDeclContext *(ClangCategoryLookupDescriptor), Uncached,
+              NoLocationInfo)
+SWIFT_REQUEST(ClangImporter, ObjCInterfaceAndImplementationRequest,
+              ObjCInterfaceAndImplementation(Decl *), SeparatelyCached,
               NoLocationInfo)
 SWIFT_REQUEST(ClangImporter, CxxRecordSemantics,
               CxxRecordSemanticsKind(const clang::CXXRecordDecl *), Cached,

lib/AST/Decl.cpp

@@ -1469,25 +1469,6 @@ bool ExtensionDecl::isObjCImplementation() const {
   return getAttrs().hasAttribute<ObjCImplementationAttr>();
 }
 
-const clang::ObjCContainerDecl *
-ExtensionDecl::getInterfaceForObjCImplementation() const {
-  auto categoryName = getCategoryNameForObjCImplementation();
-  if (!categoryName)
-    return nullptr;
-
-  auto CD = dyn_cast_or_null<ClassDecl>(getExtendedNominal());
-  if (!CD)
-    return nullptr;
-
-  auto importedDecl = CD->getImportedObjCCategory(*categoryName);
-  if (!importedDecl)
-    return nullptr;
-
-  assert(importedDecl->hasClangNode() &&
-         "@interface imported as clang-node-less decl?");
-  return cast<clang::ObjCContainerDecl>(importedDecl->getClangDecl());
-}
-
 Optional<Identifier>
 ExtensionDecl::getCategoryNameForObjCImplementation() const {
   assert(isObjCImplementation());

lib/AST/DiagnosticEngine.cpp

@@ -644,10 +644,16 @@ static void formatDiagnosticArgument(StringRef Modifier,
     break;
 
   case DiagnosticArgumentKind::Identifier:
-    assert(Modifier.empty() && "Improper modifier for identifier argument");
-    Out << FormatOpts.OpeningQuotationMark;
-    Arg.getAsIdentifier().printPretty(Out);
-    Out << FormatOpts.ClosingQuotationMark;
+    if (Modifier == "select") {
+      formatSelectionArgument(ModifierArguments, Args,
+                              Arg.getAsIdentifier() ? 1 : 0, FormatOpts,
+                              Out);
+    } else {
+      assert(Modifier.empty() && "Improper modifier for identifier argument");
+      Out << FormatOpts.OpeningQuotationMark;
+      Arg.getAsIdentifier().printPretty(Out);
+      Out << FormatOpts.ClosingQuotationMark;
+    }
     break;
 
   case DiagnosticArgumentKind::ObjCSelector: