[PATCH] D66694: [libclang][index][NFCi] Refactor machinery for skipping already parsed function bodies

[Dmitri Gribenko via Phabricator via cfe-commits]

gribozavr added inline comments.


================
Comment at: clang/tools/libclang/Indexing.cpp:126
+/// Is thread-safe.
+class SharedParsedRegionsStorage {
   std::mutex Mux;
----------------
"SharedParsedRegions"? "ThreadSafeParsedRegions"?


================
Comment at: clang/tools/libclang/Indexing.cpp:134
   void copyTo(PPRegionSetTy &Set) {
     std::lock_guard<std::mutex> MG(Mux);
     Set = ParsedRegions;
----------------
I think we should lock both the source and destination mutexes (use `std::scoped_lock` that allows to lock multiple mutexes).

Also, it would be more idiomatic to express this API as a copy constructor and an assignment operator.


================
Comment at: clang/tools/libclang/Indexing.cpp:138
 
-  void update(ArrayRef<PPRegion> Regions) {
+  void merge(ArrayRef<PPRegion> Regions) {
     std::lock_guard<std::mutex> MG(Mux);
----------------
"addParsedRegions"?


================
Comment at: clang/tools/libclang/Indexing.cpp:371
 
-  SessionSkipBodyData *SKData;
-  std::unique_ptr<TUSkipBodyControl> SKCtrl;
+  SharedParsedRegionsStorage *SKData;
+  std::unique_ptr<ParsedSrcLocationsTracker> ParsedLocsTracker;
----------------
This pointer seems to be only used in the constructor, however the constructor already has access to `skData` parameter.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D66694/new/

https://reviews.llvm.org/D66694