nridge updated this revision to Diff 263361.
nridge added a comment.
Address review comment
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D79691/new/
https://reviews.llvm.org/D79691
Files:
clang-tools-extra/clangd/ParsedAST.cpp
clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
Index: clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
===================================================================
--- clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
+++ clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
@@ -374,23 +374,17 @@
Fix(Source.range(), "ident", "change 'ide\\…' to
'ident'"))));
}
-TEST(DiagnosticTest, ClangTidyWarningAsErrorTrumpsSuppressionComment) {
+TEST(DiagnosticTest, ClangTidySuppressionCommentTrumpsWarningAsError) {
Annotations Main(R"cpp(
int main() {
int i = 3;
- double f = [[8]] / i; // NOLINT // error-ok
+ double f = [[8]] / i; // NOLINT
}
)cpp");
TestTU TU = TestTU::withCode(Main.code());
TU.ClangTidyChecks = "bugprone-integer-division";
TU.ClangTidyWarningsAsErrors = "bugprone-integer-division";
- EXPECT_THAT(
- TU.build().getDiagnostics(),
- UnorderedElementsAre(::testing::AllOf(
- Diag(Main.range(), "result of integer division used in a floating "
- "point context; possible loss of precision"),
- DiagSource(Diag::ClangTidy), DiagName("bugprone-integer-division"),
- DiagSeverity(DiagnosticsEngine::Error))));
+ EXPECT_THAT(TU.build().getDiagnostics(), UnorderedElementsAre());
}
TEST(DiagnosticsTest, Preprocessor) {
Index: clang-tools-extra/clangd/ParsedAST.cpp
===================================================================
--- clang-tools-extra/clangd/ParsedAST.cpp
+++ clang-tools-extra/clangd/ParsedAST.cpp
@@ -314,18 +314,12 @@
std::string CheckName = CTContext->getCheckName(Info.getID());
bool IsClangTidyDiag = !CheckName.empty();
if (IsClangTidyDiag) {
- // Check for warning-as-error.
- // We deliberately let this take precedence over suppression comments
- // to match clang-tidy's behaviour.
- if (DiagLevel == DiagnosticsEngine::Warning &&
- CTContext->treatAsError(CheckName)) {
- return DiagnosticsEngine::Error;
- }
-
// Check for suppression comment. Skip the check for diagnostics not
// in the main file, because we don't want that function to query the
// source buffer for preamble files. For the same reason, we ask
// shouldSuppressDiagnostic to avoid I/O.
+ // We let suppression comments take precedence over warning-as-error
+ // to match clang-tidy's behaviour.
bool IsInsideMainFile =
Info.hasSourceManager() &&
isInsideMainFile(Info.getLocation(), Info.getSourceManager());
@@ -334,6 +328,12 @@
/*AllowIO=*/false)) {
return DiagnosticsEngine::Ignored;
}
+
+ // Check for warning-as-error.
+ if (DiagLevel == DiagnosticsEngine::Warning &&
+ CTContext->treatAsError(CheckName)) {
+ return DiagnosticsEngine::Error;
+ }
}
}
return DiagLevel;
Index: clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
===================================================================
--- clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
+++ clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
@@ -374,23 +374,17 @@
Fix(Source.range(), "ident", "change 'ide\\…' to 'ident'"))));
}
-TEST(DiagnosticTest, ClangTidyWarningAsErrorTrumpsSuppressionComment) {
+TEST(DiagnosticTest, ClangTidySuppressionCommentTrumpsWarningAsError) {
Annotations Main(R"cpp(
int main() {
int i = 3;
- double f = [[8]] / i; // NOLINT // error-ok
+ double f = [[8]] / i; // NOLINT
}
)cpp");
TestTU TU = TestTU::withCode(Main.code());
TU.ClangTidyChecks = "bugprone-integer-division";
TU.ClangTidyWarningsAsErrors = "bugprone-integer-division";
- EXPECT_THAT(
- TU.build().getDiagnostics(),
- UnorderedElementsAre(::testing::AllOf(
- Diag(Main.range(), "result of integer division used in a floating "
- "point context; possible loss of precision"),
- DiagSource(Diag::ClangTidy), DiagName("bugprone-integer-division"),
- DiagSeverity(DiagnosticsEngine::Error))));
+ EXPECT_THAT(TU.build().getDiagnostics(), UnorderedElementsAre());
}
TEST(DiagnosticsTest, Preprocessor) {
Index: clang-tools-extra/clangd/ParsedAST.cpp
===================================================================
--- clang-tools-extra/clangd/ParsedAST.cpp
+++ clang-tools-extra/clangd/ParsedAST.cpp
@@ -314,18 +314,12 @@
std::string CheckName = CTContext->getCheckName(Info.getID());
bool IsClangTidyDiag = !CheckName.empty();
if (IsClangTidyDiag) {
- // Check for warning-as-error.
- // We deliberately let this take precedence over suppression comments
- // to match clang-tidy's behaviour.
- if (DiagLevel == DiagnosticsEngine::Warning &&
- CTContext->treatAsError(CheckName)) {
- return DiagnosticsEngine::Error;
- }
-
// Check for suppression comment. Skip the check for diagnostics not
// in the main file, because we don't want that function to query the
// source buffer for preamble files. For the same reason, we ask
// shouldSuppressDiagnostic to avoid I/O.
+ // We let suppression comments take precedence over warning-as-error
+ // to match clang-tidy's behaviour.
bool IsInsideMainFile =
Info.hasSourceManager() &&
isInsideMainFile(Info.getLocation(), Info.getSourceManager());
@@ -334,6 +328,12 @@
/*AllowIO=*/false)) {
return DiagnosticsEngine::Ignored;
}
+
+ // Check for warning-as-error.
+ if (DiagLevel == DiagnosticsEngine::Warning &&
+ CTContext->treatAsError(CheckName)) {
+ return DiagnosticsEngine::Error;
+ }
}
}
return DiagLevel;
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
