[clang-tidy] Allow disabling support for NOLINTBEGIN/NOLINTEND blocks.

[ymandel]

Summary

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.

[ymandel]

change to "enabled-by-default"

[gribozavr2]

Thank you for a quick workaround!

[ymandel]

remove incorrect line in comment

clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.h

 /// 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 as it requires maintaining state in between calls
 /// to this function.
 /// 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
 /// via the output argument `SuppressionErrors`.
 bool shouldSuppressDiagnostic(
     DiagnosticsEngine::Level DiagLevel, const Diagnostic &Info,
     ClangTidyContext &Context,
-    SmallVectorImpl<ClangTidyError> &SuppressionErrors, bool AllowIO = true);
+    SmallVectorImpl<ClangTidyError> &SuppressionErrors, bool AllowIO = true,
+    bool EnableNolintBlocks = false);
 
 /// Gets the Fix attached to \p Diagnostic.
 /// If there isn't a Fix attached to the diagnostic and \p AnyFix is true, Check
 /// to see if exactly one note has a Fix and return it. Otherwise return
 /// nullptr.
 const llvm::StringMap<tooling::Replacements> *
 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. Off by default because of its
+/// high cost.
 //
 // FIXME: If we move away from unit-tests, this can be moved to a private
 // implementation file.
 class ClangTidyDiagnosticConsumer : public DiagnosticConsumer {
 public:
   ClangTidyDiagnosticConsumer(ClangTidyContext &Ctx,
                               DiagnosticsEngine *ExternalDiagEngine = nullptr,
                               bool RemoveIncompatibleErrors = true,
-                              bool GetFixesFromNotes = false);
+                              bool GetFixesFromNotes = false,
+                              bool EnableNolintBlocks = false);
 
   // FIXME: The concept of converting between FixItHints and Replacements is
   // more generic and should be pulled out into a more useful Diagnostics
   // library.
   void HandleDiagnostic(DiagnosticsEngine::Level DiagLevel,
                         const Diagnostic &Info) override;
 
   // Retrieve the diagnostics that were captured.
private:
   bool passesLineFilter(StringRef FileName, unsigned LineNumber) const;
 
   void forwardDiagnostic(const Diagnostic &Info);
 
   ClangTidyContext &Context;
   DiagnosticsEngine *ExternalDiagEngine;
   bool RemoveIncompatibleErrors;
   bool GetFixesFromNotes;
+  bool EnableNolintBlocks;
   std::vector<ClangTidyError> Errors;
   std::unique_ptr<llvm::Regex> HeaderFilter;
   bool LastErrorRelatesToUserCode;
   bool LastErrorPassesLineFilter;
   bool LastErrorWasIgnored;
 };
 
 } // end namespace tidy
 } // end namespace clang
 
 #endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_CLANGTIDYDIAGNOSTICCONSUMER_H

clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp

llvm::DenseMap<unsigned, std::string>::const_iterator I =
       CheckNamesByDiagnosticID.find(DiagnosticID);
   if (I != CheckNamesByDiagnosticID.end())
     return I->second;
   return "";
 }
 
 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() {
   if (!Errors.empty()) {
     ClangTidyError &Error = Errors.back();
     if (Error.DiagnosticName == "clang-tidy-config") {
       // Never ignore these.
     } else if (!Context.isCheckEnabled(Error.DiagnosticName) &&
lineIsWithinNolintBegin(const ClangTidyContext &Context,
 
   return WithinNolintBegin && SuppressionErrors.empty();
 }
 
 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;
   std::tie(File, Offset) = SM.getDecomposedSpellingLoc(Loc);
   Optional<StringRef> Buffer = getBuffer(SM, File, AllowIO);
   if (!Buffer)
     return false;
 
lineIsMarkedWithNOLINT(const ClangTidyContext &Context,
   LastNewLinePos = PrevLines.rfind('\n');
   StringRef PrevLine = (LastNewLinePos == StringRef::npos)
                            ? PrevLines
                            : PrevLines.substr(LastNewLinePos + 1);
   if (isNOLINTFound("NOLINTNEXTLINE", CheckName, PrevLine))
     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;
     Loc = SM.getImmediateExpansionRange(Loc).getBegin();
   }
   return false;
 }
 
 namespace clang {
 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> *
 getFixIt(const tooling::Diagnostic &Diagnostic, bool GetFixFromNotes) {
   if (!Diagnostic.Message.Fix.empty())
     return &Diagnostic.Message.Fix;
   if (!GetFixFromNotes)
     return nullptr;

 } // namespace clang
 
 void ClangTidyDiagnosticConsumer::HandleDiagnostic(
     DiagnosticsEngine::Level DiagLevel, const Diagnostic &Info) {
   if (LastErrorWasIgnored && DiagLevel == DiagnosticsEngine::Note)
     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;
     return;
   }
 
   LastErrorWasIgnored = false;
   // Count warnings/errors.