[NFC] Fix objcImpl request cache

Several offsetting bugs both broke the caching of `ObjCInterfaceAndImplementationRequest` and caused it to usually miss. Fix this whole painful mess. Also has collateral improvements to simple_display().

Author: beccadax

include/swift/ClangImporter/ClangImporterRequests.h

@@ -256,7 +256,7 @@ struct ObjCInterfaceAndImplementation final {
   ObjCInterfaceAndImplementation()
       : interfaceDecls(), implementationDecl(nullptr) {}
 
-  operator bool() const {
+  bool empty() const {
     return interfaceDecls.empty();
   }
 

lib/AST/Decl.cpp

@@ -11291,7 +11291,7 @@ void swift::simple_display(llvm::raw_ostream &out, const Decl *decl) {
   }
 
   if (auto value = dyn_cast<ValueDecl>(decl)) {
-    simple_display(out, value);
+    return simple_display(out, value);
   } else if (auto ext = dyn_cast<ExtensionDecl>(decl)) {
     out << "extension of ";
     if (auto typeRepr = ext->getExtendedTypeRepr())
@@ -11301,13 +11301,13 @@ void swift::simple_display(llvm::raw_ostream &out, const Decl *decl) {
   } else if (auto med = dyn_cast<MacroExpansionDecl>(decl)) {
     out << '#' << med->getMacroName() << " in ";
     printContext(out, med->getDeclContext());
-    if (med->getLoc().isValid()) {
-      out << '@';
-      med->getLoc().print(out, med->getASTContext().SourceMgr);
-    }
   } else {
     out << "(unknown decl)";
   }
+  if (decl->getLoc().isValid()) {
+    out << '@';
+    decl->getLoc().print(out, decl->getASTContext().SourceMgr);
+  }
 }
 
 void swift::simple_display(llvm::raw_ostream &out,

lib/ClangImporter/ClangImporter.cpp

@@ -6139,15 +6139,15 @@ evaluate(Evaluator &evaluator, Decl *decl) const {
 
 void swift::simple_display(llvm::raw_ostream &out,
                            const ObjCInterfaceAndImplementation &pair) {
-  if (!pair) {
+  if (pair.empty()) {
     out << "no clang interface or @_objcImplementation";
     return;
   }
 
-  out << "clang interface ";
-  simple_display(out, pair.interfaceDecls);
-  out << " with @_objcImplementation ";
+  out << "@implementation ";
   simple_display(out, pair.implementationDecl);
+  out << " matches clang interfaces ";
+  simple_display(out, pair.interfaceDecls);
 }
 
 SourceLoc
@@ -6163,7 +6163,8 @@ llvm::TinyPtrVector<Decl *> Decl::getAllImplementedObjCDecls() const {
     return {};
 
   ObjCInterfaceAndImplementationRequest req{const_cast<Decl *>(this)};
-  return evaluateOrDefault(getASTContext().evaluator, req, {}).interfaceDecls;
+  auto result = evaluateOrDefault(getASTContext().evaluator, req, {});
+  return result.interfaceDecls;
 }
 
 DeclContext *DeclContext::getImplementedObjCContext() const {
@@ -6179,8 +6180,8 @@ Decl *Decl::getObjCImplementationDecl() const {
     return nullptr;
 
   ObjCInterfaceAndImplementationRequest req{const_cast<Decl *>(this)};
-  return evaluateOrDefault(getASTContext().evaluator, req, {})
-             .implementationDecl;
+  auto result = evaluateOrDefault(getASTContext().evaluator, req, {});
+  return result.implementationDecl;
 }
 
 llvm::TinyPtrVector<Decl *>

lib/ClangImporter/ClangImporterRequests.cpp

@@ -25,12 +25,14 @@ using namespace swift;
 std::optional<ObjCInterfaceAndImplementation>
 ObjCInterfaceAndImplementationRequest::getCachedResult() const {
   auto passedDecl = std::get<0>(getStorage());
-  if (!passedDecl)
+  if (!passedDecl) {
     return {};
+  }
 
-  if (!passedDecl->getCachedLacksObjCInterfaceOrImplementation())
+  if (passedDecl->getCachedLacksObjCInterfaceOrImplementation()) {
     // We've computed this request and found that this is a normal declaration.
     return {};
+  }
 
   // Either we've computed this request and cached a result in the ImporterImpl,
   // or we haven't computed this request. Check the caches.
@@ -44,20 +46,21 @@ ObjCInterfaceAndImplementationRequest::getCachedResult() const {
   // `ImplementationsByInterface`.
 
   Decl *implDecl = nullptr;
-  if (!passedDecl->hasClangNode()) {
-    // `passedDecl` *could* be an implementation.
+  if (passedDecl->hasClangNode()) {
+    // `passedDecl` *could* be an interface.
     auto iter = impl.ImplementationsByInterface.find(passedDecl);
     if (iter != impl.ImplementationsByInterface.end())
       implDecl = iter->second;
   } else {
-    // `passedDecl` *could* be an interface.
+    // `passedDecl` *could* be an implementation.
     implDecl = passedDecl;
   }
 
   if (implDecl) {
     auto iter = impl.InterfacesByImplementation.find(implDecl);
-    if (iter != impl.InterfacesByImplementation.end())
+    if (iter != impl.InterfacesByImplementation.end()) {
       return ObjCInterfaceAndImplementation(iter->second, implDecl);
+    }
   }
 
   // Nothing in the caches, so we must need to compute this.
@@ -68,7 +71,7 @@ void ObjCInterfaceAndImplementationRequest::
 cacheResult(ObjCInterfaceAndImplementation value) const {
   Decl *passedDecl = std::get<0>(getStorage());
 
-  if (!value) {
+  if (value.empty()) {
     // `decl` is neither an interface nor an implementation; remember this.
     passedDecl->setCachedLacksObjCInterfaceOrImplementation(true);
     return;