Skip to content

Commit 081074e

Browse files
committed
[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
1 parent d257f7c commit 081074e

File tree

2 files changed

+26
-11
lines changed

2 files changed

+26
-11
lines changed

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

Lines changed: 16 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -297,10 +297,12 @@ std::string ClangTidyContext::getCheckName(unsigned DiagnosticID) const {
297297

298298
ClangTidyDiagnosticConsumer::ClangTidyDiagnosticConsumer(
299299
ClangTidyContext &Ctx, DiagnosticsEngine *ExternalDiagEngine,
300-
bool RemoveIncompatibleErrors, bool GetFixesFromNotes)
300+
bool RemoveIncompatibleErrors, bool GetFixesFromNotes,
301+
bool EnableNolintBlocks)
301302
: Context(Ctx), ExternalDiagEngine(ExternalDiagEngine),
302303
RemoveIncompatibleErrors(RemoveIncompatibleErrors),
303-
GetFixesFromNotes(GetFixesFromNotes), LastErrorRelatesToUserCode(false),
304+
GetFixesFromNotes(GetFixesFromNotes),
305+
EnableNolintBlocks(EnableNolintBlocks), LastErrorRelatesToUserCode(false),
304306
LastErrorPassesLineFilter(false), LastErrorWasIgnored(false) {}
305307

306308
void ClangTidyDiagnosticConsumer::finalizeLastError() {
@@ -469,7 +471,8 @@ static bool
469471
lineIsMarkedWithNOLINT(const ClangTidyContext &Context,
470472
SmallVectorImpl<ClangTidyError> &SuppressionErrors,
471473
bool AllowIO, const SourceManager &SM,
472-
SourceLocation Loc, StringRef CheckName) {
474+
SourceLocation Loc, StringRef CheckName,
475+
bool EnableNolintBlocks) {
473476
// Get source code for this location.
474477
FileID File;
475478
unsigned Offset;
@@ -499,19 +502,21 @@ lineIsMarkedWithNOLINT(const ClangTidyContext &Context,
499502
return true;
500503

501504
// Check if this line is within a NOLINT(BEGIN...END) block.
502-
return lineIsWithinNolintBegin(Context, SuppressionErrors, SM, Loc, CheckName,
505+
return EnableNolintBlocks &&
506+
lineIsWithinNolintBegin(Context, SuppressionErrors, SM, Loc, CheckName,
503507
TextBeforeDiag, TextAfterDiag);
504508
}
505509

506510
static bool lineIsMarkedWithNOLINTinMacro(
507511
const Diagnostic &Info, const ClangTidyContext &Context,
508-
SmallVectorImpl<ClangTidyError> &SuppressionErrors, bool AllowIO) {
512+
SmallVectorImpl<ClangTidyError> &SuppressionErrors, bool AllowIO,
513+
bool EnableNolintBlocks) {
509514
const SourceManager &SM = Info.getSourceManager();
510515
SourceLocation Loc = Info.getLocation();
511516
std::string CheckName = Context.getCheckName(Info.getID());
512517
while (true) {
513518
if (lineIsMarkedWithNOLINT(Context, SuppressionErrors, AllowIO, SM, Loc,
514-
CheckName))
519+
CheckName, EnableNolintBlocks))
515520
return true;
516521
if (!Loc.isMacroID())
517522
return false;
@@ -526,12 +531,13 @@ namespace tidy {
526531
bool shouldSuppressDiagnostic(
527532
DiagnosticsEngine::Level DiagLevel, const Diagnostic &Info,
528533
ClangTidyContext &Context,
529-
SmallVectorImpl<ClangTidyError> &SuppressionErrors, bool AllowIO) {
534+
SmallVectorImpl<ClangTidyError> &SuppressionErrors, bool AllowIO,
535+
bool EnableNolintBlocks) {
530536
return Info.getLocation().isValid() &&
531537
DiagLevel != DiagnosticsEngine::Error &&
532538
DiagLevel != DiagnosticsEngine::Fatal &&
533539
lineIsMarkedWithNOLINTinMacro(Info, Context, SuppressionErrors,
534-
AllowIO);
540+
AllowIO, EnableNolintBlocks);
535541
}
536542

537543
const llvm::StringMap<tooling::Replacements> *
@@ -561,7 +567,8 @@ void ClangTidyDiagnosticConsumer::HandleDiagnostic(
561567
return;
562568

563569
SmallVector<ClangTidyError, 1> SuppressionErrors;
564-
if (shouldSuppressDiagnostic(DiagLevel, Info, Context, SuppressionErrors)) {
570+
if (shouldSuppressDiagnostic(DiagLevel, Info, Context, SuppressionErrors,
571+
EnableNolintBlocks)) {
565572
++Context.Stats.ErrorsIgnoredNOLINT;
566573
// Ignored a warning, should ignore related notes as well
567574
LastErrorWasIgnored = true;

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

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -219,14 +219,17 @@ class ClangTidyContext {
219219
/// If `AllowIO` is false, the function does not attempt to read source files
220220
/// from disk which are not already mapped into memory; such files are treated
221221
/// as not containing a suppression comment.
222+
/// \param EnableNolintBlocks controls whether to honor NOLINTBEGIN/NOLINTEND
223+
/// blocks; if false, only considers line-level disabling.
222224
/// If suppression is not possible due to improper use of "NOLINT" comments -
223225
/// for example, the use of a "NOLINTBEGIN" comment that is not followed by a
224226
/// "NOLINTEND" comment - a diagnostic regarding the improper use is returned
225227
/// via the output argument `SuppressionErrors`.
226228
bool shouldSuppressDiagnostic(
227229
DiagnosticsEngine::Level DiagLevel, const Diagnostic &Info,
228230
ClangTidyContext &Context,
229-
SmallVectorImpl<ClangTidyError> &SuppressionErrors, bool AllowIO = true);
231+
SmallVectorImpl<ClangTidyError> &SuppressionErrors, bool AllowIO = true,
232+
bool EnableNolintBlocks = true);
230233

231234
/// Gets the Fix attached to \p Diagnostic.
232235
/// 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);
237240

238241
/// A diagnostic consumer that turns each \c Diagnostic into a
239242
/// \c SourceManager-independent \c ClangTidyError.
243+
///
244+
/// \param EnableNolintBlocks Enables diagnostic-disabling inside blocks of
245+
/// code, delimited by NOLINTBEGIN and NOLINTEND.
240246
//
241247
// FIXME: If we move away from unit-tests, this can be moved to a private
242248
// implementation file.
@@ -245,7 +251,8 @@ class ClangTidyDiagnosticConsumer : public DiagnosticConsumer {
245251
ClangTidyDiagnosticConsumer(ClangTidyContext &Ctx,
246252
DiagnosticsEngine *ExternalDiagEngine = nullptr,
247253
bool RemoveIncompatibleErrors = true,
248-
bool GetFixesFromNotes = false);
254+
bool GetFixesFromNotes = false,
255+
bool EnableNolintBlocks = true);
249256

250257
// FIXME: The concept of converting between FixItHints and Replacements is
251258
// more generic and should be pulled out into a more useful Diagnostics
@@ -276,6 +283,7 @@ class ClangTidyDiagnosticConsumer : public DiagnosticConsumer {
276283
DiagnosticsEngine *ExternalDiagEngine;
277284
bool RemoveIncompatibleErrors;
278285
bool GetFixesFromNotes;
286+
bool EnableNolintBlocks;
279287
std::vector<ClangTidyError> Errors;
280288
std::unique_ptr<llvm::Regex> HeaderFilter;
281289
bool LastErrorRelatesToUserCode;

0 commit comments

Comments
 (0)