dexonsmith accepted this revision. dexonsmith added a comment. This revision is now accepted and ready to land.
LGTM, with another comment inline (up to you whether to do something with it). ================ Comment at: clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp:92 + // List of module files to be processed. + llvm::SmallVector<std::string> Worklist{PrebuiltModuleFilename.str()}; + PrebuiltModuleListener Listener(ModuleFiles, InputFiles, VisitInputFiles, ---------------- These filenames will usually be longer than `std::string`'s small storage, requiring separate heap allocations for each filename. Can the worklist just point at the stable pointer `StringMapEntry<std::string>*`, where a filename copy and heap allocation has already been made? (This has also has a nice side effect of handling 3x as many worklist items before needing to allocate, since the element size goes from 24B to 8B.) ================ Comment at: clang/test/ClangScanDeps/modules-pch-dangling.c:11 +// RUN: rm -rf %t +// RUN: split-file %s %t + ---------------- `split-file` makes this so-called "unwieldy" testcase amazingly easy to read! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D121533/new/ https://reviews.llvm.org/D121533 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits