[clang-tools-extra] 081074e - [clang-tidy] Allow disabling support for NOLINTBEGIN/NOLINTEND blocks.

[Yitzhak Mandelbaum via cfe-commits]

Author: Yitzhak Mandelbaum
Date: 2021-12-02T22:19:05Z
New Revision: 081074e1ea5353a3775591f7306b6fb6da02b004

URL: https://github.com/llvm/llvm-project/commit/081074e1ea5353a3775591f7306b6fb6da02b004
DIFF: https://github.com/llvm/llvm-project/commit/081074e1ea5353a3775591f7306b6fb6da02b004.diff

LOG: [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

Added: 
    

Modified: 
    clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
    clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.h

Removed: 
    


################################################################################
diff  --git a/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp b/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
index 764866d55083..66329328757b 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<ClangTidyError> &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<ClangTidyError> &SuppressionErrors, bool AllowIO) {
+    SmallVectorImpl<ClangTidyError> &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<ClangTidyError> &SuppressionErrors, bool AllowIO) {
+    SmallVectorImpl<ClangTidyError> &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<tooling::Replacements> *
@@ -561,7 +567,8 @@ void ClangTidyDiagnosticConsumer::HandleDiagnostic(
     return;
 
   SmallVector<ClangTidyError, 1> 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 9f519fb605ad..129c06f60f56 100644
--- a/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.h
+++ b/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.h
@@ -219,6 +219,8 @@ class ClangTidyContext {
 /// 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 @@ class ClangTidyContext {
 bool shouldSuppressDiagnostic(
     DiagnosticsEngine::Level DiagLevel, const Diagnostic &Info,
     ClangTidyContext &Context,
-    SmallVectorImpl<ClangTidyError> &SuppressionErrors, bool AllowIO = true);
+    SmallVectorImpl<ClangTidyError> &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 @@ class ClangTidyDiagnosticConsumer : public DiagnosticConsumer {
   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 @@ class ClangTidyDiagnosticConsumer : public DiagnosticConsumer {
   DiagnosticsEngine *ExternalDiagEngine;
   bool RemoveIncompatibleErrors;
   bool GetFixesFromNotes;
+  bool EnableNolintBlocks;
   std::vector<ClangTidyError> Errors;
   std::unique_ptr<llvm::Regex> HeaderFilter;
   bool LastErrorRelatesToUserCode;