Author: Nathan Ridge Date: 2020-05-12T02:29:03-04:00 New Revision: 5a7276b3548589590b81975d5c3e487dd91ac097
URL: https://github.com/llvm/llvm-project/commit/5a7276b3548589590b81975d5c3e487dd91ac097 DIFF: https://github.com/llvm/llvm-project/commit/5a7276b3548589590b81975d5c3e487dd91ac097.diff LOG: [clangd] Have suppression comments take precedence over warning-as-error Summary: This matches the clang-tidy behaviour. Fixes https://github.com/clangd/clangd/issues/375 Subscribers: ilya-biryukov, MaskRay, jkorous, arphaman, kadircet, usaxena95, cfe-commits Tags: #clang Differential Revision: https://reviews.llvm.org/D79691 Added: Modified: clang-tools-extra/clangd/ParsedAST.cpp clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp Removed: ################################################################################ diff --git a/clang-tools-extra/clangd/ParsedAST.cpp b/clang-tools-extra/clangd/ParsedAST.cpp index 61221aaa1491..e63f105b1b6c 100644 --- a/clang-tools-extra/clangd/ParsedAST.cpp +++ b/clang-tools-extra/clangd/ParsedAST.cpp @@ -314,18 +314,12 @@ ParsedAST::build(llvm::StringRef Filename, const ParseInputs &Inputs, 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 @@ ParsedAST::build(llvm::StringRef Filename, const ParseInputs &Inputs, /*AllowIO=*/false)) { return DiagnosticsEngine::Ignored; } + + // Check for warning-as-error. + if (DiagLevel == DiagnosticsEngine::Warning && + CTContext->treatAsError(CheckName)) { + return DiagnosticsEngine::Error; + } } } return DiagLevel; diff --git a/clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp b/clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp index b94de7412ce3..3d838c78a3c5 100644 --- a/clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp +++ b/clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp @@ -374,23 +374,17 @@ n]] = 10; // error-ok 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) { _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits