Skip to content

Commit b24a881

Browse files
committed
[clang-tidy] Add MLIR check for old op builder usage.
Moving towards new create method invocation, add check to flag old usage.
1 parent 22994ed commit b24a881

File tree

8 files changed

+253
-0
lines changed

8 files changed

+253
-0
lines changed

clang-tools-extra/clang-tidy/CMakeLists.txt

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -66,6 +66,7 @@ add_subdirectory(linuxkernel)
6666
add_subdirectory(llvm)
6767
add_subdirectory(llvmlibc)
6868
add_subdirectory(misc)
69+
add_subdirectory(mlir)
6970
add_subdirectory(modernize)
7071
if(CLANG_TIDY_ENABLE_STATIC_ANALYZER)
7172
add_subdirectory(mpi)
@@ -93,6 +94,7 @@ set(ALL_CLANG_TIDY_CHECKS
9394
clangTidyLLVMModule
9495
clangTidyLLVMLibcModule
9596
clangTidyMiscModule
97+
clangTidyMLIRModule
9698
clangTidyModernizeModule
9799
clangTidyObjCModule
98100
clangTidyOpenMPModule

clang-tools-extra/clang-tidy/ClangTidyForceLinker.h

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -94,6 +94,11 @@ extern volatile int MiscModuleAnchorSource;
9494
static int LLVM_ATTRIBUTE_UNUSED MiscModuleAnchorDestination =
9595
MiscModuleAnchorSource;
9696

97+
// This anchor is used to force the linker to link the MLIRModule.
98+
extern volatile int MLIRModuleAnchorSource;
99+
static int LLVM_ATTRIBUTE_UNUSED MLIRModuleAnchorDestination =
100+
MLIRModuleAnchorSource;
101+
97102
// This anchor is used to force the linker to link the ModernizeModule.
98103
extern volatile int ModernizeModuleAnchorSource;
99104
static int LLVM_ATTRIBUTE_UNUSED ModernizeModuleAnchorDestination =
Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
set(LLVM_LINK_COMPONENTS
2+
FrontendOpenMP
3+
Support
4+
)
5+
6+
add_clang_library(clangTidyMLIRModule STATIC
7+
MLIRTidyModule.cpp
8+
OpBuilderCheck.cpp
9+
10+
LINK_LIBS
11+
clangTidy
12+
clangTidyReadabilityModule
13+
clangTidyUtils
14+
clangTransformer
15+
16+
DEPENDS
17+
omp_gen
18+
ClangDriverOptions
19+
)
20+
21+
clang_target_link_libraries(clangTidyMLIRModule
22+
PRIVATE
23+
clangAST
24+
clangASTMatchers
25+
clangBasic
26+
clangLex
27+
clangTooling
28+
)
Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
1+
//===--- MLIRTidyModule.cpp - clang-tidy ----------------------------------===//
2+
//
3+
// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
4+
// See https://llvm.org/LICENSE.txt for license information.
5+
// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
6+
//
7+
//===----------------------------------------------------------------------===//
8+
9+
#include "../ClangTidy.h"
10+
#include "../ClangTidyModule.h"
11+
#include "../ClangTidyModuleRegistry.h"
12+
#include "OpBuilderCheck.h"
13+
14+
namespace clang::tidy {
15+
namespace mlir_check {
16+
17+
class MLIRModule : public ClangTidyModule {
18+
public:
19+
void addCheckFactories(ClangTidyCheckFactories &CheckFactories) override {
20+
CheckFactories.registerCheck<OpBuilderCheck>("mlir-op-builder");
21+
}
22+
23+
ClangTidyOptions getModuleOptions() override {
24+
ClangTidyOptions Options;
25+
return Options;
26+
}
27+
};
28+
29+
// Register the ModuleModule using this statically initialized variable.
30+
static ClangTidyModuleRegistry::Add<MLIRModule> X("mlir-module",
31+
"Adds MLIR lint checks.");
32+
33+
} // namespace mlir_check
34+
35+
// This anchor is used to force the linker to link in the generated object file
36+
// and thus register the MLIRModule.
37+
volatile int MLIRModuleAnchorSource = 0; // NOLINT(misc-use-internal-linkage)
38+
39+
} // namespace clang::tidy
Lines changed: 102 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,102 @@
1+
//===--- OpBuilderCheck.cpp - clang-tidy ----------------------------------===//
2+
//
3+
// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
4+
// See https://llvm.org/LICENSE.txt for license information.
5+
// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
6+
//
7+
//===----------------------------------------------------------------------===//
8+
9+
#include "OpBuilderCheck.h"
10+
#include "clang/ASTMatchers/ASTMatchers.h"
11+
#include "clang/Basic/LLVM.h"
12+
#include "clang/Lex/Lexer.h"
13+
#include "clang/Tooling/Transformer/RangeSelector.h"
14+
#include "clang/Tooling/Transformer/RewriteRule.h"
15+
#include "clang/Tooling/Transformer/SourceCode.h"
16+
#include "clang/Tooling/Transformer/Stencil.h"
17+
#include "llvm/Support/Error.h"
18+
19+
namespace clang::tidy::mlir_check {
20+
namespace {
21+
22+
using namespace ::clang::ast_matchers; // NOLINT: Too many names.
23+
using namespace ::clang::transformer; // NOLINT: Too many names.
24+
25+
class TypeAsWrittenStencil : public StencilInterface {
26+
public:
27+
explicit TypeAsWrittenStencil(std::string S) : id(std::move(S)) {}
28+
std::string toString() const override {
29+
return (llvm::Twine("TypeAsWritten(\"") + id + "\")").str();
30+
}
31+
32+
llvm::Error eval(const MatchFinder::MatchResult &match,
33+
std::string *result) const override {
34+
auto n = node(id)(match);
35+
if (!n)
36+
return n.takeError();
37+
auto srcRange = n->getAsRange();
38+
if (srcRange.isInvalid()) {
39+
return llvm::make_error<llvm::StringError>(llvm::errc::invalid_argument,
40+
"srcRange is invalid");
41+
}
42+
auto range = n->getTokenRange(srcRange);
43+
auto nextToken = [&](std::optional<Token> token) {
44+
if (!token)
45+
return token;
46+
return clang::Lexer::findNextToken(token->getLocation(),
47+
*match.SourceManager,
48+
match.Context->getLangOpts());
49+
};
50+
auto lessToken = clang::Lexer::findNextToken(
51+
range.getBegin(), *match.SourceManager, match.Context->getLangOpts());
52+
while (lessToken && lessToken->getKind() != clang::tok::less) {
53+
lessToken = nextToken(lessToken);
54+
}
55+
if (!lessToken) {
56+
return llvm::make_error<llvm::StringError>(llvm::errc::invalid_argument,
57+
"missing '<' token");
58+
}
59+
std::optional<Token> endToken = nextToken(lessToken);
60+
for (auto greaterToken = nextToken(endToken);
61+
greaterToken && greaterToken->getKind() != clang::tok::greater;
62+
greaterToken = nextToken(greaterToken)) {
63+
endToken = greaterToken;
64+
}
65+
if (!endToken) {
66+
return llvm::make_error<llvm::StringError>(llvm::errc::invalid_argument,
67+
"missing '>' token");
68+
}
69+
*result += clang::tooling::getText(
70+
CharSourceRange::getTokenRange(lessToken->getEndLoc(),
71+
endToken->getLastLoc()),
72+
*match.Context);
73+
return llvm::Error::success();
74+
}
75+
std::string id;
76+
};
77+
78+
Stencil typeAsWritten(StringRef Id) {
79+
// Using this instead of `describe` so that we get the exact same spelling.
80+
return std::make_shared<TypeAsWrittenStencil>(std::string(Id));
81+
}
82+
83+
RewriteRuleWith<std::string> OpBuilderCheckRule() {
84+
return makeRule(
85+
cxxMemberCallExpr(
86+
on(expr(hasType(
87+
cxxRecordDecl(isSameOrDerivedFrom("::mlir::OpBuilder"))))
88+
.bind("builder")),
89+
callee(cxxMethodDecl(hasTemplateArgument(0, templateArgument()))),
90+
callee(cxxMethodDecl(hasName("create"))))
91+
.bind("call"),
92+
changeTo(cat(typeAsWritten("call"), "::create(", node("builder"), ", ",
93+
callArgs("call"), ")")),
94+
cat("Use OpType::create(builder, ...) instead of "
95+
"builder.create<OpType>(...)"));
96+
}
97+
} // namespace
98+
99+
OpBuilderCheck::OpBuilderCheck(StringRef Name, ClangTidyContext *Context)
100+
: TransformerClangTidyCheck(OpBuilderCheckRule(), Name, Context) {}
101+
102+
} // namespace clang::tidy::mlir_check
Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MLIR_OPBUILDERCHECK_H
2+
#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MLIR_OPBUILDERCHECK_H
3+
4+
#include "../utils/TransformerClangTidyCheck.h"
5+
6+
namespace clang::tidy::mlir_check {
7+
8+
/// Checks for uses of `OpBuilder::create<T>` and suggests using `T::create`
9+
/// instead.
10+
class OpBuilderCheck : public utils::TransformerClangTidyCheck {
11+
public:
12+
OpBuilderCheck(StringRef Name, ClangTidyContext *Context);
13+
14+
bool isLanguageVersionSupported(const LangOptions &LangOpts) const override {
15+
return getLangOpts().CPlusPlus;
16+
}
17+
};
18+
19+
} // namespace clang::tidy::mlir_check
20+
21+
#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MLIR_OPBUILDERCHECK_H

clang-tools-extra/docs/ReleaseNotes.rst

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -162,6 +162,11 @@ New checks
162162
Finds uses of ``std::lock_guard`` and suggests replacing them with C++17's
163163
alternative ``std::scoped_lock``.
164164

165+
- New :doc:`mlir-op-builder
166+
<clang-tidy/checks/mlir/op-builder>` check.
167+
168+
Flags usage of old OpBuilder format.
169+
165170
- New :doc:`portability-avoid-pragma-once
166171
<clang-tidy/checks/portability/avoid-pragma-once>` check.
167172

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,51 @@
1+
// RUN: %check_clang_tidy --match-partial-fixes %s mlir-op-builder %t
2+
3+
namespace mlir {
4+
class Location {};
5+
class OpBuilder {
6+
public:
7+
template <typename OpTy, typename... Args>
8+
OpTy create(Location location, Args &&...args) {
9+
return OpTy(args...);
10+
}
11+
Location getUnknownLoc() { return Location(); }
12+
};
13+
class ImplicitLocOpBuilder : public OpBuilder {
14+
public:
15+
template <typename OpTy, typename... Args>
16+
OpTy create(Args &&...args) {
17+
return OpTy(args...);
18+
}
19+
};
20+
struct ModuleOp {
21+
ModuleOp() {}
22+
static ModuleOp create(OpBuilder &builder, Location location) {
23+
return ModuleOp();
24+
}
25+
};
26+
struct NamedOp {
27+
NamedOp(const char* name) {}
28+
static NamedOp create(OpBuilder &builder, Location location, const char* name) {
29+
return NamedOp(name);
30+
}
31+
};
32+
} // namespace mlir
33+
34+
void f() {
35+
mlir::OpBuilder builder;
36+
// CHECK-MESSAGES: :[[@LINE+2]]:3: warning: Use OpType::create(builder, ...) instead of builder.create<OpType>(...) [mlir-op-builder]
37+
// CHECK-FIXES: mlir:: ModuleOp::create(builder, builder.getUnknownLoc())
38+
builder.create<mlir:: ModuleOp>(builder.getUnknownLoc());
39+
40+
using mlir::NamedOp;
41+
// CHECK-MESSAGES: :[[@LINE+2]]:3: warning: Use OpType::create(builder, ...) instead of builder.create<OpType>(...) [mlir-op-builder]
42+
// CHECK-FIXES: NamedOp::create(builder, builder.getUnknownLoc(), "baz")
43+
builder.create<NamedOp>(builder.getUnknownLoc(), "baz");
44+
45+
mlir::ImplicitLocOpBuilder ib;
46+
// Note: extra space in the case where there is no other arguments. Could be
47+
// improved, but also clang-format will do that just post.
48+
// CHECK-MESSAGES: :[[@LINE+2]]:3: warning: Use OpType::create(builder, ...) instead of builder.create<OpType>(...) [mlir-op-builder]
49+
// CHECK-FIXES: mlir::ModuleOp::create(ib )
50+
ib.create<mlir::ModuleOp>();
51+
}

0 commit comments

Comments
 (0)