dexonsmith requested changes to this revision.
dexonsmith added a comment.
This revision now requires changes to proceed.
The code looks mostly good; some inline comments.
I think it'd be useful to have direct tests for "detecting used search paths".
That might be a nice thing to separate out and test independently. E.g., you
could add a remark, `-Rused-search-path`, which printed a diagnostic the first
time any given search path is used in a TU.
It'd also be really nice to be able to dump this info from the `.pcm` file. I
guess it'd be outside the scope of this patch, but I really think there should
be a clang-pcm-analyzer where we can put functionality like that. Somewhat
useful for more fine-grained testing; super useful as a tool for debugging.
================
Comment at: clang/include/clang/Lex/HeaderSearch.h:295
+ /// Return indices of used search paths.
+ std::set<unsigned> GetUsedSearchPathIdxs() const {
+ std::set<unsigned> UsedSearchPathIdxs;
----------------
I think the word "get" here is a bit misleading, since this isn't a simple
accessor, it's a potentially expensive computation. I suggest "compute", unless
it's changed to be computed on-the-fly.
I wonder, would llvm::BitVector be more appropriate here, since we expect it to
count from 0 and be relatively dense?
================
Comment at: clang/lib/Serialization/ASTReader.cpp:4798-4806
+ case USED_HEADER_SEARCH_PATHS:
+ if (F) {
+ unsigned Idx = 0;
+ unsigned Count = Record[Idx++];
+ for (unsigned I = 0; I < Count; ++I)
+ F->UsedHeaderSearchPathIdxs.insert(Record[Idx++]);
+ }
----------------
If this were stored as a blob it could be lazily parsed; WDYT?
Alternatively, it might be more compact if stored as a bit vector, storing
64-bits positions per element of the record.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D102488/new/
https://reviews.llvm.org/D102488
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
