Re: [PATCH] D24845: [clang-tidy] fix for NOLINT after macro expansion
This revision was automatically updated to reflect the committed changes.
Closed by commit rL282330: [clang-tidy] fix for NOLINT after macro expansion
(authored by mgehre).
Changed prior to commit:
https://reviews.llvm.org/D24845?vs=72214=72391#toc
Repository:
rL LLVM
https://reviews.llvm.org/D24845
Files:
clang-tools-extra/trunk/clang-tidy/ClangTidyDiagnosticConsumer.cpp
clang-tools-extra/trunk/test/clang-tidy/nolint.cpp
Index: clang-tools-extra/trunk/clang-tidy/ClangTidyDiagnosticConsumer.cpp
===
--- clang-tools-extra/trunk/clang-tidy/ClangTidyDiagnosticConsumer.cpp
+++ clang-tools-extra/trunk/clang-tidy/ClangTidyDiagnosticConsumer.cpp
@@ -294,12 +294,24 @@
return false;
}
+static bool LineIsMarkedWithNOLINTinMacro(SourceManager ,
+ 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 ) {
if (Info.getLocation().isValid() &&
DiagLevel != DiagnosticsEngine::Error &&
DiagLevel != DiagnosticsEngine::Fatal &&
- LineIsMarkedWithNOLINT(Diags->getSourceManager(), Info.getLocation())) {
+ LineIsMarkedWithNOLINTinMacro(Diags->getSourceManager(),
Info.getLocation())) {
++Context.Stats.ErrorsIgnoredNOLINT;
return;
}
Index: clang-tools-extra/trunk/test/clang-tidy/nolint.cpp
===
--- clang-tools-extra/trunk/test/clang-tidy/nolint.cpp
+++ clang-tools-extra/trunk/test/clang-tidy/nolint.cpp
@@ -13,4 +13,18 @@
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)
Index: clang-tools-extra/trunk/clang-tidy/ClangTidyDiagnosticConsumer.cpp
===
--- clang-tools-extra/trunk/clang-tidy/ClangTidyDiagnosticConsumer.cpp
+++ clang-tools-extra/trunk/clang-tidy/ClangTidyDiagnosticConsumer.cpp
@@ -294,12 +294,24 @@
return false;
}
+static bool LineIsMarkedWithNOLINTinMacro(SourceManager ,
+ 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 ) {
if (Info.getLocation().isValid() &&
DiagLevel != DiagnosticsEngine::Error &&
DiagLevel != DiagnosticsEngine::Fatal &&
- LineIsMarkedWithNOLINT(Diags->getSourceManager(), Info.getLocation())) {
+ LineIsMarkedWithNOLINTinMacro(Diags->getSourceManager(), Info.getLocation())) {
++Context.Stats.ErrorsIgnoredNOLINT;
return;
}
Index: clang-tools-extra/trunk/test/clang-tidy/nolint.cpp
===
--- clang-tools-extra/trunk/test/clang-tidy/nolint.cpp
+++ clang-tools-extra/trunk/test/clang-tidy/nolint.cpp
@@ -13,4 +13,18 @@
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
[PATCH] D24845: [clang-tidy] fix for NOLINT after macro expansion
mgehre created this revision.
mgehre added reviewers: alexfh, aaron.ballman, hokein.
mgehre added a subscriber: cfe-commits.
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.
https://reviews.llvm.org/D24845
Files:
clang-tidy/ClangTidyDiagnosticConsumer.cpp
test/clang-tidy/nolint.cpp
Index: test/clang-tidy/nolint.cpp
===
--- test/clang-tidy/nolint.cpp
+++ test/clang-tidy/nolint.cpp
@@ -13,4 +13,18 @@
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)
Index: clang-tidy/ClangTidyDiagnosticConsumer.cpp
===
--- clang-tidy/ClangTidyDiagnosticConsumer.cpp
+++ clang-tidy/ClangTidyDiagnosticConsumer.cpp
@@ -294,12 +294,24 @@
return false;
}
+static bool LineIsMarkedWithNOLINTinMacro(SourceManager ,
+ SourceLocation Loc) {
+ while (true) {
+if (LineIsMarkedWithNOLINT(SM, Loc))
+ return true;
+if (!Loc.isMacroID())
+ break;
+Loc = SM.getImmediateExpansionRange(Loc).first;
+ }
+ return false;
+}
+
void ClangTidyDiagnosticConsumer::HandleDiagnostic(
DiagnosticsEngine::Level DiagLevel, const Diagnostic ) {
if (Info.getLocation().isValid() &&
DiagLevel != DiagnosticsEngine::Error &&
DiagLevel != DiagnosticsEngine::Fatal &&
- LineIsMarkedWithNOLINT(Diags->getSourceManager(), Info.getLocation())) {
+ LineIsMarkedWithNOLINTinMacro(Diags->getSourceManager(),
Info.getLocation())) {
++Context.Stats.ErrorsIgnoredNOLINT;
return;
}
Index: test/clang-tidy/nolint.cpp
===
--- test/clang-tidy/nolint.cpp
+++ test/clang-tidy/nolint.cpp
@@ -13,4 +13,18 @@
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)
Index: clang-tidy/ClangTidyDiagnosticConsumer.cpp
===
--- clang-tidy/ClangTidyDiagnosticConsumer.cpp
+++ clang-tidy/ClangTidyDiagnosticConsumer.cpp
@@ -294,12 +294,24 @@
return false;
}
+static bool LineIsMarkedWithNOLINTinMacro(SourceManager ,
+ SourceLocation Loc) {
+ while (true) {
+if (LineIsMarkedWithNOLINT(SM, Loc))
+ return true;
+if (!Loc.isMacroID())
+ break;
+Loc = SM.getImmediateExpansionRange(Loc).first;
+ }
+ return false;
+}
+
void ClangTidyDiagnosticConsumer::HandleDiagnostic(
DiagnosticsEngine::Level DiagLevel, const Diagnostic ) {
if (Info.getLocation().isValid() &&
DiagLevel != DiagnosticsEngine::Error &&
DiagLevel != DiagnosticsEngine::Fatal &&
- LineIsMarkedWithNOLINT(Diags->getSourceManager(), Info.getLocation())) {
+ LineIsMarkedWithNOLINTinMacro(Diags->getSourceManager(), Info.getLocation())) {
++Context.Stats.ErrorsIgnoredNOLINT;
return;
}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
