[clang][index] Move ParsedSrcLocationsTracker to libIndex

[jkorous]

Summary

This patch just exposes ParsedSrcLocationsTracker in libIndex.

In https://reviews.llvm.org/D66125 we originally discussed possibility of wrapping this in some simpler API. Unfortunately I don't see a way how to do that as the primary use-case is multi-threaded indexing via libclang.

The way this works is that libclang client keeps shared state of what has already been indexed and pass it to libclang functions it calls. I. e. we need to preserve some state and need to deal with synchronization - this is the purpose of SharedParsedRegionsStorage.
We also need access to preprocessor which we get only in CreateASTConsumer() of relevant frontend action. I. e. we need to store this reference (plus reference to PPConditionalDirectiveRecord) somewhere (currently ParsedSrcLocationsTracker).

I don't see a way how to make this much simpler but open to ideas! (Also, not entirely happy with naming - suggestions are welcome.)

[jkorous]

• Addressed some comments from https://reviews.llvm.org/D66694.
• Fixed included header name.

[gribozavr]

In https://reviews.llvm.org/D66125 we originally discussed possibility of wrapping this in some simpler API. Unfortunately I don't see a way how to do that as the primary use-case is multi-threaded indexing via libclang.

Sorry for not being able to communicate what I meant. Please take a look at this patch series that removes unnecessary layers of FrontendActions, adds the callback that I was talking about, and refactors clangd and libclang to use that callback:

https://reviews.llvm.org/D66875
https://reviews.llvm.org/D66876
https://reviews.llvm.org/D66877
https://reviews.llvm.org/D66878
https://reviews.llvm.org/D66879

[gribozavr]

With those patches committed, libIndex exposes a callback for skipping function bodies that both libclang and clangd now use. I don't think there's a reason to move these types to libIndex, they will continue having only one user, libclang. WDYT?

[jkorous]

Sorry, I missed your reply.

Sure, works for me! For some reason I thought you considered this feature to be interesting enough to be exposed in libIndex.

clang/include/clang/Index/ParsedSrcLocationsTracker.h

