steakhal created this revision.
steakhal added reviewers: aaron.ballman, njames93, klimek, alexfh.
Herald added subscribers: carlosgalvezp, martong, xazax.hun.
steakhal requested review of this revision.
Herald added a project: clang-tools-extra.
Herald added a subscriber: cfe-commits.
Invalid SourceRanges can occur generally if the code does not compile,
thus we expect clang error diagnostics.
Unlike `clang`, `clang-tidy` did not swallow invalid source ranges, but
tried to highlight them, and blow various assertions.
The following two examples produce invalid source ranges, but this is
not a complete list:
void test(x); // error: unknown type name 'x'
struct Foo {
member; // error: C++ requires a type specifier for all declarations
};
Thanks @Whisperity helping me fix this.
https://reviews.llvm.org/D114254
Files:
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
Index: clang-tools-extra/unittests/clang-tidy/ClangTidyDiagnosticConsumerTest.cpp
===================================================================
--- clang-tools-extra/unittests/clang-tidy/ClangTidyDiagnosticConsumerTest.cpp
+++ clang-tools-extra/unittests/clang-tidy/ClangTidyDiagnosticConsumerTest.cpp
@@ -37,6 +37,33 @@
}
};
+class InvalidRangeTestCheck : public ClangTidyCheck {
+public:
+ InvalidRangeTestCheck(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");
+ SourceLocation ValidBeginLoc = Var->getBeginLoc();
+ SourceLocation ValidEndLoc = Var->getEndLoc();
+ SourceLocation InvalidLoc;
+ ASSERT_TRUE(ValidBeginLoc.isValid());
+ ASSERT_TRUE(ValidEndLoc.isValid());
+ ASSERT_TRUE(InvalidLoc.isInvalid());
+
+ diag(ValidBeginLoc, "valid->valid")
+ << SourceRange(ValidBeginLoc, ValidEndLoc);
+ diag(ValidBeginLoc, "valid->invalid")
+ << SourceRange(ValidBeginLoc, InvalidLoc);
+ diag(ValidBeginLoc, "invalid->valid")
+ << SourceRange(InvalidLoc, ValidEndLoc);
+ diag(ValidBeginLoc, "invalid->invalid")
+ << SourceRange(InvalidLoc, InvalidLoc);
+ }
+};
+
} // namespace
TEST(ClangTidyDiagnosticConsumer, SortsErrors) {
@@ -66,6 +93,24 @@
EXPECT_EQ(7ul, Errors[0].Message.Ranges[0].Length);
}
+TEST(ClangTidyDiagnosticConsumer, InvalidSourceLocationRangesIgnored) {
+ std::vector<ClangTidyError> Errors;
+ runCheckOnCode<InvalidRangeTestCheck>("int x;", &Errors);
+ EXPECT_EQ(4ul, Errors.size());
+
+ EXPECT_EQ("invalid->invalid", Errors[0].Message.Message);
+ EXPECT_EQ(0ul, Errors[0].Message.Ranges.size());
+
+ EXPECT_EQ("invalid->valid", Errors[1].Message.Message);
+ EXPECT_EQ(0ul, Errors[1].Message.Ranges.size());
+
+ EXPECT_EQ("valid->invalid", Errors[2].Message.Message);
+ EXPECT_EQ(0ul, Errors[2].Message.Ranges.size());
+
+ EXPECT_EQ("valid->valid", Errors[3].Message.Message);
+ EXPECT_EQ(1ul, Errors[3].Message.Ranges.size());
+}
+
} // namespace test
} // namespace tidy
} // namespace clang
Index: clang-tools-extra/test/clang-tidy/infrastructure/export-diagnostics.cpp
===================================================================
--- clang-tools-extra/test/clang-tidy/infrastructure/export-diagnostics.cpp
+++ clang-tools-extra/test/clang-tidy/infrastructure/export-diagnostics.cpp
@@ -7,6 +7,11 @@
int a[-1];
int b[0];
+void test(x);
+struct Foo {
+ member;
+};
+
// 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'
// CHECK-MESSAGES: {{^}}note: expanded from here{{$}}
@@ -14,6 +19,8 @@
// 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-MESSAGES: -input.cpp:6:11: error: unknown type name 'x' [clang-diagnostic-error]
+// CHECK-MESSAGES: -input.cpp:8:3: error: C++ requires a type specifier for all declarations [clang-diagnostic-error]
// CHECK-YAML: ---
// CHECK-YAML-NEXT: MainSourceFile: '{{.*}}-input.cpp'
@@ -71,4 +78,20 @@
// CHECK-YAML-NEXT: Length: 1
// CHECK-YAML-NEXT: Level: Warning
// CHECK-YAML-NEXT: BuildDirectory: '{{.*}}'
+// CHECK-YAML-NEXT: - DiagnosticName: clang-diagnostic-error
+// CHECK-YAML-NEXT: DiagnosticMessage:
+// CHECK-YAML-NEXT: Message: 'unknown type name ''x'''
+// CHECK-YAML-NEXT: FilePath: '{{.*}}-input.cpp'
+// CHECK-YAML-NEXT: FileOffset: 67
+// CHECK-YAML-NEXT: Replacements: []
+// CHECK-YAML-NEXT: Level: Error
+// CHECK-YAML-NEXT: BuildDirectory: '{{.*}}'
+// CHECK-YAML-NEXT: - DiagnosticName: clang-diagnostic-error
+// CHECK-YAML-NEXT: DiagnosticMessage:
+// CHECK-YAML-NEXT: Message: 'C++ requires a type specifier for all declarations'
+// CHECK-YAML-NEXT: FilePath: '{{.*}}-input.cpp'
+// CHECK-YAML-NEXT: FileOffset: 86
+// CHECK-YAML-NEXT: Replacements: []
+// CHECK-YAML-NEXT: Level: Error
+// CHECK-YAML-NEXT: BuildDirectory: '{{.*}}'
// CHECK-YAML-NEXT: ...
Index: clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
===================================================================
--- clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
+++ clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
@@ -79,16 +79,22 @@
return CharSourceRange::getCharRange(SourceRange.getBegin(), End);
};
+ // We are only interested in valid ranges.
+ auto ValidRanges =
+ llvm::make_filter_range(Ranges, [](const CharSourceRange &R) {
+ return R.getAsRange().isValid();
+ });
+
if (Level == DiagnosticsEngine::Note) {
Error.Notes.push_back(TidyMessage);
- for (const CharSourceRange &SourceRange : Ranges)
+ for (const CharSourceRange &SourceRange : ValidRanges)
Error.Notes.back().Ranges.emplace_back(Loc.getManager(),
ToCharRange(SourceRange));
return;
}
assert(Error.Message.Message.empty() && "Overwriting a diagnostic message");
Error.Message = TidyMessage;
- for (const CharSourceRange &SourceRange : Ranges)
+ for (const CharSourceRange &SourceRange : ValidRanges)
Error.Message.Ranges.emplace_back(Loc.getManager(),
ToCharRange(SourceRange));
}
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
