salman-javed-nz created this revision. salman-javed-nz added reviewers: nridge, paulaltin. salman-javed-nz added a project: clang-tools-extra. Herald added subscribers: carlosgalvezp, xazax.hun. Herald added a project: All. salman-javed-nz requested review of this revision. Herald added a subscriber: cfe-commits.
5da7c04 <https://reviews.llvm.org/rG5da7c040030c4af72dcc21220f579098469c554e> 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 within macros such as the example below were not being detected: #define CREATE_STRUCT(name) \ struct name { /* NOLINT */ \ int a = 0; \ int b; \ }; CREATE_STRUCT(X) Revert to pre-patch behaviour and add test cases to cover this issue. Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D126138 Files: clang-tools-extra/clang-tidy/NoLintDirectiveHandler.cpp clang-tools-extra/test/clang-tidy/infrastructure/nolint.cpp Index: clang-tools-extra/test/clang-tidy/infrastructure/nolint.cpp =================================================================== --- clang-tools-extra/test/clang-tidy/infrastructure/nolint.cpp +++ clang-tools-extra/test/clang-tidy/infrastructure/nolint.cpp @@ -96,6 +96,13 @@ #define MACRO_NOLINT class G { G(int i); }; // NOLINT MACRO_NOLINT +#define MACRO_MULTILINE_NOLINT(X) \ +class X { \ + X(int i); /* NOLINT */ \ +}; +MACRO_MULTILINE_NOLINT(G1) +MACRO_MULTILINE_NOLINT(G2) + #define DOUBLE_MACRO MACRO(H) // NOLINT DOUBLE_MACRO @@ -116,4 +123,4 @@ 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) Index: clang-tools-extra/clang-tidy/NoLintDirectiveHandler.cpp =================================================================== --- clang-tools-extra/clang-tidy/NoLintDirectiveHandler.cpp +++ clang-tools-extra/clang-tidy/NoLintDirectiveHandler.cpp @@ -266,7 +266,7 @@ return true; if (!DiagLoc.isMacroID()) return false; - DiagLoc = SrcMgr.getImmediateMacroCallerLoc(DiagLoc); + DiagLoc = SrcMgr.getImmediateExpansionRange(DiagLoc).getBegin(); } return false; }
Index: clang-tools-extra/test/clang-tidy/infrastructure/nolint.cpp =================================================================== --- clang-tools-extra/test/clang-tidy/infrastructure/nolint.cpp +++ clang-tools-extra/test/clang-tidy/infrastructure/nolint.cpp @@ -96,6 +96,13 @@ #define MACRO_NOLINT class G { G(int i); }; // NOLINT MACRO_NOLINT +#define MACRO_MULTILINE_NOLINT(X) \ +class X { \ + X(int i); /* NOLINT */ \ +}; +MACRO_MULTILINE_NOLINT(G1) +MACRO_MULTILINE_NOLINT(G2) + #define DOUBLE_MACRO MACRO(H) // NOLINT DOUBLE_MACRO @@ -116,4 +123,4 @@ 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) Index: clang-tools-extra/clang-tidy/NoLintDirectiveHandler.cpp =================================================================== --- clang-tools-extra/clang-tidy/NoLintDirectiveHandler.cpp +++ clang-tools-extra/clang-tidy/NoLintDirectiveHandler.cpp @@ -266,7 +266,7 @@ return true; if (!DiagLoc.isMacroID()) return false; - DiagLoc = SrcMgr.getImmediateMacroCallerLoc(DiagLoc); + DiagLoc = SrcMgr.getImmediateExpansionRange(DiagLoc).getBegin(); } return false; }
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits