[clang-tools-extra] r366337 - [clang-tidy] Adjust location of namespace comment diagnostic

Ilya Biryukov via cfe-commits cfe-commits at lists.llvm.org
Wed Jul 17 08:22:15 PDT 2019


Author: ibiryukov
Date: Wed Jul 17 08:22:14 2019
New Revision: 366337

URL: http://llvm.org/viewvc/llvm-project?rev=366337&view=rev
Log:
[clang-tidy] Adjust location of namespace comment diagnostic

Summary:
If there is no comment, place it at the closing brace of a namespace
definition. Previously it was placed at the next character after the
closing brace.

The new position produces a better location for highlighting in clangd
and does not seem to make matters worse for clang-tidy.

Reviewers: alexfh, hokein

Reviewed By: alexfh, hokein

Subscribers: xazax.hun, kadircet, cfe-commits

Tags: #clang

Differential Revision: https://reviews.llvm.org/D64861

Modified:
    clang-tools-extra/trunk/clang-tidy/readability/NamespaceCommentCheck.cpp
    clang-tools-extra/trunk/test/clang-tidy/google-readability-namespace-comments.cpp
    clang-tools-extra/trunk/test/clang-tidy/google-readability-nested-namespace-comments.cpp
    clang-tools-extra/trunk/test/clang-tidy/select-checks.cpp

Modified: clang-tools-extra/trunk/clang-tidy/readability/NamespaceCommentCheck.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/readability/NamespaceCommentCheck.cpp?rev=366337&r1=366336&r2=366337&view=diff
==============================================================================
--- clang-tools-extra/trunk/clang-tidy/readability/NamespaceCommentCheck.cpp (original)
+++ clang-tools-extra/trunk/clang-tidy/readability/NamespaceCommentCheck.cpp Wed Jul 17 08:22:14 2019
@@ -9,6 +9,7 @@
 #include "NamespaceCommentCheck.h"
 #include "clang/AST/ASTContext.h"
 #include "clang/ASTMatchers/ASTMatchers.h"
+#include "clang/Basic/SourceLocation.h"
 #include "clang/Lex/Lexer.h"
 #include "llvm/ADT/StringExtras.h"
 
@@ -181,7 +182,13 @@ void NamespaceCommentCheck::check(const
           ? "anonymous namespace"
           : ("namespace '" + NestedNamespaceName.str() + "'");
 
-  diag(AfterRBrace, Message)
+  // Place diagnostic at an old comment, or closing brace if we did not have it.
+  SourceLocation DiagLoc =
+      OldCommentRange.getBegin() != OldCommentRange.getEnd()
+          ? OldCommentRange.getBegin()
+          : ND->getRBraceLoc();
+
+  diag(DiagLoc, Message)
       << NamespaceName
       << FixItHint::CreateReplacement(
              CharSourceRange::getCharRange(OldCommentRange),

Modified: clang-tools-extra/trunk/test/clang-tidy/google-readability-namespace-comments.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-tidy/google-readability-namespace-comments.cpp?rev=366337&r1=366336&r2=366337&view=diff
==============================================================================
--- clang-tools-extra/trunk/test/clang-tidy/google-readability-namespace-comments.cpp (original)
+++ clang-tools-extra/trunk/test/clang-tidy/google-readability-namespace-comments.cpp Wed Jul 17 08:22:14 2019
@@ -7,9 +7,9 @@ namespace n2 {
 void f(); // So that the namespace isn't empty.
 
 
-// CHECK-MESSAGES: :[[@LINE+4]]:2: warning: namespace 'n2' not terminated with a closing comment [google-readability-namespace-comments]
+// CHECK-MESSAGES: :[[@LINE+4]]:1: warning: namespace 'n2' not terminated with a closing comment [google-readability-namespace-comments]
 // CHECK-MESSAGES: :[[@LINE-7]]:11: note: namespace 'n2' starts here
-// CHECK-MESSAGES: :[[@LINE+2]]:3: warning: namespace 'n1' not terminated with
+// CHECK-MESSAGES: :[[@LINE+2]]:2: warning: namespace 'n1' not terminated with
 // CHECK-MESSAGES: :[[@LINE-10]]:11: note: namespace 'n1' starts here
 }}
 // CHECK-FIXES: }  // namespace n2
@@ -25,7 +25,7 @@ void f(); // So that the namespace isn't
 // 5
 // 6
 // 7
-// CHECK-MESSAGES: :[[@LINE+2]]:2: warning: namespace 'macro_expansion' not terminated with
+// CHECK-MESSAGES: :[[@LINE+2]]:1: warning: namespace 'macro_expansion' not terminated with
 // CHECK-MESSAGES: :[[@LINE-10]]:11: note: namespace 'macro_expansion' starts here
 }
 // CHECK-FIXES: }  // namespace macro_expansion

Modified: clang-tools-extra/trunk/test/clang-tidy/google-readability-nested-namespace-comments.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-tidy/google-readability-nested-namespace-comments.cpp?rev=366337&r1=366336&r2=366337&view=diff
==============================================================================
--- clang-tools-extra/trunk/test/clang-tidy/google-readability-nested-namespace-comments.cpp (original)
+++ clang-tools-extra/trunk/test/clang-tidy/google-readability-nested-namespace-comments.cpp Wed Jul 17 08:22:14 2019
@@ -7,9 +7,9 @@ namespace n3 {
 void f();
 
 
-// CHECK-MESSAGES: :[[@LINE+4]]:2: warning: namespace 'n3' not terminated with
+// CHECK-MESSAGES: :[[@LINE+4]]:1: warning: namespace 'n3' not terminated with
 // CHECK-MESSAGES: :[[@LINE-7]]:11: note: namespace 'n3' starts here
-// CHECK-MESSAGES: :[[@LINE+2]]:3: warning: namespace 'n1::n2' not terminated with a closing comment [google-readability-namespace-comments]
+// CHECK-MESSAGES: :[[@LINE+2]]:2: warning: namespace 'n1::n2' not terminated with a closing comment [google-readability-namespace-comments]
 // CHECK-MESSAGES: :[[@LINE-10]]:11: note: namespace 'n1::n2' starts here
 }}
 // CHECK-FIXES: }  // namespace n3

Modified: clang-tools-extra/trunk/test/clang-tidy/select-checks.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-tidy/select-checks.cpp?rev=366337&r1=366336&r2=366337&view=diff
==============================================================================
--- clang-tools-extra/trunk/test/clang-tidy/select-checks.cpp (original)
+++ clang-tools-extra/trunk/test/clang-tidy/select-checks.cpp Wed Jul 17 08:22:14 2019
@@ -5,7 +5,7 @@
 
 namespace i {
 }
-// CHECK: :[[@LINE-1]]:2: warning: namespace 'i' not terminated with a closing comment [llvm-namespace-comment]
+// CHECK: :[[@LINE-1]]:1: warning: namespace 'i' not terminated with a closing comment [llvm-namespace-comment]
 
 // Expect no warnings from the google-explicit-constructor check:
 class A { A(int i); };




More information about the cfe-commits mailing list