This is an archive of the discontinued LLVM Phabricator instance.

[Clangd] Fixed clangd diagnostics priority
ClosedPublic

Authored by SureYeaah on Jun 12 2019, 10:27 AM.

Details

Summary
  • Fixed diagnostics where zero width inserted ranges were being used instead of the whole token
  • Added unit tests

Diff Detail

Event Timeline

SureYeaah created this revision.Jun 12 2019, 10:27 AM
Herald added a project: Restricted Project. · View Herald TranscriptJun 12 2019, 10:27 AM
SureYeaah edited the summary of this revision. (Show Details)Jun 12 2019, 10:28 AM
SureYeaah edited projects, added Restricted Project; removed Restricted Project.
Herald added a project: Restricted Project. · View Herald TranscriptJun 12 2019, 10:29 AM
SureYeaah updated this revision to Diff 204351.Jun 12 2019, 1:36 PM

Remove extra newline

Mostly LG, just a few re-orderings to make code more readable and get rid of redundant Lexer calls.

clang-tools-extra/clangd/Diagnostics.cpp
108

since we are going to lex again in case of R.isValid(), you can get rid of this lexer call.

109

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()).

111

LLVM style uses UpperCammelCase for variable names. so this should be something like, Token Tok;, same for the bool below.

119

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.

SureYeaah updated this revision to Diff 204467.Jun 13 2019, 3:13 AM

Refactored code as pointed out by @kadircet

kadircet accepted this revision.Jun 13 2019, 3:35 AM

LGTM, thanks!

Also please make sure you've clang-formatted the code before you land this.

clang-tools-extra/clangd/Diagnostics.cpp
93–94

FallbackRange is not returned anymore, you can safely delete it.

104

I think you can simply it further by:

// Fallback to zero-width range at diagnostic location.
auto R = CharSourceRange::getCharRange(Loc);
Token Tok;
// For non-comment tokens, use token at the location.
if (!Lexer::getRawToken(Loc, Tok, M, L, true) && Tok.isNot(tok::comment))
  R = CharSourceRange::getTokenRange(Tok.getLocation(), Tok.getEndLoc());
return halfOpenToRange(M, R);
This revision is now accepted and ready to land.Jun 13 2019, 3:35 AM
SureYeaah updated this revision to Diff 204489.Jun 13 2019, 4:56 AM
SureYeaah marked 4 inline comments as done.

Simplified logic for diagnostics range.

SureYeaah updated this revision to Diff 204494.Jun 13 2019, 5:03 AM
SureYeaah marked 2 inline comments as done.

Formatted code

This revision was automatically updated to reflect the committed changes.
Herald added a project: Restricted Project. · View Herald TranscriptJun 13 2019, 5:30 AM

Code gets shorter, tests get longer, very nice :-)