Skip to content

Commit

Permalink
[clang-tidy] Fix #55134 (regression introduced by 5da7c04)
Browse files Browse the repository at this point in the history
5da7c04 introduced a regression in the NOLINT macro checking loop, replacing the
call to `getImmediateExpansionRange().getBegin()` with
`getImmediateMacroCallerLoc()`, which has similar but subtly different
behaviour.

The consequence is that NOLINTs cannot suppress diagnostics when they are
attached to a token that came from a macro **argument**, rather than elsewhere
in the macro expansion.

Revert to pre-patch behaviour and add test cases to cover this issue.

Differential Revision: https://reviews.llvm.org/D126138
  • Loading branch information
salman-javed-nz committed May 24, 2022
1 parent b2b0322 commit 9ff4f2d
Show file tree
Hide file tree
Showing 3 changed files with 24 additions and 3 deletions.
2 changes: 1 addition & 1 deletion clang-tools-extra/clang-tidy/NoLintDirectiveHandler.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -266,7 +266,7 @@ bool NoLintDirectiveHandler::Impl::diagHasNoLintInMacro(
return true;
if (!DiagLoc.isMacroID())
return false;
DiagLoc = SrcMgr.getImmediateMacroCallerLoc(DiagLoc);
DiagLoc = SrcMgr.getImmediateExpansionRange(DiagLoc).getBegin();
}
return false;
}
Expand Down
4 changes: 4 additions & 0 deletions clang-tools-extra/docs/ReleaseNotes.rst
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,10 @@ Improvements to clang-tidy
means it is advised to use YAML's block style initiated by the pipe character `|` for the `Checks`
section in order to benefit from the easier syntax that works without commas.

- Fixed a regression introduced in clang-tidy 14.0.0, which prevented NOLINTs
from suppressing diagnostics associated with macro arguments. This fixes
`Issue 55134 <https://github.com/llvm/llvm-project/issues/55134>`_.

New checks
^^^^^^^^^^

Expand Down
21 changes: 19 additions & 2 deletions clang-tools-extra/test/clang-tidy/infrastructure/nolint.cpp
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
// REQUIRES: static-analyzer
// RUN: %check_clang_tidy %s google-explicit-constructor,clang-diagnostic-unused-variable,clang-analyzer-core.UndefinedBinaryOperatorResult,modernize-avoid-c-arrays,cppcoreguidelines-avoid-c-arrays %t -- -extra-arg=-Wunused-variable -- -I%S/Inputs/nolint
// RUN: %check_clang_tidy %s google-explicit-constructor,clang-diagnostic-unused-variable,clang-analyzer-core.UndefinedBinaryOperatorResult,modernize-avoid-c-arrays,cppcoreguidelines-avoid-c-arrays,cppcoreguidelines-pro-type-member-init %t -- -extra-arg=-Wunused-variable -- -I%S/Inputs/nolint

#include "trigger_warning.h"
void I(int& Out) {
Expand Down Expand Up @@ -96,6 +96,23 @@ MACRO_NOARG // NOLINT
#define MACRO_NOLINT class G { G(int i); }; // NOLINT
MACRO_NOLINT

// Check that we can suppress diagnostics about macro arguments (as opposed to
// diagnostics about the macro contents itself).
#define MACRO_SUPPRESS_DIAG_FOR_ARG_1(X) \
class X { \
X(int i); /* NOLINT(google-explicit-constructor) */ \
};

MACRO_SUPPRESS_DIAG_FOR_ARG_1(G1)

#define MACRO_SUPPRESS_DIAG_FOR_ARG_2(X) \
struct X { /* NOLINT(cppcoreguidelines-pro-type-member-init) */ \
int a = 0; \
int b; \
};

MACRO_SUPPRESS_DIAG_FOR_ARG_2(G2)

#define DOUBLE_MACRO MACRO(H) // NOLINT
DOUBLE_MACRO

Expand All @@ -116,4 +133,4 @@ int array2[10]; // NOLINT(cppcoreguidelines-avoid-c-arrays)
int array3[10]; // NOLINT(cppcoreguidelines-avoid-c-arrays,modernize-avoid-c-arrays)
int array4[10]; // NOLINT(*-avoid-c-arrays)

// CHECK-MESSAGES: Suppressed 34 warnings (34 NOLINT)
// CHECK-MESSAGES: Suppressed 36 warnings (36 NOLINT)

0 comments on commit 9ff4f2d

Please sign in to comment.