[PATCH] D91789: [clang-tidy] find/fix unneeded trailing semicolons in macros

[Nathan James via Phabricator via cfe-commits]

njames93 added inline comments.


================
Comment at: clang-tools-extra/clang-tidy/linuxkernel/MacroTrailingSemiCheck.cpp:15
+
+#include <stdio.h>
+
----------------
Any reason for including the c standard header?


================
Comment at: clang-tools-extra/clang-tidy/linuxkernel/MacroTrailingSemiCheck.cpp:102-105
+            for (std::vector<const MacroInfo *>::iterator it =
+                     SuspectMacros.begin();
+                 it != SuspectMacros.end(); ++it) {
+              const MacroInfo *MI = *it;
----------------
Could this be a range for loop.


================
Comment at: clang-tools-extra/clang-tidy/linuxkernel/MacroTrailingSemiCheck.cpp:106-111
+              auto TI = MI->tokens_begin();
+              for (; TI != MI->tokens_end(); TI++) {
+                SourceLocation L = TI->getLocation();
+                if (SpellingLoc == L)
+                  break;
+              }
----------------
Could this be refactored using `llvm::find_if`


================
Comment at: clang-tools-extra/clang-tidy/linuxkernel/MacroTrailingSemiCheck.cpp:116
+                SourceLocation EndLoc = Tok.getEndLoc();
+                auto H = FixItHint::CreateRemoval(SourceRange(FixLoc, EndLoc));
+                diag(FixLoc, "unneeded semicolon") << H;
----------------
No need for this to be a temporary


================
Comment at: clang-tools-extra/test/clang-tidy/checkers/linuxkernel-macro-trailing-semi.c:4
+#define M(a) a++;
+// CHECK-MESSAGES: warning: unneeded semicolon
+// CHECK-MESSAGES: note: FIX-IT applied suggested code changes
----------------
Please specify the location where this warning is emitted, you can check other test files but the typical format is
`// CHECK-MESSAGES: :[[@LINE-<Offset>]]:<Column>: warning: unneeded semicolon`.


================
Comment at: clang-tools-extra/test/clang-tidy/checkers/linuxkernel-macro-trailing-semi.c:5
+// CHECK-MESSAGES: warning: unneeded semicolon
+// CHECK-MESSAGES: note: FIX-IT applied suggested code changes
+
----------------
Don't need to check the FIX-IT note, however it is preferred if you check the fix-it was applied correctly.
`// CHECK-FIXES` will run FileCheck over the input file after clang-tidy has gone in and made changes then check for lines in the output.
I haven't read enough into where the fix is but an example would be:
`// CHECK-FIXES: #define M(a) a++{{$}}`
the `{{$}}` says regex for end of line


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D91789/new/

https://reviews.llvm.org/D91789