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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
