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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
