sammccall added a comment.
Didn't really address this part
> But I don't think it matters in the long run especially when we want to
> handle different types of pragmas in the future
> So I'd suggest just building a side table information
That's a more general approach for sure. But it does seem there's some
complexity: what is the data structure and how do we merge it back into the
graph. (e.g. are we going to record line numbers of each #include in the graph
for this merging?)
So let's think about the concrete cases:
- export
- begin_exports...end_exports
- private maybe?
The TL;DR is that each of these either stand alone, or we see the directive
before the inclusion it describes in a way that's easy to keep track of
incrementally. So I'm not really seeing much implementation difficulty to
justify trying to move the directives and inclusion processing apart.
Export seems basically similar. We're going to see the pragma immediately
before the directive.
The same stateful solution seems just as clear, I don't see any problem with it
not being in the main file.
(There's the "same line" issue of having to look up line tables for everything,
maybe we can dodge it by comparing SourceLocations in raw form instead, or use
the statefulness to avoid checks in most cases)
Begin/end is a bit different. The stateful solution looks like: keep a
`Set<FileID>` where exports are turned on, mutate it when you see a directive,
and check it when you seen an inclusion.
The side-table solution is a bit more complicated: a map of fileIDs to a list
of ranges or something. Manageable for sure.
Private I think we have to treat as a standalone directive that mutates the
current file, and the reference to another file is treated as a string. It's
not a file the PP has necessarily seen. So the result is a map<file, string>
and it seems adequate for this to be written straight into IncludeStructure.
================
Comment at: clang-tools-extra/clangd/Headers.cpp:119
+ bool HandleComment(Preprocessor &PP, SourceRange Range) override {
+ llvm::StringRef Text =
+ Lexer::getSourceText(CharSourceRange::getCharRange(Range),
----------------
This code will run for every comment in the preamble, so we should probably be
careful about its performance.
I'd guess the cheapest thing is:
- check that Range.begin() is not a macro
- SM.getCharacterData(Range.begin()) yields a `char*` that's guaranteed
null-terminated
- try to consume the `IWYU pragma:` prefix
This doesn't deal with makeFileCharRange, touch End at all, etc.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D114072/new/
https://reviews.llvm.org/D114072
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
