[PATCH] D52544: Improve diagnostics range reporting.

[Kadir Cetinkaya via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL343197: Improve diagnostics range reporting. (authored by kadircet, committed by ).
Herald added a subscriber: llvm-commits.

Repository:
  rL LLVM

https://reviews.llvm.org/D52544

Files:
  clang-tools-extra/trunk/clangd/Diagnostics.cpp
  clang-tools-extra/trunk/unittests/clangd/ClangdUnitTests.cpp


Index: clang-tools-extra/trunk/unittests/clangd/ClangdUnitTests.cpp
===================================================================
--- clang-tools-extra/trunk/unittests/clangd/ClangdUnitTests.cpp
+++ clang-tools-extra/trunk/unittests/clangd/ClangdUnitTests.cpp
@@ -79,8 +79,9 @@
     int main() {
       $typo[[go\
 o]]();
-      foo()$semicolon[[]]
+      foo()$semicolon[[]]//with comments
       $unk[[unknown]]();
+      double bar = $type[["foo"]];
     }
   )cpp");
   EXPECT_THAT(
@@ -93,11 +94,16 @@
                     Fix(Test.range("typo"), "foo", "change 'go\\ o' to 'foo'")),
                 // This is a pretty normal range.
                 WithNote(Diag(Test.range("decl"), "'foo' declared here"))),
-          // This range is zero-width, and at the end of a line.
+          // This range is zero-width and insertion. Therefore make sure we are
+          // not expanding it into other tokens. Since we are not going to
+          // replace those.
           AllOf(Diag(Test.range("semicolon"), "expected ';' after expression"),
                 WithFix(Fix(Test.range("semicolon"), ";", "insert ';'"))),
           // This range isn't provided by clang, we expand to the token.
-          Diag(Test.range("unk"), "use of undeclared identifier 'unknown'")));
+          Diag(Test.range("unk"), "use of undeclared identifier 'unknown'"),
+          Diag(Test.range("type"),
+               "cannot initialize a variable of type 'double' with an lvalue "
+               "of type 'const char [4]'")));
 }
 
 TEST(DiagnosticsTest, FlagsMatter) {
Index: clang-tools-extra/trunk/clangd/Diagnostics.cpp
===================================================================
--- clang-tools-extra/trunk/clangd/Diagnostics.cpp
+++ clang-tools-extra/trunk/clangd/Diagnostics.cpp
@@ -52,17 +52,28 @@
   auto &M = D.getSourceManager();
   auto Loc = M.getFileLoc(D.getLocation());
   // Accept the first range that contains the location.
+  llvm::Optional<Range> FallbackRange;
   for (const auto &CR : D.getRanges()) {
     auto R = Lexer::makeFileCharRange(CR, M, L);
     if (locationInRange(Loc, R, M))
       return halfOpenToRange(M, R);
+    // If there are no ranges that contain the location report the first range.
+    if (!FallbackRange)
+      FallbackRange = halfOpenToRange(M, R);
   }
   // The range may be given as a fixit hint instead.
   for (const auto &F : D.getFixItHints()) {
     auto R = Lexer::makeFileCharRange(F.RemoveRange, M, L);
     if (locationInRange(Loc, R, M))
       return halfOpenToRange(M, R);
+    // If there's a fixit that performs insertion, it has zero-width. Therefore
+    // it can't contain the location of the diag, but it might be possible that
+    // this should be reported as range. For example missing semicolon.
+    if (!FallbackRange && R.getBegin() == R.getEnd())
+      FallbackRange = halfOpenToRange(M, R);
   }
+  if (FallbackRange)
+    return *FallbackRange;
   // If no suitable range is found, just use the token at the location.
   auto R = Lexer::makeFileCharRange(CharSourceRange::getTokenRange(Loc), M, L);
   if (!R.isValid()) // Fall back to location only, let the editor deal with it.


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D52544.167288.patch
Type: text/x-patch
Size: 3174 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20180927/02fa986a/attachment-0001.bin>