[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:134
   void copyTo(PPRegionSetTy &Set) {
     std::lock_guard<std::mutex> MG(Mux);
     Set = ParsedRegions;
----------------
jkorous wrote:
> gribozavr wrote:
> > 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.
> Sorry, I am not following. You are aware of the argument being a `DenseSet`, right? This doesn't look like a named copy-ctor to me.
> 
> The way this works is that `ParsedSrcLocationsTracker` takes snapshot of the `SharedParsedRegionsStorage` in its constructor (using `copyTo()`), then uses its own local data and synchronizes the local data with `SharedParsedRegionsStorage` in `syncWithStorage()` method.
> 
> Maybe you just made a case for removing the typedef?
> Sorry, I am not following. You are aware of the argument being a DenseSet, right? This doesn't look like a named copy-ctor to me.

Sorry -- you're right. I confused `PPRegionSetTy` with `SharedParsedRegionsStorage`.

> Maybe you just made a case for removing the typedef?

I think that would be an improvement and happy to do that! PTAL at https://reviews.llvm.org/D67077 .




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D66694