kadircet added inline comments.
================
Comment at: clang-tools-extra/clangd/IncludeCleaner.cpp:380
+ auto Loc = SM.getFileLoc(Ref.RefLocation);
+ auto Range = Lexer::makeFileCharRange(
+ CharSourceRange{SourceRange{Loc}, true}, SM, AST.getLangOpts());
----------------
VitaNuo wrote:
> kadircet wrote:
> > I don't think you need to re-lex the token here, as `Loc` is already a
> > non-macro location in main file, you can still use
> > `Tokens.spelledTokenAt(Loc)`. Am i missing something?
> It's for consistency with clangd diagnostic generation
> [here](http://google3/third_party/llvm/llvm-project/clang-tools-extra/clangd/Diagnostics.cpp;l=114;rcl=512058308)
> (according to Sam), but both should work.
> Would you rather prefer `Tokens.spelledTokenAt(Loc)`?
I'd rather go with `spelledTokenAt` here, diagnostics code deals with locations
outside of the main file, hence it makes sense for that logic to re-run the
lexer. but i feel like re-running lexer here will actually create more
confusion, as we tend to re-use results in token buffers rather than using raw
lexer in rest of the codebase, as it confused me above.
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
