Author: mgehre Date: Sat Sep 24 11:06:53 2016 New Revision: 282330 URL: http://llvm.org/viewvc/llvm-project?rev=282330&view=rev Log: [clang-tidy] fix for NOLINT after macro expansion
Summary: When having ``` c++ #define MACRO code-with-warning MACRO; // NOLINT ``` clang-tidy would still show the warning, because it searched for "NOLINT" only in the first line, not on the second. This caused e.g. https://llvm.org/bugs/show_bug.cgi?id=29089 (where the macro was defined in a system header). See also the added test cases. Now clang-tidy looks at the line of macro invocation and every line of macro definition for a NOLINT comment. Reviewers: alexfh, aaron.ballman, hokein Subscribers: cfe-commits Differential Revision: https://reviews.llvm.org/D24845 Modified: clang-tools-extra/trunk/clang-tidy/ClangTidyDiagnosticConsumer.cpp clang-tools-extra/trunk/test/clang-tidy/nolint.cpp Modified: clang-tools-extra/trunk/clang-tidy/ClangTidyDiagnosticConsumer.cpp URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/ClangTidyDiagnosticConsumer.cpp?rev=282330&r1=282329&r2=282330&view=diff ============================================================================== --- clang-tools-extra/trunk/clang-tidy/ClangTidyDiagnosticConsumer.cpp (original) +++ clang-tools-extra/trunk/clang-tidy/ClangTidyDiagnosticConsumer.cpp Sat Sep 24 11:06:53 2016 @@ -294,12 +294,24 @@ static bool LineIsMarkedWithNOLINT(Sourc return false; } +static bool LineIsMarkedWithNOLINTinMacro(SourceManager &SM, + SourceLocation Loc) { + while (true) { + if (LineIsMarkedWithNOLINT(SM, Loc)) + return true; + if (!Loc.isMacroID()) + return false; + Loc = SM.getImmediateExpansionRange(Loc).first; + } + return false; +} + void ClangTidyDiagnosticConsumer::HandleDiagnostic( DiagnosticsEngine::Level DiagLevel, const Diagnostic &Info) { if (Info.getLocation().isValid() && DiagLevel != DiagnosticsEngine::Error && DiagLevel != DiagnosticsEngine::Fatal && - LineIsMarkedWithNOLINT(Diags->getSourceManager(), Info.getLocation())) { + LineIsMarkedWithNOLINTinMacro(Diags->getSourceManager(), Info.getLocation())) { ++Context.Stats.ErrorsIgnoredNOLINT; return; } Modified: clang-tools-extra/trunk/test/clang-tidy/nolint.cpp URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-tidy/nolint.cpp?rev=282330&r1=282329&r2=282330&view=diff ============================================================================== --- clang-tools-extra/trunk/test/clang-tidy/nolint.cpp (original) +++ clang-tools-extra/trunk/test/clang-tidy/nolint.cpp Sat Sep 24 11:06:53 2016 @@ -13,4 +13,18 @@ void f() { int j; // NOLINT } -// CHECK-MESSAGES: Suppressed 3 warnings (3 NOLINT) +#define MACRO(X) class X { X(int i); }; +MACRO(D) +// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: single-argument constructors must be marked explicit +MACRO(E) // NOLINT + +#define MACRO_NOARG class F { F(int i); }; +MACRO_NOARG // NOLINT + +#define MACRO_NOLINT class G { G(int i); }; // NOLINT +MACRO_NOLINT + +#define DOUBLE_MACRO MACRO(H) // NOLINT +DOUBLE_MACRO + +// CHECK-MESSAGES: Suppressed 7 warnings (7 NOLINT) _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits