trixirt created this revision.
trixirt added reviewers: nickdesaulniers, alexfh, hokein, aaron.ballman.
trixirt added a project: clang-tools-extra.
Herald added subscribers: cfe-commits, xazax.hun, mgorny.
Herald added a project: clang.
trixirt requested review of this revision.
Cleaning up -Wextra-semi-stmt in the linux kernel shows a high
incidence of macros with trailing semicolons
#define M(a) a++; <-- clang-tidy fixes problem here
int f() {
int v = 0;
M(v); <-- clang reports problem here
return v;
}
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D91789
Files:
clang-tools-extra/clang-tidy/linuxkernel/CMakeLists.txt
clang-tools-extra/clang-tidy/linuxkernel/LinuxKernelTidyModule.cpp
clang-tools-extra/clang-tidy/linuxkernel/MacroTrailingSemiCheck.cpp
clang-tools-extra/clang-tidy/linuxkernel/MacroTrailingSemiCheck.h
clang-tools-extra/test/clang-tidy/checkers/linuxkernel-macro-trailing-semi.c
Index: clang-tools-extra/test/clang-tidy/checkers/linuxkernel-macro-trailing-semi.c
===================================================================
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/linuxkernel-macro-trailing-semi.c
@@ -0,0 +1,27 @@
+// RUN: %check_clang_tidy %s linuxkernel-macro-trailing-semi %t
+
+#define M(a) a++;
+// CHECK-MESSAGES: warning: unneeded semicolon
+// CHECK-MESSAGES: note: FIX-IT applied suggested code changes
+
+int f() {
+ int v = 0;
+ M(v);
+ return v;
+}
+
+// A corner case
+// An empty macro could conditionally contain another path so
+// do not warn or fix.
+#ifdef UNSET_CONDITION
+#define E() ;
+#else
+#define E()
+#endif
+#define N(a) a++;
+
+int g() {
+ int v = 0;
+ N(v) E();
+ return v;
+}
Index: clang-tools-extra/clang-tidy/linuxkernel/MacroTrailingSemiCheck.h
===================================================================
--- /dev/null
+++ clang-tools-extra/clang-tidy/linuxkernel/MacroTrailingSemiCheck.h
@@ -0,0 +1,40 @@
+//===--- MacroTrailingSemiCheck.h - clang-tidy ------------------*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_LINUXKERNEL_MACROTRAILINGSEMICHECK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_LINUXKERNEL_MACROTRAILINGSEMICHECK_H
+
+#include "../ClangTidyCheck.h"
+#include "clang/Lex/MacroInfo.h"
+#include <vector>
+
+namespace clang {
+namespace tidy {
+namespace linuxkernel {
+
+class MacroTrailingSemiCheck : public ClangTidyCheck {
+public:
+ MacroTrailingSemiCheck(StringRef Name, ClangTidyContext *Context)
+ : ClangTidyCheck(Name, Context) {}
+ void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+ void registerPPCallbacks(const SourceManager &SM, Preprocessor *PP,
+ Preprocessor *ModuleExpanderPP) override final;
+
+ void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+ void checkMacro(SourceManager &sourceMgr, const Token &MacroNameTok,
+ const MacroInfo *MI);
+
+private:
+ std::vector<const MacroInfo *> SuspectMacros;
+};
+
+} // namespace linuxkernel
+} // namespace tidy
+} // namespace clang
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_LINUXKERNEL_MACROTRAILINGSEMI_CHECK_H
Index: clang-tools-extra/clang-tidy/linuxkernel/MacroTrailingSemiCheck.cpp
===================================================================
--- /dev/null
+++ clang-tools-extra/clang-tidy/linuxkernel/MacroTrailingSemiCheck.cpp
@@ -0,0 +1,139 @@
+//===--- MacroTrailingSemiCheck.cpp - clang-tidy---------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "MacroTrailingSemiCheck.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/Lex/PPCallbacks.h"
+#include "clang/Lex/Preprocessor.h"
+
+#include <stdio.h>
+
+using namespace clang::ast_matchers;
+
+namespace clang {
+namespace tidy {
+namespace linuxkernel {
+
+class MacroTrailingSemiCheckPPCallbacks : public PPCallbacks {
+public:
+ MacroTrailingSemiCheckPPCallbacks(Preprocessor *PP,
+ MacroTrailingSemiCheck *Check)
+ : PP(PP), Check(Check) {}
+
+ void MacroDefined(const Token &MacroNameTok,
+ const MacroDirective *MD) override {
+ auto *MI = MD->getMacroInfo();
+
+ if (MI->isBuiltinMacro() || MI->isObjectLike())
+ return;
+ Check->checkMacro(PP->getSourceManager(), MacroNameTok, MI);
+ }
+
+private:
+ Preprocessor *PP;
+ MacroTrailingSemiCheck *Check;
+};
+
+void MacroTrailingSemiCheck::registerMatchers(MatchFinder *Finder) {
+ // From the reproducer
+ // #define M(a) a++;
+ // int f() {
+ // int v = 0;
+ // M(v);
+ // return v;
+ // }
+ // The AST
+ // `-FunctionDecl
+ // `-CompoundStmt <--- "comp", 'C'
+ // ...
+ // |-UnaryOperator
+ // | `-DeclRefExpr <-------- 'E'
+ // |-NullStmt <------ "null" 'N'
+ // ...
+ Finder->addMatcher(compoundStmt(has(nullStmt().bind("null"))).bind("comp"),
+ this);
+}
+
+void MacroTrailingSemiCheck::registerPPCallbacks(
+ const SourceManager &SM, Preprocessor *PP, Preprocessor *ModuleExpanderPP) {
+ ModuleExpanderPP->addPPCallbacks(
+ std::make_unique<MacroTrailingSemiCheckPPCallbacks>(ModuleExpanderPP,
+ this));
+}
+
+void MacroTrailingSemiCheck::check(const MatchFinder::MatchResult &Result) {
+ const auto *C = Result.Nodes.getNodeAs<CompoundStmt>("comp");
+ const auto *N = Result.Nodes.getNodeAs<NullStmt>("null");
+
+ auto Current = C->body_begin();
+ auto Next = Current;
+ Next++;
+ while (Next != C->body_end()) {
+
+ if (*Next == N) {
+ // This code has the same AST as the reproducer
+ //
+ // #define NOT_EMPTY(a) a++;
+ // #define EMPTY()
+ // void foo (int a) {
+ // NOT_EMPTY(a);
+ // EMPTY();
+ // }
+ //
+ // But we do not want to remove the ; because the
+ // macro may only be conditionally empty.
+ // ex/ the release version of a debug macro
+ //
+ // So check that the NullStmt does not have a
+ // leading empty macro.
+ if (!N->hasLeadingEmptyMacro() && N->getEndLoc().isValid()) {
+ if (const auto *E = dyn_cast<Expr>(*Current)) {
+ SourceLocation Loc = E->getEndLoc();
+ if (Loc.isMacroID()) {
+ auto &SM = Result.Context->getSourceManager();
+ FullSourceLoc SpellingLoc = FullSourceLoc(Loc, SM).getSpellingLoc();
+
+ for (std::vector<const MacroInfo *>::iterator it =
+ SuspectMacros.begin();
+ it != SuspectMacros.end(); ++it) {
+ const MacroInfo *MI = *it;
+ auto TI = MI->tokens_begin();
+ for (; TI != MI->tokens_end(); TI++) {
+ SourceLocation L = TI->getLocation();
+ if (SpellingLoc == L)
+ break;
+ }
+ if (TI != MI->tokens_end()) {
+ auto Tok = MI->getReplacementToken(MI->getNumTokens() - 1);
+ SourceLocation FixLoc = Tok.getLocation();
+ SourceLocation EndLoc = Tok.getEndLoc();
+ auto H = FixItHint::CreateRemoval(SourceRange(FixLoc, EndLoc));
+ diag(FixLoc, "unneeded semicolon") << H;
+ break;
+ }
+ }
+ }
+ }
+ }
+ }
+ Current = Next;
+ Next++;
+ }
+}
+void MacroTrailingSemiCheck::checkMacro(SourceManager &SM,
+ const Token &MacroNameTok,
+ const MacroInfo *MI) {
+ unsigned num = MI->getNumTokens();
+ if (num && MI->getReplacementToken(num - 1).is(tok::semi))
+ SuspectMacros.push_back(MI);
+}
+
+} // namespace linuxkernel
+} // namespace tidy
+} // namespace clang
Index: clang-tools-extra/clang-tidy/linuxkernel/LinuxKernelTidyModule.cpp
===================================================================
--- clang-tools-extra/clang-tidy/linuxkernel/LinuxKernelTidyModule.cpp
+++ clang-tools-extra/clang-tidy/linuxkernel/LinuxKernelTidyModule.cpp
@@ -9,6 +9,7 @@
#include "../ClangTidy.h"
#include "../ClangTidyModule.h"
#include "../ClangTidyModuleRegistry.h"
+#include "MacroTrailingSemiCheck.h"
#include "MustCheckErrsCheck.h"
#include "SwitchSemiCheck.h"
@@ -20,6 +21,8 @@
class LinuxKernelModule : public ClangTidyModule {
public:
void addCheckFactories(ClangTidyCheckFactories &CheckFactories) override {
+ CheckFactories.registerCheck<MacroTrailingSemiCheck>(
+ "linuxkernel-macro-trailing-semi");
CheckFactories.registerCheck<MustCheckErrsCheck>(
"linuxkernel-must-check-errs");
CheckFactories.registerCheck<SwitchSemiCheck>("linuxkernel-switch-semi");
Index: clang-tools-extra/clang-tidy/linuxkernel/CMakeLists.txt
===================================================================
--- clang-tools-extra/clang-tidy/linuxkernel/CMakeLists.txt
+++ clang-tools-extra/clang-tidy/linuxkernel/CMakeLists.txt
@@ -5,6 +5,7 @@
add_clang_library(clangTidyLinuxKernelModule
LinuxKernelTidyModule.cpp
+ MacroTrailingSemiCheck.cpp
MustCheckErrsCheck.cpp
SwitchSemiCheck.cpp
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
