[cxx-interop] Remove some duplicated lookups

Repeatedly lookup up a key from a dictionary can be justified whenever
the content of the dictionary might change between the lookups (so any
references into the dictionary might get invalidated). We had a couple
of instances where as far as I can tell no such modifications should
happen between two lookups with identical keys. This PR simplifies the
code to remove the extra lookups. It also removes a dictionary that was
completely unused.

Author: Gabor Horvath

lib/ClangImporter/ClangImporter.cpp

@@ -2478,8 +2478,9 @@ ModuleDecl *ClangImporter::Implementation::finishLoadingClangModule(
   // well, and may do unnecessary work.
   ClangModuleUnit *wrapperUnit = getWrapperForModule(clangModule, importLoc);
   ModuleDecl *result = wrapperUnit->getParentModule();
-  if (!ModuleWrappers[clangModule].getInt()) {
-    ModuleWrappers[clangModule].setInt(true);
+  auto &moduleWrapper = ModuleWrappers[clangModule];
+  if (!moduleWrapper.getInt()) {
+    moduleWrapper.setInt(true);
     (void) namelookup::getAllImports(result);
   }
 
@@ -2889,11 +2890,11 @@ void ClangImporter::Implementation::addImportDiagnostic(
     ImportDiagnosticTarget target, Diagnostic &&diag,
     clang::SourceLocation loc) {
   ImportDiagnostic importDiag = ImportDiagnostic(target, diag, loc);
-  if (SwiftContext.LangOpts.DisableExperimentalClangImporterDiagnostics ||
-      CollectedDiagnostics.count(importDiag))
+  if (SwiftContext.LangOpts.DisableExperimentalClangImporterDiagnostics)
+    return;
+  auto [_, inserted] = CollectedDiagnostics.insert(importDiag);
+  if (!inserted)
     return;
-
-  CollectedDiagnostics.insert(importDiag);
   ImportDiagnostics[target].push_back(importDiag);
 }
 
@@ -6846,8 +6847,9 @@ void ClangImporter::Implementation::dumpSwiftLookupTables() {
   // Print out the lookup tables for the various modules.
   for (auto moduleName : moduleNames) {
     llvm::errs() << "<<" << moduleName << " lookup table>>\n";
-    LookupTables[moduleName]->deserializeAll();
-    LookupTables[moduleName]->dump(llvm::errs());
+    auto &lookupTable = LookupTables[moduleName];
+    lookupTable->deserializeAll();
+    lookupTable->dump(llvm::errs());
   }
 
   llvm::errs() << "<<Bridging header lookup table>>\n";
@@ -7410,8 +7412,10 @@ ClangImporter::getCXXFunctionTemplateSpecialization(SubstitutionMap subst,
   if (!newFn)
     return ConcreteDeclRef(decl);
 
-  if (Impl.specializedFunctionTemplates.count(newFn))
-    return ConcreteDeclRef(Impl.specializedFunctionTemplates[newFn]);
+  auto [fnIt, inserted] =
+      Impl.specializedFunctionTemplates.try_emplace(newFn, nullptr);
+  if (!inserted)
+    return ConcreteDeclRef(fnIt->second);
 
   auto newDecl = cast_or_null<ValueDecl>(
       decl->getASTContext().getClangModuleLoader()->importDeclDirectly(
@@ -7434,7 +7438,7 @@ ClangImporter::getCXXFunctionTemplateSpecialization(SubstitutionMap subst,
     }
   }
 
-  Impl.specializedFunctionTemplates[newFn] = newDecl;
+  fnIt->getSecond() = newDecl;
   return ConcreteDeclRef(newDecl);
 }
 
@@ -8006,7 +8010,7 @@ CxxRecordAsSwiftType::evaluate(Evaluator &evaluator,
   mod->lookupValue(desc.ctx.getIdentifier(swiftName), NLKind::UnqualifiedLookup,
                    results);
   if (results.size() == 1) {
-    if (dyn_cast<ClassDecl>(results[0]))
+    if (isa<ClassDecl>(results[0]))
       return results[0];
   }
   return nullptr;

lib/ClangImporter/DWARFImporter.cpp

@@ -112,14 +112,15 @@ ModuleDecl *ClangImporter::Implementation::loadModuleDWARF(
 
   // FIXME: Implement submodule support!
   Identifier name = path[0].Item;
-  auto it = DWARFModuleUnits.find(name);
-  if (it != DWARFModuleUnits.end())
+  auto [it, inserted] = DWARFModuleUnits.try_emplace(name, nullptr);
+  if (!inserted)
     return it->second->getParentModule();
 
+  auto itCopy = it; // Capturing structured bindings is a C++20 feature.
   auto *M = ModuleDecl::create(name, SwiftContext,
                                [&](ModuleDecl *M, auto addFile) {
     auto *wrapperUnit = new (SwiftContext) DWARFModuleUnit(*M, *this);
-    DWARFModuleUnits.insert({name, wrapperUnit});
+    itCopy->second = wrapperUnit;
     addFile(wrapperUnit);
   });
   M->setIsNonSwiftModule();

lib/ClangImporter/ImportDecl.cpp

@@ -74,6 +74,7 @@
 #include "llvm/ADT/StringExtras.h"
 #include "llvm/ADT/StringMap.h"
 #include "llvm/ADT/StringSwitch.h"
+#include "llvm/ADT/TinyPtrVector.h"
 #include "llvm/Support/Path.h"
 
 #include <algorithm>
@@ -2578,11 +2579,11 @@ namespace {
           Impl.addAlternateDecl(subscriptImpl, subscript);
         }
 
-        if (Impl.cxxDereferenceOperators.find(result) !=
-            Impl.cxxDereferenceOperators.end()) {
+        auto getterAndSetterIt = Impl.cxxDereferenceOperators.find(result);
+        if (getterAndSetterIt != Impl.cxxDereferenceOperators.end()) {
           // If this type has a dereference operator, synthesize a computed
           // property called `pointee` for it.
-          auto getterAndSetter = Impl.cxxDereferenceOperators[result];
+          auto getterAndSetter = getterAndSetterIt->second;
 
           VarDecl *pointeeProperty =
               synthesizer.makeDereferencedPointeeProperty(
@@ -5076,10 +5077,9 @@ namespace {
 
       // Check whether there's some special method to import.
       if (!forceClassMethod) {
-        if (dc == Impl.importDeclContextOf(decl, decl->getDeclContext()) &&
-            !Impl.ImportedDecls[{decl->getCanonicalDecl(), getVersion()}])
-          Impl.ImportedDecls[{decl->getCanonicalDecl(), getVersion()}]
-            = result;
+        if (dc == Impl.importDeclContextOf(decl, decl->getDeclContext()))
+          Impl.ImportedDecls.try_emplace(
+              {decl->getCanonicalDecl(), getVersion()}, result);
 
         if (importedName.isSubscriptAccessor()) {
           // If this was a subscript accessor, try to create a
@@ -7696,8 +7696,8 @@ SwiftDeclConverter::importSubscript(Decl *decl,
 
   // Note that we've created this subscript.
   Impl.Subscripts[{getter, setter}] = subscript;
-  if (setter && !Impl.Subscripts[{getter, nullptr}])
-    Impl.Subscripts[{getter, nullptr}] = subscript;
+  if (setter)
+    Impl.Subscripts.try_emplace({getter, nullptr}, subscript);
 
   // Make the getter/setter methods unavailable.
   if (!getter->isUnavailable())
@@ -8413,11 +8413,12 @@ SourceFile &ClangImporter::Implementation::getClangSwiftAttrSourceFile(
     StringRef attributeText,
     bool cached
 ) {
+  ::TinyPtrVector<SourceFile *> *sourceFiles = nullptr;
   if (cached) {
-    auto &sourceFiles = ClangSwiftAttrSourceFiles[attributeText];
+    sourceFiles = &ClangSwiftAttrSourceFiles[attributeText];
 
     // Check whether we've already created a source file.
-    for (auto sourceFile : sourceFiles) {
+    for (auto sourceFile : *sourceFiles) {
       if (sourceFile->getParentModule() == &module)
         return *sourceFile;
     }
@@ -8443,10 +8444,8 @@ SourceFile &ClangImporter::Implementation::getClangSwiftAttrSourceFile(
   auto sourceFile = new (SwiftContext)
       SourceFile(module, SourceFileKind::Library, bufferID);
 
-  if (cached) {
-    auto &sourceFiles = ClangSwiftAttrSourceFiles[attributeText];
-    sourceFiles.push_back(sourceFile);
-  }
+  if (cached)
+    sourceFiles->push_back(sourceFile);
 
   return *sourceFile;
 }
@@ -9839,9 +9838,9 @@ ClangImporter::Implementation::importDeclContextOf(
   // Clang submodule of the declaration.
   const clang::Module *declSubmodule = *getClangSubmoduleForDecl(decl);
   auto extensionKey = std::make_pair(nominal, declSubmodule);
-  auto knownExtension = extensionPoints.find(extensionKey);
-  if (knownExtension != extensionPoints.end())
-    return knownExtension->second;
+  auto [it, inserted] = extensionPoints.try_emplace(extensionKey, nullptr);
+  if (!inserted)
+    return it->getSecond();
 
   // Create a new extension for this nominal type/Clang submodule pair.
   auto ext = ExtensionDecl::create(SwiftContext, SourceLoc(), nullptr, {},
@@ -9854,7 +9853,7 @@ ClangImporter::Implementation::importDeclContextOf(
   // Record this extension so we can find it later. We do this early because
   // once we've set the member loader, we don't know when the compiler will use
   // it and end up back in this method.
-  extensionPoints[extensionKey] = ext;
+  it->getSecond() = ext;
   ext->setMemberLoader(this, reinterpret_cast<uintptr_t>(declSubmodule));
 
   if (auto protoDecl = ext->getExtendedProtocolDecl()) {

lib/ClangImporter/ImportMacro.cpp

@@ -779,10 +779,11 @@ ValueDecl *ClangImporter::Implementation::importMacro(Identifier name,
   PrettyStackTraceStringAction stackRAII{"importing macro", name.str()};
 
   // Look for macros imported with the same name.
-  auto known = ImportedMacros.find(name);
-  if (known == ImportedMacros.end()) {
+  auto [known, inserted] = ImportedMacros.try_emplace(
+      name, SmallVector<std::pair<const clang::MacroInfo *, ValueDecl *>, 2>{});
+  if (inserted) {
     // Push in a placeholder to break circularity.
-    ImportedMacros[name].push_back({macro, nullptr});
+    known->getSecond().push_back({macro, nullptr});
   } else {
     // Check whether this macro has already been imported.
     for (const auto &entry : known->second) {

lib/ClangImporter/ImportType.cpp

@@ -3574,8 +3574,8 @@ static ModuleDecl *tryLoadModule(ASTContext &C,
                              llvm::DenseMap<Identifier, ModuleDecl *>
                                &checkedModules) {
   // If we've already done this check, return the cached result.
-  auto known = checkedModules.find(moduleName);
-  if (known != checkedModules.end())
+  auto [known, inserted] = checkedModules.try_emplace(moduleName, nullptr);
+  if (!inserted)
     return known->second;
 
   ModuleDecl *module;
@@ -3587,7 +3587,7 @@ static ModuleDecl *tryLoadModule(ASTContext &C,
   else
     module = C.getModuleByIdentifier(moduleName);
 
-  checkedModules[moduleName] = module;
+  known->getSecond() = module;
   return module;
 }
 

lib/ClangImporter/ImporterImpl.h

@@ -801,10 +801,6 @@ class LLVM_LIBRARY_VISIBILITY ClangImporter::Implementation
   llvm::DenseMap<const Decl *, ArrayRef<ProtocolDecl *>>
     ImportedProtocols;
 
-  /// The set of declaration context for which we've already ruled out the
-  /// presence of globals-as-members.
-  llvm::DenseSet<const IterableDeclContext *> checkedGlobalsAsMembers;
-
   void startedImportingEntity();
 
 public: