kadircet added a comment.
Mostly LG, just a few re-orderings to make code more readable and get rid of
redundant Lexer calls.
================
Comment at: clang-tools-extra/clangd/Diagnostics.cpp:108
+ // Otherwise use zero width insertion range
auto R = Lexer::makeFileCharRange(CharSourceRange::getTokenRange(Loc), M, L);
+ if (R.isValid()) {
----------------
since we are going to lex again in case of `R.isValid()`, you can get rid of
this lexer call.
================
Comment at: clang-tools-extra/clangd/Diagnostics.cpp:109
auto R = Lexer::makeFileCharRange(CharSourceRange::getTokenRange(Loc), M, L);
- if (!R.isValid()) // Fall back to location only, let the editor deal with it.
+ if (R.isValid()) {
+ // check if token at location is a priority i.e. not a comment
----------------
let's change this condition to rather check for `Lexer::getRawToken` succeeded
and underlying token `isNot(tok::comment)`.
Then you can simply construct the range with
`CharSourceRange::getTokenRange(Tok.getLocation(), Tok.getEndLoc())`.
================
Comment at: clang-tools-extra/clangd/Diagnostics.cpp:111
+ // check if token at location is a priority i.e. not a comment
+ Token token;
+ bool isTokenPriority = false;
----------------
LLVM style uses `UpperCammelCase` for variable names. so this should be
something like, `Token Tok;`, same for the bool below.
================
Comment at: clang-tools-extra/clangd/Diagnostics.cpp:119
+ else if (FallbackRange)
+ return *FallbackRange;
+ else // Fall back to location only, let the editor deal with it.
----------------
the FallbackRange(if set) and `CharSourceRange::getCharRange(Loc);` are the
same.
So you can get rid of `FallbackRange` and merge the last two branches of this
conditional.
Repository:
rCTE Clang Tools Extra
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D63222/new/
https://reviews.llvm.org/D63222
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
