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