VitaNuo marked an inline comment as done.
VitaNuo added a comment.
Thanks for the comments!
================
Comment at: clang-tools-extra/clangd/IncludeCleaner.cpp:382
- auto &Tokens = AST.getTokens();
- auto SpelledForExpanded =
- Tokens.spelledForExpanded(Tokens.expandedTokens(Ref.RefLocation));
- if (!SpelledForExpanded)
+ auto ExpansionLoc = SM.getExpansionLoc(Ref.RefLocation);
+ const auto *Token = AST.getTokens().spelledTokenAt(ExpansionLoc);
----------------
kadircet wrote:
> can you actually get the spelling location first and use expansion location
> only when it isn't inside the main file?
> that way we get to preserve the behaviour for macro arguments, which warrants
> a test like:
> ```
> #define FOO(X) const X x
>
> FOO([[Foo]]); // we should have a missing include for symbol Foo here
> ```
>
> and can we have a comment about reasons and implications, e.g. something like:
> ```
> Spelling locations can point into preamble section at times. We don't want to
> emit diagnostics there as the offsets might be off when preamble is stale.
> In such cases we fallback to expansion locations, as they're always in the
> main file. This means we fail to attach diagnostics to spelling of symbol
> names inside macro bodies.
> // FIXME: Use presumed locations to make sure we can correctly generate
> diagnostics even in such cases.
> ```
Sam suggested a magic solution for this, which is
`SM.getFileLoc(Ref.RefLocation)`. Its documentation says:
```
/// Given \p Loc, if it is a macro location return the expansion
/// location or the spelling location, depending on if it comes from a
/// macro argument or not.
```
Sounds like what we need, right?
================
Comment at: clang-tools-extra/include-cleaner/lib/Analysis.cpp:36
walkAST(*Root, [&](SourceLocation Loc, NamedDecl &ND, RefType RT) {
- if (!SM.isWrittenInMainFile(SM.getSpellingLoc(Loc)))
+ if (!SM.isWrittenInMainFile(SM.getSpellingLoc(Loc)) &&
+ SM.getDecomposedLoc(SM.getSpellingLoc(Loc)).first !=
----------------
kadircet wrote:
> you can directly use expansion location instead of checking both and
> mentioning preamble file id here, eg:
> `!SM.isWrittenInMainFile(SM.getExpansionLoc(Loc))`
This actually leads to both `BAR` and `Foo` in `#define BAR(x) Foo *x` in
`foo.h` being recognized as missing and attaches 2 diagnostics to `[[BAR]](b)`
in the main file. AFAIU, this is exactly what you're asking me to prevent in
the previous comment.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D146727/new/
https://reviews.llvm.org/D146727
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
