diff options
author | Salman Javed <mail@salmanjaved.org> | 2022-05-25 10:15:12 +1200 |
---|---|---|
committer | Tom Stellard <tstellar@redhat.com> | 2022-06-03 06:36:22 -0700 |
commit | 576e5b39ae4d4bab8c8f3456a0d6b37bb46dfdb7 (patch) | |
tree | 1a173eae9a1d7595cd59614e10404a05f2ef3179 | |
parent | 99b5eb2d3a61b55a61849cebabf1d5d66b4a13c4 (diff) | |
download | llvm-576e5b39ae4d4bab8c8f3456a0d6b37bb46dfdb7.tar.gz |
[clang-tidy] Fix #55134 (regression introduced by 5da7c04)
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
(cherry picked from commit 9ff4f2dfea632d63e3a57a88a2faa634aae5c772)
-rw-r--r-- | clang-tools-extra/clang-tidy/NoLintDirectiveHandler.cpp | 2 | ||||
-rw-r--r-- | clang-tools-extra/docs/ReleaseNotes.rst | 4 | ||||
-rw-r--r-- | clang-tools-extra/test/clang-tidy/infrastructure/nolint.cpp | 21 |
3 files changed, 24 insertions, 3 deletions
diff --git a/clang-tools-extra/clang-tidy/NoLintDirectiveHandler.cpp b/clang-tools-extra/clang-tidy/NoLintDirectiveHandler.cpp index 667ffdb5fab6..d482e49e0f6d 100644 --- a/clang-tools-extra/clang-tidy/NoLintDirectiveHandler.cpp +++ b/clang-tools-extra/clang-tidy/NoLintDirectiveHandler.cpp @@ -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; } diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst index a794983708cf..34b2fc0b588f 100644 --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -210,6 +210,10 @@ Improvements to clang-tidy - Added support for external plugin checks with `-load`. +- 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 ^^^^^^^^^^ diff --git a/clang-tools-extra/test/clang-tidy/infrastructure/nolint.cpp b/clang-tools-extra/test/clang-tidy/infrastructure/nolint.cpp index 9ef194232b6b..bc952ee03b5a 100644 --- a/clang-tools-extra/test/clang-tidy/infrastructure/nolint.cpp +++ b/clang-tools-extra/test/clang-tidy/infrastructure/nolint.cpp @@ -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) { @@ -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 @@ -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) |