+//===- ParsedSrcLocationsTracker.h ------------------------------*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_CLANG_INDEX_PARSEDSRCLOCATIONSTRACKER_H
+#define LLVM_CLANG_INDEX_PARSEDSRCLOCATIONSTRACKER_H
+
+#include "clang/Lex/HeaderSearch.h"
+#include "clang/Lex/PPConditionalDirectiveRecord.h"
+#include "clang/Lex/Preprocessor.h"
+#include "llvm/ADT/ArrayRef.h"
+#include "llvm/ADT/DenseSet.h"
+#include "llvm/ADT/FoldingSet.h"
+#include "llvm/Support/FileSystem.h"
+
+#include <mutex>
+
+namespace clang {
+namespace index {
+
+/// A "region" in source code identified by the file/offset of the
+/// preprocessor conditional directive that it belongs to.
+/// Multiple, non-consecutive ranges can be parts of the same region.
+///
+/// As an example of different regions separated by preprocessor directives:
+///
+/// \code
+///   #1
+/// #ifdef BLAH
+///   #2
+/// #ifdef CAKE
+///   #3
+/// #endif
+///   #2
+/// #endif
+///   #1
+/// \endcode
+///
+/// There are 3 regions, with non-consecutive parts:
+///   #1 is identified as the beginning of the file
+///   #2 is identified as the location of "#ifdef BLAH"
+///   #3 is identified as the location of "#ifdef CAKE"
+
+class PPRegion {
+  llvm::sys::fs::UniqueID UniqueID;
+  time_t ModTime;
+  unsigned Offset;
+public:
+  PPRegion() : UniqueID(0, 0), ModTime(), Offset() {}
+  PPRegion(llvm::sys::fs::UniqueID UniqueID, unsigned offset, time_t modTime)
+      : UniqueID(UniqueID), ModTime(modTime), Offset(offset) {}
+
+  const llvm::sys::fs::UniqueID &getUniqueID() const { return UniqueID; }
+  unsigned getOffset() const { return Offset; }
+  time_t getModTime() const { return ModTime; }
+
+  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;
+
+} // namespace index
+} // namespace clang
+
+namespace llvm {
+  using clang::index::PPRegion;
+
+  template <>
+  struct DenseMapInfo<PPRegion> {
+    static inline PPRegion getEmptyKey() {
+      return PPRegion(llvm::sys::fs::UniqueID(0, 0), unsigned(-1), 0);
+    }
+    static inline PPRegion getTombstoneKey() {
+      return PPRegion(llvm::sys::fs::UniqueID(0, 0), unsigned(-2), 0);
+    }
+
+    static unsigned getHashValue(const PPRegion &S) {
+      llvm::FoldingSetNodeID ID;
+      const llvm::sys::fs::UniqueID &UniqueID = S.getUniqueID();
+      ID.AddInteger(UniqueID.getFile());
+      ID.AddInteger(UniqueID.getDevice());
+      ID.AddInteger(S.getOffset());
+      ID.AddInteger(S.getModTime());
+      return ID.ComputeHash();
+    }
+
+    static bool isEqual(const PPRegion &LHS, const PPRegion &RHS) {
+      return LHS == RHS;
+    }
+  };
+} // namespace llvm
+
+namespace clang {
+namespace index {
+
+/// Keeps track of function bodies that have already been parsed.
+class ThreadSafeParsedRegions {
+  std::mutex Mux;
+  PPRegionSetTy ParsedRegions;
+
+public:
+  ~ThreadSafeParsedRegions() = default;
+
+  void copyTo(PPRegionSetTy &Set) {
+    std::lock_guard<std::mutex> MG(Mux);
+    Set = ParsedRegions;
+  }
+
+  void addParsedRegions(ArrayRef<PPRegion> Regions) {
+    std::lock_guard<std::mutex> MG(Mux);
+    ParsedRegions.insert(Regions.begin(), Regions.end());
+  }
+};
+
+/// Provides information whether source locations have already been parsed in
+/// another FrontendAction.
+///
+/// Is NOT thread-safe.
+class ParsedSrcLocationsTracker {
+  ThreadSafeParsedRegions &ParsedRegionsStorage;
+  PPConditionalDirectiveRecord &PPRec;
+  Preprocessor &PP;
+
+  /// Snapshot of the shared state at the point when this instance was
+  /// constructed.
+  PPRegionSetTy ParsedRegions;
+  /// 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(ThreadSafeParsedRegions &ParsedRegionsStorage,
+                            PPConditionalDirectiveRecord &ppRec,
+                            Preprocessor &pp)
+      : ParsedRegionsStorage(ParsedRegionsStorage), PPRec(ppRec), PP(pp) {
+    ParsedRegionsStorage.copyTo(ParsedRegions);
+  }
+
+  /// \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);
+
+  /// Updates ParsedRegionsStorage with newly parsed regions.
+  void syncWithStorage();
+private:
+  PPRegion getRegion(SourceLocation Loc, FileID FID, const FileEntry *FE);
+  bool isParsedOnceInclude(const FileEntry *FE);
+};
+
+} // namespace index
+} // namespace clang
+
+#endif // LLVM_CLANG_INDEX_SKIPALREADYINDEXEDCODE_H

clang/lib/Index/CMakeLists.txt

 set(LLVM_LINK_COMPONENTS
   Core
   Support
   )
 
 add_clang_library(clangIndex
   CodegenNameGenerator.cpp
   CommentToXML.cpp
   FileIndexRecord.cpp
   IndexBody.cpp
   IndexDecl.cpp
   IndexingAction.cpp
   IndexingContext.cpp
   IndexSymbol.cpp
   IndexTypeSourceInfo.cpp
+  ParsedSrcLocationsTracker.cpp
   USRGeneration.cpp
 
   ADDITIONAL_HEADERS
   IndexingContext.h
   SimpleFormatContext.h
 
   LINK_LIBS
   clangAST
   clangBasic
   clangFormat
   clangFrontend
   clangLex
   clangRewrite
   clangSerialization
   clangToolingCore
   )

clang/lib/Index/ParsedSrcLocationsTracker.cpp

+//===- ParsedSrcLocationsTracker.cpp ----------------------------*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "clang/Index/ParsedSrcLocationsTracker.h"
+
+namespace clang {
+namespace index {
+
+bool ParsedSrcLocationsTracker::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);
+  if (!LastIsParsed)
+    NewParsedRegions.emplace_back(std::move(region));
+  return LastIsParsed;
+}
+
+void ParsedSrcLocationsTracker::syncWithStorage() { ParsedRegionsStorage.addParsedRegions(NewParsedRegions); }
+
+PPRegion ParsedSrcLocationsTracker::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());
+    }
+    return PPRegion();
+  };
+
+  SourceLocation RegionLoc = PPRec.findConditionalDirectiveRegionLoc(Loc);
+  assert(RegionLoc.isFileID());
+  if (RegionLoc.isInvalid())
+    return Bail();
+
+  FileID RegionFID;
+  unsigned RegionOffset;
+  std::tie(RegionFID, RegionOffset) =
+      PPRec.getSourceManager().getDecomposedLoc(RegionLoc);
+
+  if (RegionFID != FID)
+    return Bail();
+
+  const llvm::sys::fs::UniqueID &ID = FE->getUniqueID();
+  return PPRegion(ID, RegionOffset, FE->getModificationTime());
+}
+
+bool ParsedSrcLocationsTracker::isParsedOnceInclude(const FileEntry *FE) {
+  return PP.getHeaderSearchInfo().isFileMultipleIncludeGuarded(FE);
+}
+
+} // namespace index
+} // namespace clang

