[libclang] Refactored SharedParsedRegionsStorage

[gribozavr]

Summary

Removed the PPRegionSetTy typedef because it is only used 3 times, and
obscures code more than it helps.

Renamed SharedParsedRegionsStorage to ThreadSafeParsedRegions, because
that better reflects the reason for this type to exist.

Replaced the copyTo() method that had an out parameter with a getter.

Renamed the merge() method to addParsedRegions().

Renamed ParsedSrcLocationsTracker::ParsedRegions to
ParsedRegionsSnapshot, which better reflects its role.

[jkorous]

Thanks for polishing this!

cfe/trunk/tools/libclang/Indexing.cpp

public:
   bool isInvalid() const { return *this == PPRegion(); }
 
   friend bool operator==(const PPRegion &lhs, const PPRegion &rhs) {
     return lhs.UniqueID == rhs.UniqueID && lhs.Offset == rhs.Offset &&
            lhs.ModTime == rhs.ModTime;
   }
 };
 
-typedef llvm::DenseSet<PPRegion> PPRegionSetTy;
-
 } // end anonymous namespace
 
 namespace llvm {
 
   template <>
   struct DenseMapInfo<PPRegion> {
     static inline PPRegion getEmptyKey() {
       return PPRegion(llvm::sys::fs::UniqueID(0, 0), unsigned(-1), 0);
namespace llvm {
   };
 }
 
 namespace {
 
 /// Keeps track of function bodies that have already been parsed.
 ///
 /// Is thread-safe.
-class SharedParsedRegionsStorage {
-  std::mutex Mux;
-  PPRegionSetTy ParsedRegions;
+class ThreadSafeParsedRegions {
+  mutable std::mutex Mutex;
+  llvm::DenseSet<PPRegion> ParsedRegions;
 
 public:
-  ~SharedParsedRegionsStorage() = default;
+  ~ThreadSafeParsedRegions() = default;
 
-  void copyTo(PPRegionSetTy &Set) {
-    std::lock_guard<std::mutex> MG(Mux);
-    Set = ParsedRegions;
+  llvm::DenseSet<PPRegion> getParsedRegions() const {
+    std::lock_guard<std::mutex> MG(Mutex);
+    return ParsedRegions;
   }
 
-  void merge(ArrayRef<PPRegion> Regions) {
-    std::lock_guard<std::mutex> MG(Mux);
+  void addParsedRegions(ArrayRef<PPRegion> Regions) {
+    std::lock_guard<std::mutex> MG(Mutex);
     ParsedRegions.insert(Regions.begin(), Regions.end());
   }
 };
 
 /// Provides information whether source locations have already been parsed in
 /// another FrontendAction.
 ///
 /// Is NOT thread-safe.
 class ParsedSrcLocationsTracker {
-  SharedParsedRegionsStorage &ParsedRegionsStorage;
+  ThreadSafeParsedRegions &ParsedRegionsStorage;
   PPConditionalDirectiveRecord &PPRec;
   Preprocessor &PP;
 
   /// Snapshot of the shared state at the point when this instance was
   /// constructed.
-  PPRegionSetTy ParsedRegions;
+  llvm::DenseSet<PPRegion> ParsedRegionsSnapshot;
   /// Regions that were queried during this instance lifetime.
   SmallVector<PPRegion, 32> NewParsedRegions;
 
   /// Caching the last queried region.
   PPRegion LastRegion;
   bool LastIsParsed;
 
 public:
   /// Creates snapshot of \p ParsedRegionsStorage.
-  ParsedSrcLocationsTracker(SharedParsedRegionsStorage &ParsedRegionsStorage,
+  ParsedSrcLocationsTracker(ThreadSafeParsedRegions &ParsedRegionsStorage,
                             PPConditionalDirectiveRecord &ppRec,
                             Preprocessor &pp)
-      : ParsedRegionsStorage(ParsedRegionsStorage), PPRec(ppRec), PP(pp) {
-    ParsedRegionsStorage.copyTo(ParsedRegions);
-  }
+      : ParsedRegionsStorage(ParsedRegionsStorage), PPRec(ppRec), PP(pp),
+        ParsedRegionsSnapshot(ParsedRegionsStorage.getParsedRegions()) {}
 
   /// \returns true iff \p Loc has already been parsed.
   ///
   /// Can provide false-negative in case the location was parsed after this
   /// instance had been constructed.
   bool hasAlredyBeenParsed(SourceLocation Loc, FileID FID,
                            const FileEntry *FE) {
     assert(FE);
     PPRegion region = getRegion(Loc, FID, FE);
     if (region.isInvalid())
       return false;
 
     // Check common case, consecutive functions in the same region.
     if (LastRegion == region)
       return LastIsParsed;
 
     LastRegion = region;
     // Source locations can't be revisited during single TU parsing.
     // That means if we hit the same region again, it's a different location in
     // the same region and so the "is parsed" value from the snapshot is still
     // correct.
-    LastIsParsed = ParsedRegions.count(region);
+    LastIsParsed = ParsedRegionsSnapshot.count(region);
     if (!LastIsParsed)
       NewParsedRegions.emplace_back(std::move(region));
     return LastIsParsed;
   }
 
   /// Updates ParsedRegionsStorage with newly parsed regions.
-  void syncWithStorage() { ParsedRegionsStorage.merge(NewParsedRegions); }
+  void syncWithStorage() {
+    ParsedRegionsStorage.addParsedRegions(NewParsedRegions);
+  }
 
 private:
   PPRegion getRegion(SourceLocation Loc, FileID FID, const FileEntry *FE) {
     assert(FE);
     auto Bail = [this, FE]() {
       if (isParsedOnceInclude(FE)) {
         const llvm::sys::fs::UniqueID &ID = FE->getUniqueID();
         return PPRegion(ID, 0, FE->getModificationTime());

 //===----------------------------------------------------------------------===//
 // IndexingFrontendAction
 //===----------------------------------------------------------------------===//
 
 class IndexingFrontendAction : public ASTFrontendAction {
   std::shared_ptr<CXIndexDataConsumer> DataConsumer;
   IndexingOptions Opts;
 
-  SharedParsedRegionsStorage *SKData;
+  ThreadSafeParsedRegions *SKData;
   std::unique_ptr<ParsedSrcLocationsTracker> ParsedLocsTracker;
 
 public:
   IndexingFrontendAction(std::shared_ptr<CXIndexDataConsumer> dataConsumer,
                          const IndexingOptions &Opts,
-                         SharedParsedRegionsStorage *skData)
+                         ThreadSafeParsedRegions *skData)
       : DataConsumer(std::move(dataConsumer)), Opts(Opts), SKData(skData) {}
 
   std::unique_ptr<ASTConsumer> CreateASTConsumer(CompilerInstance &CI,
                                                  StringRef InFile) override {
     PreprocessorOptions &PPOpts = CI.getPreprocessorOpts();
 
     if (!PPOpts.ImplicitPCHInclude.empty()) {
       auto File = CI.getFileManager().getFile(PPOpts.ImplicitPCHInclude);
if (index_options & CXIndexOpt_IndexFunctionLocalSymbols)
     IdxOpts.IndexFunctionLocals = true;
   if (index_options & CXIndexOpt_IndexImplicitTemplateInstantiations)
     IdxOpts.IndexImplicitInstantiation = true;
   return IdxOpts;
 }
 
 struct IndexSessionData {
   CXIndex CIdx;
-  std::unique_ptr<SharedParsedRegionsStorage> SkipBodyData;
+  std::unique_ptr<ThreadSafeParsedRegions> SkipBodyData =
+      std::make_unique<ThreadSafeParsedRegions>();
 
-  explicit IndexSessionData(CXIndex cIdx)
-      : CIdx(cIdx), SkipBodyData(new SharedParsedRegionsStorage) {}
+  explicit IndexSessionData(CXIndex cIdx) : CIdx(cIdx) {}
 };
 
 } // anonymous namespace
 
 static CXErrorCode clang_indexSourceFile_Impl(
     CXIndexAction cxIdxAction, CXClientData client_data,
     IndexerCallbacks *client_index_callbacks, unsigned index_callbacks_size,
     unsigned index_options, const char *source_filename,