From 081074e1ea5353a3775591f7306b6fb6da02b004 Mon Sep 17 00:00:00 2001 From: Yitzhak Mandelbaum Date: Thu, 2 Dec 2021 19:52:47 +0000 Subject: [PATCH] [clang-tidy] Allow disabling support for NOLINTBEGIN/NOLINTEND blocks. This patch parameterizes the clang-tidy diagnostic consumer with a boolean that controls whether to honor NOLINTBEGIN/NOLINTEND blocks. The current support for scanning these blocks is very costly -- O(n*m) in the size of files (n) and number of diagnostics found (m), with a large constant factor. So, the patch allows clients to disable it. Future patches should make the feature more efficient, but this will mitigate in the interim. Differential Revision: https://reviews.llvm.org/D114981 --- .../clang-tidy/ClangTidyDiagnosticConsumer.cpp | 25 ++++++++++++++-------- .../clang-tidy/ClangTidyDiagnosticConsumer.h | 12 +++++++++-- 2 files changed, 26 insertions(+), 11 deletions(-) diff --git a/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp b/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp index 764866d..66329328 100644 --- a/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp +++ b/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp @@ -297,10 +297,12 @@ std::string ClangTidyContext::getCheckName(unsigned DiagnosticID) const { ClangTidyDiagnosticConsumer::ClangTidyDiagnosticConsumer( ClangTidyContext &Ctx, DiagnosticsEngine *ExternalDiagEngine, - bool RemoveIncompatibleErrors, bool GetFixesFromNotes) + bool RemoveIncompatibleErrors, bool GetFixesFromNotes, + bool EnableNolintBlocks) : Context(Ctx), ExternalDiagEngine(ExternalDiagEngine), RemoveIncompatibleErrors(RemoveIncompatibleErrors), - GetFixesFromNotes(GetFixesFromNotes), LastErrorRelatesToUserCode(false), + GetFixesFromNotes(GetFixesFromNotes), + EnableNolintBlocks(EnableNolintBlocks), LastErrorRelatesToUserCode(false), LastErrorPassesLineFilter(false), LastErrorWasIgnored(false) {} void ClangTidyDiagnosticConsumer::finalizeLastError() { @@ -469,7 +471,8 @@ static bool lineIsMarkedWithNOLINT(const ClangTidyContext &Context, SmallVectorImpl &SuppressionErrors, bool AllowIO, const SourceManager &SM, - SourceLocation Loc, StringRef CheckName) { + SourceLocation Loc, StringRef CheckName, + bool EnableNolintBlocks) { // Get source code for this location. FileID File; unsigned Offset; @@ -499,19 +502,21 @@ lineIsMarkedWithNOLINT(const ClangTidyContext &Context, return true; // Check if this line is within a NOLINT(BEGIN...END) block. - return lineIsWithinNolintBegin(Context, SuppressionErrors, SM, Loc, CheckName, + return EnableNolintBlocks && + lineIsWithinNolintBegin(Context, SuppressionErrors, SM, Loc, CheckName, TextBeforeDiag, TextAfterDiag); } static bool lineIsMarkedWithNOLINTinMacro( const Diagnostic &Info, const ClangTidyContext &Context, - SmallVectorImpl &SuppressionErrors, bool AllowIO) { + SmallVectorImpl &SuppressionErrors, bool AllowIO, + bool EnableNolintBlocks) { const SourceManager &SM = Info.getSourceManager(); SourceLocation Loc = Info.getLocation(); std::string CheckName = Context.getCheckName(Info.getID()); while (true) { if (lineIsMarkedWithNOLINT(Context, SuppressionErrors, AllowIO, SM, Loc, - CheckName)) + CheckName, EnableNolintBlocks)) return true; if (!Loc.isMacroID()) return false; @@ -526,12 +531,13 @@ namespace tidy { bool shouldSuppressDiagnostic( DiagnosticsEngine::Level DiagLevel, const Diagnostic &Info, ClangTidyContext &Context, - SmallVectorImpl &SuppressionErrors, bool AllowIO) { + SmallVectorImpl &SuppressionErrors, bool AllowIO, + bool EnableNolintBlocks) { return Info.getLocation().isValid() && DiagLevel != DiagnosticsEngine::Error && DiagLevel != DiagnosticsEngine::Fatal && lineIsMarkedWithNOLINTinMacro(Info, Context, SuppressionErrors, - AllowIO); + AllowIO, EnableNolintBlocks); } const llvm::StringMap * @@ -561,7 +567,8 @@ void ClangTidyDiagnosticConsumer::HandleDiagnostic( return; SmallVector SuppressionErrors; - if (shouldSuppressDiagnostic(DiagLevel, Info, Context, SuppressionErrors)) { + if (shouldSuppressDiagnostic(DiagLevel, Info, Context, SuppressionErrors, + EnableNolintBlocks)) { ++Context.Stats.ErrorsIgnoredNOLINT; // Ignored a warning, should ignore related notes as well LastErrorWasIgnored = true; diff --git a/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.h b/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.h index 9f519fb..129c06f 100644 --- a/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.h +++ b/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.h @@ -219,6 +219,8 @@ private: /// If `AllowIO` is false, the function does not attempt to read source files /// from disk which are not already mapped into memory; such files are treated /// as not containing a suppression comment. +/// \param EnableNolintBlocks controls whether to honor NOLINTBEGIN/NOLINTEND +/// blocks; if false, only considers line-level disabling. /// If suppression is not possible due to improper use of "NOLINT" comments - /// for example, the use of a "NOLINTBEGIN" comment that is not followed by a /// "NOLINTEND" comment - a diagnostic regarding the improper use is returned @@ -226,7 +228,8 @@ private: bool shouldSuppressDiagnostic( DiagnosticsEngine::Level DiagLevel, const Diagnostic &Info, ClangTidyContext &Context, - SmallVectorImpl &SuppressionErrors, bool AllowIO = true); + SmallVectorImpl &SuppressionErrors, bool AllowIO = true, + bool EnableNolintBlocks = true); /// Gets the Fix attached to \p Diagnostic. /// If there isn't a Fix attached to the diagnostic and \p AnyFix is true, Check @@ -237,6 +240,9 @@ getFixIt(const tooling::Diagnostic &Diagnostic, bool AnyFix); /// A diagnostic consumer that turns each \c Diagnostic into a /// \c SourceManager-independent \c ClangTidyError. +/// +/// \param EnableNolintBlocks Enables diagnostic-disabling inside blocks of +/// code, delimited by NOLINTBEGIN and NOLINTEND. // // FIXME: If we move away from unit-tests, this can be moved to a private // implementation file. @@ -245,7 +251,8 @@ public: ClangTidyDiagnosticConsumer(ClangTidyContext &Ctx, DiagnosticsEngine *ExternalDiagEngine = nullptr, bool RemoveIncompatibleErrors = true, - bool GetFixesFromNotes = false); + bool GetFixesFromNotes = false, + bool EnableNolintBlocks = true); // FIXME: The concept of converting between FixItHints and Replacements is // more generic and should be pulled out into a more useful Diagnostics @@ -276,6 +283,7 @@ private: DiagnosticsEngine *ExternalDiagEngine; bool RemoveIncompatibleErrors; bool GetFixesFromNotes; + bool EnableNolintBlocks; std::vector Errors; std::unique_ptr HeaderFilter; bool LastErrorRelatesToUserCode; -- 2.7.4