Merge pull request #19292 from jrose-apple/get-out-the-map

Minor improvements to the use of StringMap/StringSet

Author: jrose-apple

include/swift/Basic/SourceManager.h

@@ -31,13 +31,13 @@ class SourceManager {
   unsigned CodeCompletionOffset;
 
   /// Associates buffer identifiers to buffer IDs.
-  llvm::StringMap<unsigned> BufIdentIDMap;
+  llvm::DenseMap<StringRef, unsigned> BufIdentIDMap;
 
   /// A cache mapping buffer identifiers to vfs Status entries.
   ///
   /// This is as much a hack to prolong the lifetime of status objects as it is
   /// to speed up stats.
-  mutable llvm::StringMap<clang::vfs::Status> StatusCache;
+  mutable llvm::DenseMap<StringRef, clang::vfs::Status> StatusCache;
 
   // \c #sourceLocation directive handling.
   struct VirtualFile {

include/swift/Driver/DependencyGraph.h

@@ -143,33 +143,6 @@ class DependencyGraphImpl {
 
   LoadResult loadFromBuffer(const void *node, llvm::MemoryBuffer &buffer);
 
-  // FIXME: We should be able to use llvm::mapped_iterator for this, but
-  // StringMapConstIterator isn't quite an InputIterator (no ->).
-  class StringSetIterator {
-    llvm::StringSet<>::const_iterator I;
-  public:
-    using value_type = llvm::StringSet<>::const_iterator::value_type;
-    using iterator_category = std::input_iterator_tag;
-    using difference_type = ptrdiff_t;
-    using reference = value_type &;
-    using pointer = value_type *;
-
-    /*implicit*/ StringSetIterator(llvm::StringSet<>::const_iterator base)
-       : I(base) {}
-
-    StringSetIterator &operator++() {
-      ++I;
-      return *this;
-    }
-
-    StringRef operator*() const {
-      return I->getKey();
-    }
-
-    bool operator==(StringSetIterator other) const { return I == other.I; }
-    bool operator!=(StringSetIterator other) const { return I != other.I; }
-  };
-
 protected:
   LoadResult loadFromString(const void *node, StringRef data);
   LoadResult loadFromPath(const void *node, StringRef path);
@@ -195,9 +168,8 @@ class DependencyGraphImpl {
   }
 
 public:
-  llvm::iterator_range<StringSetIterator> getExternalDependencies() const {
-    return llvm::make_range(StringSetIterator(ExternalDependencies.begin()),
-                            StringSetIterator(ExternalDependencies.end()));
+  decltype(ExternalDependencies.keys()) getExternalDependencies() const {
+    return ExternalDependencies.keys();
   }
 };
 

lib/AST/ASTContext.cpp

@@ -124,6 +124,8 @@ struct ASTContext::Implementation {
   /// lazy parsers for imported module files and source files.
   llvm::SmallPtrSet<LazyMemberParser*, 2> lazyParsers;
 
+  // FIXME: This is a StringMap rather than a StringSet because StringSet
+  // doesn't allow passing in a pre-existing allocator.
   llvm::StringMap<char, llvm::BumpPtrAllocator&> IdentifierTable;
 
   /// The declaration of Swift.AssignmentPrecedence.

lib/ClangImporter/ClangImporter.cpp

@@ -3289,7 +3289,7 @@ bool ClangImporter::Implementation::forEachLookupTable(
   // Collect and sort the set of module names.
   SmallVector<StringRef, 4> moduleNames;
   for (const auto &entry : LookupTables) {
-    moduleNames.push_back(entry.first());
+    moduleNames.push_back(entry.first);
   }
   llvm::array_pod_sort(moduleNames.begin(), moduleNames.end());
 
@@ -3597,7 +3597,7 @@ void ClangImporter::Implementation::dumpSwiftLookupTables() {
   // Sort the module names so we can print in a deterministic order.
   SmallVector<StringRef, 4> moduleNames;
   for (const auto &lookupTable : LookupTables) {
-    moduleNames.push_back(lookupTable.first());
+    moduleNames.push_back(lookupTable.first);
   }
   array_pod_sort(moduleNames.begin(), moduleNames.end());
 

lib/ClangImporter/ImporterImpl.h

@@ -279,7 +279,8 @@ struct PlatformAvailability {
 };
 }
 
-using LookupTableMap = llvm::StringMap<std::unique_ptr<SwiftLookupTable>>;
+using LookupTableMap =
+    llvm::DenseMap<StringRef, std::unique_ptr<SwiftLookupTable>>;
 
 /// The result of importing a clang type. It holds both the Swift Type
 /// as well as a bool in which 'true' indicates either:
@@ -489,10 +490,6 @@ class LLVM_LIBRARY_VISIBILITY ClangImporter::Implementation
     return getNameImporter().getEnumKind(decl);
   }
 
-  // TODO: drop this accessor as soon as we further de-couple the swift name
-  // lookup tables from the Impl.
-  LookupTableMap &getLookupTables() { return LookupTables; }
-
 private:
   /// A mapping from imported declarations to their "alternate" declarations,
   /// for cases where a single Clang declaration is imported to two

lib/Driver/Driver.cpp

@@ -591,7 +591,7 @@ static bool populateOutOfDateMap(InputInfoMap &map,
     // If a file was removed, we've lost its dependency info. Rebuild everything.
     // FIXME: Can we do better?
     if (ShowIncrementalBuildDecisions) {
-      llvm::StringSet<> inputArgs;
+      llvm::DenseSet<StringRef> inputArgs;
       for (auto &inputPair : inputs) {
         inputArgs.insert(inputPair.second->getValue());
       }
@@ -1186,7 +1186,7 @@ static bool checkInputExistence(const Driver &D, const DerivedArgList &Args,
 void Driver::buildInputs(const ToolChain &TC,
                          const DerivedArgList &Args,
                          InputFileList &Inputs) const {
-  llvm::StringMap<StringRef> SourceFileNames;
+  llvm::DenseMap<StringRef, StringRef> SourceFileNames;
 
   for (Arg *A : Args) {
     if (A->getOption().getKind() == Option::InputClass) {

lib/Driver/ToolChain.cpp

@@ -296,7 +296,7 @@ static void
 sortJobsToMatchCompilationInputs(ArrayRef<const Job *> unsortedJobs,
                                  SmallVectorImpl<const Job *> &sortedJobs,
                                  Compilation &C) {
-  llvm::StringMap<const Job *> jobsByInput;
+  llvm::DenseMap<StringRef, const Job *> jobsByInput;
   for (const Job *J : unsortedJobs) {
     const CompileJobAction *CJA = cast<CompileJobAction>(&J->getSource());
     const InputAction *IA = findSingleSwiftInput(CJA);

lib/Driver/ToolChains.cpp

@@ -504,7 +504,7 @@ void ToolChain::JobContext::addFrontendCommandLineInputArguments(
     const bool mayHavePrimaryInputs, const bool useFileList,
     const bool usePrimaryFileList, const bool filterByType,
     ArgStringList &arguments) const {
-  llvm::StringSet<> primaries;
+  llvm::DenseSet<StringRef> primaries;
 
   if (mayHavePrimaryInputs) {
     for (const Action *A : InputActions) {

lib/IDE/Refactoring.cpp

@@ -387,15 +387,17 @@ class RenameRangeDetailCollector : public Renamer {
 };
 
 class TextReplacementsRenamer : public Renamer {
-  llvm::StringMap<char> &ReplaceTextContext;
+  llvm::StringSet<> &ReplaceTextContext;
   std::vector<Replacement> Replacements;
 
 public:
   const DeclNameViewer New;
 
 private:
   StringRef registerText(StringRef Text) {
-    return ReplaceTextContext.insert({Text, char()}).first->getKey();
+    if (Text.empty())
+      return Text;
+    return ReplaceTextContext.insert(Text).first->getKey();
   }
 
   StringRef getCallArgLabelReplacement(StringRef OldLabelRange,
@@ -497,7 +499,7 @@ class TextReplacementsRenamer : public Renamer {
 public:
   TextReplacementsRenamer(const SourceManager &SM, StringRef OldName,
                           StringRef NewName,
-                          llvm::StringMap<char> &ReplaceTextContext)
+                          llvm::StringSet<> &ReplaceTextContext)
       : Renamer(SM, OldName), ReplaceTextContext(ReplaceTextContext),
         New(NewName) {
     assert(Old.isValid() && New.isValid());
@@ -3295,7 +3297,7 @@ int swift::ide::syntacticRename(SourceFile *SF, ArrayRef<RenameLoc> RenameLocs,
     return true; // Already diagnosed.
 
   size_t index = 0;
-  llvm::StringMap<char> ReplaceTextContext;
+  llvm::StringSet<> ReplaceTextContext;
   for(const RenameLoc &Rename: RenameLocs) {
     ResolvedLoc &Resolved = ResolvedLocs[index++];
     TextReplacementsRenamer Renamer(SM, Rename.OldName, Rename.NewName,

lib/IRGen/IRGen.cpp

@@ -1010,7 +1010,7 @@ static void performParallelIRGeneration(
                   PrimaryGM->addLinkLibrary(linkLib);
                 });
 
-  llvm::StringSet<> referencedGlobals;
+  llvm::DenseSet<StringRef> referencedGlobals;
 
   for (auto it = irgen.begin(); it != irgen.end(); ++it) {
     IRGenModule *IGM = it->second;

lib/PrintAsObjC/PrintAsObjC.cpp

@@ -65,8 +65,8 @@ static bool isAnyObjectOrAny(Type type) {
 
 /// Returns true if \p name matches a keyword in any Clang language mode.
 static bool isClangKeyword(Identifier name) {
-  static const llvm::StringSet<> keywords = []{
-    llvm::StringSet<> set;
+  static const llvm::DenseSet<StringRef> keywords = []{
+    llvm::DenseSet<StringRef> set;
     // FIXME: clang::IdentifierInfo /nearly/ has the API we need to do this
     // in a more principled way, but not quite.
 #define KEYWORD(SPELLING, FLAGS) \

lib/SIL/SILVerifier.cpp

@@ -5065,7 +5065,7 @@ void SILModule::verify() const {
     return;
 #endif
   // Uniquing set to catch symbol name collisions.
-  llvm::StringSet<> symbolNames;
+  llvm::DenseSet<StringRef> symbolNames;
 
   // When merging partial modules, we only link functions from the current
   // module, without enabling "LinkAll" mode or running the SILLinker pass;

lib/Serialization/SerializedModuleLoader.cpp

@@ -20,6 +20,7 @@
 #include "swift/Basic/SourceManager.h"
 #include "swift/Basic/Version.h"
 #include "llvm/ADT/SmallString.h"
+#include "llvm/ADT/StringSet.h"
 #include "llvm/Support/FileSystem.h"
 #include "llvm/Support/MemoryBuffer.h"
 #include "llvm/Support/Path.h"
@@ -376,19 +377,15 @@ FileUnit *SerializedModuleLoader::loadAST(
     // Figure out /which/ dependencies are missing.
     // FIXME: Dependencies should be de-duplicated at serialization time,
     // not now.
-    llvm::StringMap<bool> duplicates;
+    llvm::StringSet<> duplicates;
     llvm::SmallVector<ModuleFile::Dependency, 4> missing;
     std::copy_if(loadedModuleFile->getDependencies().begin(),
                  loadedModuleFile->getDependencies().end(),
                  std::back_inserter(missing),
                  [&duplicates](const ModuleFile::Dependency &dependency)->bool {
       if (dependency.isLoaded() || dependency.isHeader())
         return false;
-      bool &seen = duplicates[dependency.RawPath];
-      if (seen)
-        return false;
-      seen = true;
-      return true;
+      return duplicates.insert(dependency.RawPath).second;
     });
 
     // FIXME: only show module part of RawAccessPath