Index: clang-tools-extra/trunk/clang-tidy/ClangTidyDiagnosticConsumer.h =================================================================== --- clang-tools-extra/trunk/clang-tidy/ClangTidyDiagnosticConsumer.h +++ clang-tools-extra/trunk/clang-tidy/ClangTidyDiagnosticConsumer.h @@ -217,6 +217,23 @@ bool AllowEnablingAnalyzerAlphaCheckers; }; +/// Check whether a given diagnostic should be suppressed due to the presence +/// of a "NOLINT" suppression comment. +/// This is exposed so that other tools that present clang-tidy diagnostics +/// (such as clangd) can respect the same suppression rules as clang-tidy. +/// This does not handle suppression of notes following a suppressed diagnostic; +/// that is left to the caller is it requires maintaining state in between calls +/// to this function. +/// The `CheckMacroExpansion` parameter determines whether the function should +/// handle the case where the diagnostic is inside a macro expansion. A degree +/// of control over this is needed because handling this case can require +/// examining source files other than the one in which the diagnostic is +/// located, and in some use cases we cannot rely on such other files being +/// mapped in the SourceMapper. +bool ShouldSuppressDiagnostic(DiagnosticsEngine::Level DiagLevel, + const Diagnostic &Info, ClangTidyContext &Context, + bool CheckMacroExpansion = true); + /// \brief A diagnostic consumer that turns each \c Diagnostic into a /// \c SourceManager-independent \c ClangTidyError. // @@ -246,7 +263,7 @@ /// \brief Updates \c LastErrorRelatesToUserCode and LastErrorPassesLineFilter /// according to the diagnostic \p Location. - void checkFilters(SourceLocation Location, const SourceManager& Sources); + void checkFilters(SourceLocation Location, const SourceManager &Sources); bool passesLineFilter(StringRef FileName, unsigned LineNumber) const; ClangTidyContext &Context; Index: clang-tools-extra/trunk/clang-tidy/ClangTidyDiagnosticConsumer.cpp =================================================================== --- clang-tools-extra/trunk/clang-tidy/ClangTidyDiagnosticConsumer.cpp +++ clang-tools-extra/trunk/clang-tidy/ClangTidyDiagnosticConsumer.cpp @@ -166,11 +166,13 @@ bool contains(StringRef S) { switch (auto &Result = Cache[S]) { - case Yes: return true; - case No: return false; - case None: - Result = Globs.contains(S) ? Yes : No; - return Result == Yes; + case Yes: + return true; + case No: + return false; + case None: + Result = Globs.contains(S) ? Yes : No; + return Result == Yes; } llvm_unreachable("invalid enum"); } @@ -387,16 +389,30 @@ return false; } +namespace clang { +namespace tidy { + +bool ShouldSuppressDiagnostic(DiagnosticsEngine::Level DiagLevel, + const Diagnostic &Info, ClangTidyContext &Context, + bool CheckMacroExpansion) { + return Info.getLocation().isValid() && + DiagLevel != DiagnosticsEngine::Error && + DiagLevel != DiagnosticsEngine::Fatal && + (CheckMacroExpansion ? LineIsMarkedWithNOLINTinMacro + : LineIsMarkedWithNOLINT)(Info.getSourceManager(), + Info.getLocation(), + Info.getID(), Context); +} + +} // namespace tidy +} // namespace clang + void ClangTidyDiagnosticConsumer::HandleDiagnostic( DiagnosticsEngine::Level DiagLevel, const Diagnostic &Info) { if (LastErrorWasIgnored && DiagLevel == DiagnosticsEngine::Note) return; - if (Info.getLocation().isValid() && DiagLevel != DiagnosticsEngine::Error && - DiagLevel != DiagnosticsEngine::Fatal && - LineIsMarkedWithNOLINTinMacro(Info.getSourceManager(), - Info.getLocation(), Info.getID(), - Context)) { + if (ShouldSuppressDiagnostic(DiagLevel, Info, Context)) { ++Context.Stats.ErrorsIgnoredNOLINT; // Ignored a warning, should ignore related notes as well LastErrorWasIgnored = true; Index: clang-tools-extra/trunk/clangd/ClangdUnit.cpp =================================================================== --- clang-tools-extra/trunk/clangd/ClangdUnit.cpp +++ clang-tools-extra/trunk/clangd/ClangdUnit.cpp @@ -113,12 +113,12 @@ } void EndOfMainFile() { - for (const auto& Entry : MainFileMacros) + for (const auto &Entry : MainFileMacros) Out->push_back(Entry.getKey()); llvm::sort(*Out); } - private: +private: const SourceManager &SM; bool InMainFile = true; llvm::StringSet<> MainFileMacros; @@ -332,6 +332,30 @@ CTContext->setASTContext(&Clang->getASTContext()); CTContext->setCurrentFile(MainInput.getFile()); CTFactories.createChecks(CTContext.getPointer(), CTChecks); + ASTDiags.suppressDiagnostics([&CTContext]( + DiagnosticsEngine::Level DiagLevel, + const clang::Diagnostic &Info) { + if (CTContext) { + bool IsClangTidyDiag = !CTContext->getCheckName(Info.getID()).empty(); + if (IsClangTidyDiag) { + // Skip the ShouldSuppressDiagnostic 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. + bool IsInsideMainFile = + Info.hasSourceManager() && + Info.getSourceManager().isWrittenInMainFile( + Info.getSourceManager().getFileLoc(Info.getLocation())); + if (IsInsideMainFile && tidy::ShouldSuppressDiagnostic( + DiagLevel, Info, *CTContext, + /* CheckMacroExpansion = */ false)) { + return true; + } + } + } + return false; + }); Preprocessor *PP = &Clang->getPreprocessor(); for (const auto &Check : CTChecks) { // FIXME: the PP callbacks skip the entire preamble. Index: clang-tools-extra/trunk/clangd/Diagnostics.h =================================================================== --- clang-tools-extra/trunk/clangd/Diagnostics.h +++ clang-tools-extra/trunk/clangd/Diagnostics.h @@ -128,17 +128,25 @@ using DiagFixer = std::function(DiagnosticsEngine::Level, const clang::Diagnostic &)>; + using DiagFilter = + std::function; /// 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; + } private: void flushLastDiag(); DiagFixer Fixer = nullptr; + DiagFilter SuppressionFilter = nullptr; std::vector Output; llvm::Optional LangOpts; llvm::Optional LastDiag; llvm::DenseSet IncludeLinesWithErrors; + bool LastPrimaryDiagnosticWasSuppressed = false; }; } // namespace clangd Index: clang-tools-extra/trunk/clangd/Diagnostics.cpp =================================================================== --- clang-tools-extra/trunk/clangd/Diagnostics.cpp +++ clang-tools-extra/trunk/clangd/Diagnostics.cpp @@ -514,6 +514,12 @@ // Handle the new main diagnostic. flushLastDiag(); + if (SuppressionFilter && SuppressionFilter(DiagLevel, Info)) { + LastPrimaryDiagnosticWasSuppressed = true; + return; + } + LastPrimaryDiagnosticWasSuppressed = false; + LastDiag = Diag(); LastDiag->ID = Info.getID(); FillDiagBase(*LastDiag); @@ -528,6 +534,13 @@ } } else { // Handle a note to an existing diagnostic. + + // If a diagnostic was suppressed due to the suppression filter, + // also suppress notes associated with it. + if (LastPrimaryDiagnosticWasSuppressed) { + return; + } + if (!LastDiag) { assert(false && "Adding a note without main diagnostic"); IgnoreDiagnostics::log(DiagLevel, Info); Index: clang-tools-extra/trunk/clangd/unittests/DiagnosticsTests.cpp =================================================================== --- clang-tools-extra/trunk/clangd/unittests/DiagnosticsTests.cpp +++ clang-tools-extra/trunk/clangd/unittests/DiagnosticsTests.cpp @@ -207,6 +207,26 @@ "multiple unsequenced modifications to 'y'"))); } +TEST(DiagnosticTest, ClangTidySuppressionComment) { + Annotations Main(R"cpp( + int main() { + int i = 3; + double d = 8 / i; // NOLINT + // NOLINTNEXTLINE + double e = 8 / i; + double f = [[8]] / i; + } + )cpp"); + TestTU TU = TestTU::withCode(Main.code()); + TU.ClangTidyChecks = "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")))); +} + TEST(DiagnosticsTest, Preprocessor) { // This looks like a preamble, but there's an #else in the middle! // Check that: @@ -767,6 +787,7 @@ "a type specifier for all declarations"), WithNote(Diag(Header.range(), "error occurred here"))))); } + } // namespace } // namespace clangd