clang/tools/libclang/Indexing.cpp

 #include "CXTranslationUnit.h"
 #include "clang/AST/ASTConsumer.h"
 #include "clang/Frontend/ASTUnit.h"
 #include "clang/Frontend/CompilerInstance.h"
 #include "clang/Frontend/CompilerInvocation.h"
 #include "clang/Frontend/FrontendAction.h"
 #include "clang/Frontend/Utils.h"
 #include "clang/Index/IndexingAction.h"
+#include "clang/Index/ParsedSrcLocationsTracker.h"
 #include "clang/Lex/HeaderSearch.h"
 #include "clang/Lex/PPCallbacks.h"
 #include "clang/Lex/PPConditionalDirectiveRecord.h"
 #include "clang/Lex/Preprocessor.h"
 #include "clang/Lex/PreprocessorOptions.h"
 #include "llvm/Support/CrashRecoveryContext.h"
 #include "llvm/Support/MemoryBuffer.h"
 #include <cstdio>
 #include <mutex>
 #include <utility>
 
 using namespace clang;
 using namespace clang::index;
 using namespace cxtu;
 using namespace cxindex;
 
 namespace {
 
 //===----------------------------------------------------------------------===//
-// Skip Parsed Bodies
-//===----------------------------------------------------------------------===//
-
-/// A "region" in source code identified by the file/offset of the
-/// preprocessor conditional directive that it belongs to.
-/// Multiple, non-consecutive ranges can be parts of the same region.
-///
-/// As an example of different regions separated by preprocessor directives:
-///
-/// \code
-///   #1
-/// #ifdef BLAH
-///   #2
-/// #ifdef CAKE
-///   #3
-/// #endif
-///   #2
-/// #endif
-///   #1
-/// \endcode
-///
-/// There are 3 regions, with non-consecutive parts:
-///   #1 is identified as the beginning of the file
-///   #2 is identified as the location of "#ifdef BLAH"
-///   #3 is identified as the location of "#ifdef CAKE"
-///
-class PPRegion {
-  llvm::sys::fs::UniqueID UniqueID;
-  time_t ModTime;
-  unsigned Offset;
-public:
-  PPRegion() : UniqueID(0, 0), ModTime(), Offset() {}
-  PPRegion(llvm::sys::fs::UniqueID UniqueID, unsigned offset, time_t modTime)
-      : UniqueID(UniqueID), ModTime(modTime), Offset(offset) {}
-
-  const llvm::sys::fs::UniqueID &getUniqueID() const { return UniqueID; }
-  unsigned getOffset() const { return Offset; }
-  time_t getModTime() const { return ModTime; }
-
-  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);
-    }
-    static inline PPRegion getTombstoneKey() {
-      return PPRegion(llvm::sys::fs::UniqueID(0, 0), unsigned(-2), 0);
-    }
-
-    static unsigned getHashValue(const PPRegion &S) {
-      llvm::FoldingSetNodeID ID;
-      const llvm::sys::fs::UniqueID &UniqueID = S.getUniqueID();
-      ID.AddInteger(UniqueID.getFile());
-      ID.AddInteger(UniqueID.getDevice());
-      ID.AddInteger(S.getOffset());
-      ID.AddInteger(S.getModTime());
-      return ID.ComputeHash();
-    }
-
-    static bool isEqual(const PPRegion &LHS, const PPRegion &RHS) {
-      return LHS == RHS;
-    }
-  };
-}
-
-namespace {
-
-/// Keeps track of function bodies that have already been parsed.
-///
-/// Is thread-safe.
-class SharedParsedRegionsStorage {
-  std::mutex Mux;
-  PPRegionSetTy ParsedRegions;
-
-public:
-  ~SharedParsedRegionsStorage() = default;
-
-  void copyTo(PPRegionSetTy &Set) {
-    std::lock_guard<std::mutex> MG(Mux);
-    Set = ParsedRegions;
-  }
-
-  void merge(ArrayRef<PPRegion> Regions) {
-    std::lock_guard<std::mutex> MG(Mux);
-    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;
-  PPConditionalDirectiveRecord &PPRec;
-  Preprocessor &PP;
-
-  /// Snapshot of the shared state at the point when this instance was
-  /// constructed.
-  PPRegionSetTy ParsedRegions;
-  /// 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,
-                            PPConditionalDirectiveRecord &ppRec,
-                            Preprocessor &pp)
-      : ParsedRegionsStorage(ParsedRegionsStorage), PPRec(ppRec), PP(pp) {
-    ParsedRegionsStorage.copyTo(ParsedRegions);
-  }
-
-  /// \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);
-    if (!LastIsParsed)
-      NewParsedRegions.emplace_back(std::move(region));
-    return LastIsParsed;
-  }
-
-  /// Updates ParsedRegionsStorage with newly parsed regions.
-  void syncWithStorage() { ParsedRegionsStorage.merge(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());
-      }
-      return PPRegion();
-    };
-
-    SourceLocation RegionLoc = PPRec.findConditionalDirectiveRegionLoc(Loc);
-    assert(RegionLoc.isFileID());
-    if (RegionLoc.isInvalid())
-      return Bail();
-
-    FileID RegionFID;
-    unsigned RegionOffset;
-    std::tie(RegionFID, RegionOffset) =
-        PPRec.getSourceManager().getDecomposedLoc(RegionLoc);
-
-    if (RegionFID != FID)
-      return Bail();
-
-    const llvm::sys::fs::UniqueID &ID = FE->getUniqueID();
-    return PPRegion(ID, RegionOffset, FE->getModificationTime());
-  }
-
-  bool isParsedOnceInclude(const FileEntry *FE) {
-    return PP.getHeaderSearchInfo().isFileMultipleIncludeGuarded(FE);
-  }
-};
-
-//===----------------------------------------------------------------------===//
 // IndexPPCallbacks
 //===----------------------------------------------------------------------===//
 
 class IndexPPCallbacks : public PPCallbacks {
   Preprocessor &PP;
   CXIndexDataConsumer &DataConsumer;
   bool IsMainFileEntered;
 

 };
 
 //===----------------------------------------------------------------------===//
 // IndexingFrontendAction
 //===----------------------------------------------------------------------===//
 
 class IndexingFrontendAction : public ASTFrontendAction {
   std::shared_ptr<CXIndexDataConsumer> DataConsumer;
-
-  SharedParsedRegionsStorage *SKData;
-  std::unique_ptr<ParsedSrcLocationsTracker> ParsedLocsTracker;
+  ThreadSafeParsedRegions *SKData;
 
 public:
   IndexingFrontendAction(std::shared_ptr<CXIndexDataConsumer> dataConsumer,
-                         SharedParsedRegionsStorage *skData)
+                         ThreadSafeParsedRegions *skData)
       : DataConsumer(std::move(dataConsumer)), 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 (File)
         DataConsumer->importedPCH(*File);
     }
 
     DataConsumer->setASTContext(CI.getASTContext());
     Preprocessor &PP = CI.getPreprocessor();
     PP.addPPCallbacks(std::make_unique<IndexPPCallbacks>(PP, *DataConsumer));
     DataConsumer->setPreprocessor(CI.getPreprocessorPtr());
 
+    std::unique_ptr<ParsedSrcLocationsTracker> ParsedLocsTracker;
     if (SKData) {
       auto *PPRec = new PPConditionalDirectiveRecord(PP.getSourceManager());
       PP.addPPCallbacks(std::unique_ptr<PPCallbacks>(PPRec));
-      ParsedLocsTracker =
-          std::make_unique<ParsedSrcLocationsTracker>(*SKData, *PPRec, PP);
+      ParsedLocsTracker = std::make_unique<ParsedSrcLocationsTracker>(*SKData, *PPRec, PP);
     }
 
     return std::make_unique<IndexingConsumer>(*DataConsumer,
-                                              ParsedLocsTracker.get());
+                                              ParsedLocsTracker.release());
   }
 
   TranslationUnitKind getTranslationUnitKind() override {
     if (DataConsumer->shouldIndexImplicitTemplateInsts())
       return TU_Complete;
     else
       return TU_Prefix;
   }
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;
 
   explicit IndexSessionData(CXIndex cIdx)
-      : CIdx(cIdx), SkipBodyData(new SharedParsedRegionsStorage) {}
+      : CIdx(cIdx), SkipBodyData(new ThreadSafeParsedRegions) {}
 };
 
 } // 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,