[PATCH] D146727: [clangd] Use expansion location for missing include diagnostics.

[Kadir Cetinkaya via Phabricator via cfe-commits]

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