nridge updated this revision to Diff 199956.
nridge added a comment.
Update as per changes to dependent patch D60953
<https://reviews.llvm.org/D60953>
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D61841/new/
https://reviews.llvm.org/D61841
Files:
clang-tools-extra/clangd/ClangdUnit.cpp
clang-tools-extra/clangd/Diagnostics.cpp
clang-tools-extra/clangd/Diagnostics.h
clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
clang-tools-extra/clangd/unittests/TestTU.cpp
clang-tools-extra/clangd/unittests/TestTU.h
Index: clang-tools-extra/clangd/unittests/TestTU.h
===================================================================
--- clang-tools-extra/clangd/unittests/TestTU.h
+++ clang-tools-extra/clangd/unittests/TestTU.h
@@ -57,6 +57,7 @@
std::vector<const char *> ExtraArgs;
llvm::Optional<std::string> ClangTidyChecks;
+ llvm::Optional<std::string> ClangTidyWarningsAsErrors;
// Index to use when building AST.
const SymbolIndex *ExternalIndex = nullptr;
Index: clang-tools-extra/clangd/unittests/TestTU.cpp
===================================================================
--- clang-tools-extra/clangd/unittests/TestTU.cpp
+++ clang-tools-extra/clangd/unittests/TestTU.cpp
@@ -48,6 +48,7 @@
Inputs.FS = buildTestFS(Files);
Inputs.Opts = ParseOptions();
Inputs.Opts.ClangTidyOpts.Checks = ClangTidyChecks;
+ Inputs.Opts.ClangTidyOpts.WarningsAsErrors = ClangTidyWarningsAsErrors;
Inputs.Index = ExternalIndex;
if (Inputs.Index)
Inputs.Opts.SuggestMissingIncludes = true;
Index: clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
===================================================================
--- clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
+++ clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
@@ -73,6 +73,7 @@
MATCHER_P(DiagSource, S, "") { return arg.Source == S; }
MATCHER_P(DiagName, N, "") { return arg.Name == N; }
+MATCHER_P(DiagSeverity, S, "") { return arg.Severity == S; }
MATCHER_P(EqualToFix, Fix, "LSP fix " + llvm::to_string(Fix)) {
if (arg.Message != Fix.Message)
@@ -227,6 +228,44 @@
DiagSource(Diag::ClangTidy), DiagName("bugprone-integer-division"))));
}
+TEST(DiagnosticTest, ClangTidyWarningAsError) {
+ Annotations Main(R"cpp(
+ int main() {
+ int i = 3;
+ double f = [[8]] / i;
+ }
+ )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))));
+}
+
+TEST(DiagnosticTest, ClangTidyWarningAsErrorTrumpsSuppressionComment) {
+ Annotations Main(R"cpp(
+ int main() {
+ int i = 3;
+ 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))));
+}
+
TEST(DiagnosticsTest, Preprocessor) {
// This looks like a preamble, but there's an #else in the middle!
// Check that:
Index: clang-tools-extra/clangd/Diagnostics.h
===================================================================
--- clang-tools-extra/clangd/Diagnostics.h
+++ clang-tools-extra/clangd/Diagnostics.h
@@ -128,20 +128,20 @@
using DiagFixer = std::function<std::vector<Fix>(DiagnosticsEngine::Level,
const clang::Diagnostic &)>;
- using DiagFilter =
- std::function<bool(DiagnosticsEngine::Level, const clang::Diagnostic &)>;
+ using LevelAdjuster = std::function<DiagnosticsEngine::Level(
+ DiagnosticsEngine::Level, const clang::Diagnostic &)>;
/// If set, possibly adds fixes for diagnostics using \p Fixer.
void contributeFixes(DiagFixer Fixer) { this->Fixer = Fixer; }
- /// If set, ignore diagnostics for which \p SuppressionFilter returns true.
- void suppressDiagnostics(DiagFilter SuppressionFilter) {
- this->SuppressionFilter = SuppressionFilter;
- }
+ /// If set, this allows the client of this class to adjust the level of
+ /// diagnostics, such as promoting warnings to errors, or ignoring
+ /// diagnostics.
+ void setLevelAdjuster(LevelAdjuster Adjuster) { this->Adjuster = Adjuster; }
private:
void flushLastDiag();
DiagFixer Fixer = nullptr;
- DiagFilter SuppressionFilter = nullptr;
+ LevelAdjuster Adjuster = nullptr;
std::vector<Diag> Output;
llvm::Optional<LangOptions> LangOpts;
llvm::Optional<Diag> LastDiag;
Index: clang-tools-extra/clangd/Diagnostics.cpp
===================================================================
--- clang-tools-extra/clangd/Diagnostics.cpp
+++ clang-tools-extra/clangd/Diagnostics.cpp
@@ -514,9 +514,12 @@
// Handle the new main diagnostic.
flushLastDiag();
- if (SuppressionFilter && SuppressionFilter(DiagLevel, Info)) {
- LastPrimaryDiagnosticWasSuppressed = true;
- return;
+ if (Adjuster) {
+ DiagLevel = Adjuster(DiagLevel, Info);
+ if (DiagLevel == DiagnosticsEngine::Ignored) {
+ LastPrimaryDiagnosticWasSuppressed = true;
+ return;
+ }
}
LastPrimaryDiagnosticWasSuppressed = false;
Index: clang-tools-extra/clangd/ClangdUnit.cpp
===================================================================
--- clang-tools-extra/clangd/ClangdUnit.cpp
+++ clang-tools-extra/clangd/ClangdUnit.cpp
@@ -332,14 +332,22 @@
CTContext->setASTContext(&Clang->getASTContext());
CTContext->setCurrentFile(MainInput.getFile());
CTFactories.createChecks(CTContext.getPointer(), CTChecks);
- ASTDiags.suppressDiagnostics([&CTContext](
- DiagnosticsEngine::Level DiagLevel,
- const clang::Diagnostic &Info) {
+ ASTDiags.setLevelAdjuster([&CTContext](DiagnosticsEngine::Level DiagLevel,
+ const clang::Diagnostic &Info) {
if (CTContext) {
- bool IsClangTidyDiag = !CTContext->getCheckName(Info.getID()).empty();
+ std::string CheckName = CTContext->getCheckName(Info.getID());
+ bool IsClangTidyDiag = !CheckName.empty();
if (IsClangTidyDiag) {
- // Skip the ShouldSuppressDiagnostic check for diagnostics not in
- // the main file, because we don't want that function to query the
+ // 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 not to follow macro expansions, since
// those might take us into a preamble file as well.
@@ -350,11 +358,11 @@
if (IsInsideMainFile && tidy::ShouldSuppressDiagnostic(
DiagLevel, Info, *CTContext,
/* CheckMacroExpansion = */ false)) {
- return true;
+ return DiagnosticsEngine::Ignored;
}
}
}
- return false;
+ return DiagLevel;
});
Preprocessor *PP = &Clang->getPreprocessor();
for (const auto &Check : CTChecks) {
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
