[clang-tools-extra] 8fa3975 - [libtooling][clang-tidy] Fix off-by-one rendering issue with SourceRanges
via cfe-commits
cfe-commits at lists.llvm.org
Sat Apr 10 09:53:21 PDT 2021
Author: Whisperity
Date: 2021-04-10T18:52:55+02:00
New Revision: 8fa39752477b225294cde0967a3b4c9c492e699c
URL: https://github.com/llvm/llvm-project/commit/8fa39752477b225294cde0967a3b4c9c492e699c
DIFF: https://github.com/llvm/llvm-project/commit/8fa39752477b225294cde0967a3b4c9c492e699c.diff
LOG: [libtooling][clang-tidy] Fix off-by-one rendering issue with SourceRanges
There was an off-by-one issue with calculating the *exact* end location
of token ranges (as given by SomeDecl->getSourceRange()) which resulted in:
xxx(something)
^~~~~~~~ // Note the missing ~ under the last character.
In addition, a test is added to keep the behaviour in check in the future.
This patch hotfixes commit 3b677b81cec7b3c5132aee8fccc30252d87deb69.
Added:
Modified:
clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
clang-tools-extra/test/clang-tidy/infrastructure/export-diagnostics.cpp
clang-tools-extra/unittests/clang-tidy/ClangTidyDiagnosticConsumerTest.cpp
Removed:
################################################################################
diff --git a/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp b/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
index 0590715562d08..08d5c0d6704ba 100644
--- a/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
+++ b/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
@@ -68,11 +68,12 @@ class ClangTidyDiagnosticRenderer : public DiagnosticRenderer {
// into a real CharRange for the diagnostic printer later.
// Whatever we store here gets decoupled from the current SourceManager, so
// we **have to** know the exact position and length of the highlight.
- const auto &ToCharRange = [this, &Loc](const CharSourceRange &SourceRange) {
+ auto ToCharRange = [this, &Loc](const CharSourceRange &SourceRange) {
if (SourceRange.isCharRange())
return SourceRange;
+ assert(SourceRange.isTokenRange());
SourceLocation End = Lexer::getLocForEndOfToken(
- SourceRange.getEnd(), 1, Loc.getManager(), LangOpts);
+ SourceRange.getEnd(), 0, Loc.getManager(), LangOpts);
return CharSourceRange::getCharRange(SourceRange.getBegin(), End);
};
diff --git a/clang-tools-extra/test/clang-tidy/infrastructure/export-diagnostics.cpp b/clang-tools-extra/test/clang-tidy/infrastructure/export-diagnostics.cpp
index af8272ffcc632..414c7967bc825 100644
--- a/clang-tools-extra/test/clang-tidy/infrastructure/export-diagnostics.cpp
+++ b/clang-tools-extra/test/clang-tidy/infrastructure/export-diagnostics.cpp
@@ -1,10 +1,11 @@
// RUN: grep -Ev "// *[A-Z-]+:" %s > %t-input.cpp
-// RUN: not clang-tidy %t-input.cpp -checks='-*,google-explicit-constructor,clang-diagnostic-missing-prototypes' -export-fixes=%t.yaml -- -Wmissing-prototypes > %t.msg 2>&1
+// RUN: not clang-tidy %t-input.cpp -checks='-*,google-explicit-constructor,clang-diagnostic-missing-prototypes,clang-diagnostic-zero-length-array' -export-fixes=%t.yaml -- -Wmissing-prototypes -Wzero-length-array > %t.msg 2>&1
// RUN: FileCheck -input-file=%t.msg -check-prefix=CHECK-MESSAGES %s -implicit-check-not='{{warning|error|note}}:'
// RUN: FileCheck -input-file=%t.yaml -check-prefix=CHECK-YAML %s
#define X(n) void n ## n() {}
X(f)
int a[-1];
+int b[0];
// CHECK-MESSAGES: -input.cpp:2:1: warning: no previous prototype for function 'ff' [clang-diagnostic-missing-prototypes]
// CHECK-MESSAGES: -input.cpp:1:19: note: expanded from macro 'X'
@@ -12,14 +13,14 @@ int a[-1];
// CHECK-MESSAGES: -input.cpp:2:1: note: declare 'static' if the function is not intended to be used outside of this translation unit
// CHECK-MESSAGES: -input.cpp:1:14: note: expanded from macro 'X'
// CHECK-MESSAGES: -input.cpp:3:7: error: 'a' declared as an array with a negative size [clang-diagnostic-error]
+// CHECK-MESSAGES: -input.cpp:4:7: warning: zero size arrays are an extension [clang-diagnostic-zero-length-array]
// CHECK-YAML: ---
// CHECK-YAML-NEXT: MainSourceFile: '{{.*}}-input.cpp'
// CHECK-YAML-NEXT: Diagnostics:
// CHECK-YAML-NEXT: - DiagnosticName: clang-diagnostic-missing-prototypes
// CHECK-YAML-NEXT: DiagnosticMessage:
-// CHECK-YAML-NEXT: Message: 'no previous prototype for function
-// ''ff'''
+// CHECK-YAML-NEXT: Message: 'no previous prototype for function ''ff'''
// CHECK-YAML-NEXT: FilePath: '{{.*}}-input.cpp'
// CHECK-YAML-NEXT: FileOffset: 30
// CHECK-YAML-NEXT: Replacements: []
@@ -55,7 +56,19 @@ int a[-1];
// CHECK-YAML-NEXT: Ranges:
// CHECK-YAML-NEXT: - FilePath: '{{.*}}-input.cpp'
// CHECK-YAML-NEXT: FileOffset: 41
-// CHECK-YAML-NEXT: Length: 1
+// CHECK-YAML-NEXT: Length: 2
// CHECK-YAML-NEXT: Level: Error
// CHECK-YAML-NEXT: BuildDirectory: '{{.*}}'
+// CHECK-YAML-NEXT: - DiagnosticName: clang-diagnostic-zero-length-array
+// CHECK-YAML-NEXT: DiagnosticMessage:
+// CHECK-YAML-NEXT: Message: zero size arrays are an extension
+// CHECK-YAML-NEXT: FilePath: '{{.*}}-input.cpp'
+// CHECK-YAML-NEXT: FileOffset: 52
+// CHECK-YAML-NEXT: Replacements: []
+// CHECK-YAML-NEXT: Ranges:
+// CHECK-YAML-NEXT: - FilePath: '{{.*}}-input.cpp'
+// CHECK-YAML-NEXT: FileOffset: 52
+// CHECK-YAML-NEXT: Length: 1
+// CHECK-YAML-NEXT: Level: Warning
+// CHECK-YAML-NEXT: BuildDirectory: '{{.*}}'
// CHECK-YAML-NEXT: ...
diff --git a/clang-tools-extra/unittests/clang-tidy/ClangTidyDiagnosticConsumerTest.cpp b/clang-tools-extra/unittests/clang-tidy/ClangTidyDiagnosticConsumerTest.cpp
index fec9fddccd0a5..9760e3ff5af4f 100644
--- a/clang-tools-extra/unittests/clang-tidy/ClangTidyDiagnosticConsumerTest.cpp
+++ b/clang-tools-extra/unittests/clang-tidy/ClangTidyDiagnosticConsumerTest.cpp
@@ -23,6 +23,20 @@ class TestCheck : public ClangTidyCheck {
diag(Var->getTypeSpecStartLoc(), "type specifier");
}
};
+
+class HighlightTestCheck : public ClangTidyCheck {
+public:
+ HighlightTestCheck(StringRef Name, ClangTidyContext *Context)
+ : ClangTidyCheck(Name, Context) {}
+ void registerMatchers(ast_matchers::MatchFinder *Finder) override {
+ Finder->addMatcher(ast_matchers::varDecl().bind("var"), this);
+ }
+ void check(const ast_matchers::MatchFinder::MatchResult &Result) override {
+ const auto *Var = Result.Nodes.getNodeAs<VarDecl>("var");
+ diag(Var->getLocation(), "highlight range") << Var->getSourceRange();
+ }
+};
+
} // namespace
TEST(ClangTidyDiagnosticConsumer, SortsErrors) {
@@ -34,6 +48,24 @@ TEST(ClangTidyDiagnosticConsumer, SortsErrors) {
EXPECT_EQ("variable", Errors[2].Message.Message);
}
+TEST(ClangTidyDiagnosticConsumer, HandlesSourceRangeHighlight) {
+ std::vector<ClangTidyError> Errors;
+ runCheckOnCode<HighlightTestCheck>("int abc;", &Errors);
+ EXPECT_EQ(1ul, Errors.size());
+ EXPECT_EQ("highlight range", Errors[0].Message.Message);
+
+ // int abc;
+ // ____^
+ // 01234
+ EXPECT_EQ(4, Errors[0].Message.FileOffset);
+
+ // int abc
+ // ~~~~~~~ -> Length 7. (0-length highlights are nonsensical.)
+ EXPECT_EQ(1, Errors[0].Message.Ranges.size());
+ EXPECT_EQ(0, Errors[0].Message.Ranges[0].FileOffset);
+ EXPECT_EQ(7, Errors[0].Message.Ranges[0].Length);
+}
+
} // namespace test
} // namespace tidy
} // namespace clang
More information about the cfe-commits
mailing list