https://github.com/jpienaar updated https://github.com/llvm/llvm-project/pull/159423
>From f23c42bea591a122ab366a749c4959e44b405d50 Mon Sep 17 00:00:00 2001 From: Jacques Pienaar <[email protected]> Date: Wed, 17 Sep 2025 18:53:01 +0000 Subject: [PATCH 01/11] [clang-tidy][mlir] Expand to cover pointer of builder Previously this only checked for OpBuilder usage, but it could also be invoked via pointer. Also change how call range is calculated to avoid false overlaps which limits rewriting builder calls inside arguments of builder calls. --- .../llvm/UseNewMLIROpBuilderCheck.cpp | 71 ++++++++++--------- .../checkers/llvm/use-new-mlir-op-builder.cpp | 38 +++++++++- 2 files changed, 75 insertions(+), 34 deletions(-) diff --git a/clang-tools-extra/clang-tidy/llvm/UseNewMLIROpBuilderCheck.cpp b/clang-tools-extra/clang-tidy/llvm/UseNewMLIROpBuilderCheck.cpp index 0d81b9a9e38ca..8705c10f5c646 100644 --- a/clang-tools-extra/clang-tidy/llvm/UseNewMLIROpBuilderCheck.cpp +++ b/clang-tools-extra/clang-tidy/llvm/UseNewMLIROpBuilderCheck.cpp @@ -23,13 +23,11 @@ namespace { using namespace ::clang::ast_matchers; using namespace ::clang::transformer; -EditGenerator rewrite(RangeSelector Call, RangeSelector Builder, - RangeSelector CallArgs) { +EditGenerator rewrite(RangeSelector Call, RangeSelector Builder) { // This is using an EditGenerator rather than ASTEdit as we want to warn even // if in macro. - return [Call = std::move(Call), Builder = std::move(Builder), - CallArgs = - std::move(CallArgs)](const MatchFinder::MatchResult &Result) + return [Call = std::move(Call), + Builder = std::move(Builder)](const MatchFinder::MatchResult &Result) -> Expected<SmallVector<transformer::Edit, 1>> { Expected<CharSourceRange> CallRange = Call(Result); if (!CallRange) @@ -54,7 +52,7 @@ EditGenerator rewrite(RangeSelector Call, RangeSelector Builder, auto NextToken = [&](std::optional<Token> CurrentToken) { if (!CurrentToken) return CurrentToken; - if (CurrentToken->getEndLoc() >= CallRange->getEnd()) + if (CurrentToken->is(clang::tok::eof)) return std::optional<Token>(); return clang::Lexer::findNextToken(CurrentToken->getLocation(), SM, LangOpts); @@ -68,9 +66,10 @@ EditGenerator rewrite(RangeSelector Call, RangeSelector Builder, return llvm::make_error<llvm::StringError>(llvm::errc::invalid_argument, "missing '<' token"); } + std::optional<Token> EndToken = NextToken(LessToken); - for (std::optional<Token> GreaterToken = NextToken(EndToken); - GreaterToken && GreaterToken->getKind() != clang::tok::greater; + std::optional<Token> GreaterToken = NextToken(EndToken); + for (; GreaterToken && GreaterToken->getKind() != clang::tok::greater; GreaterToken = NextToken(GreaterToken)) { EndToken = GreaterToken; } @@ -79,12 +78,21 @@ EditGenerator rewrite(RangeSelector Call, RangeSelector Builder, "missing '>' token"); } + std::optional<Token> ArgStart = NextToken(GreaterToken); + if (!ArgStart || ArgStart->getKind() != clang::tok::l_paren) { + return llvm::make_error<llvm::StringError>(llvm::errc::invalid_argument, + "missing '(' token"); + } + std::optional<Token> Arg = NextToken(ArgStart); + if (!Arg) { + return llvm::make_error<llvm::StringError>(llvm::errc::invalid_argument, + "unexpected end of file"); + } + bool hasArgs = Arg->getKind() != clang::tok::r_paren; + Expected<CharSourceRange> BuilderRange = Builder(Result); if (!BuilderRange) return BuilderRange.takeError(); - Expected<CharSourceRange> CallArgsRange = CallArgs(Result); - if (!CallArgsRange) - return CallArgsRange.takeError(); // Helper for concatting below. auto GetText = [&](const CharSourceRange &Range) { @@ -93,18 +101,19 @@ EditGenerator rewrite(RangeSelector Call, RangeSelector Builder, Edit Replace; Replace.Kind = EditKind::Range; - Replace.Range = *CallRange; - std::string CallArgsStr; - // Only emit args if there are any. - if (auto CallArgsText = GetText(*CallArgsRange).ltrim(); - !CallArgsText.rtrim().empty()) { - CallArgsStr = llvm::formatv(", {}", CallArgsText); + Replace.Range.setBegin(CallRange->getBegin()); + Replace.Range.setEnd(ArgStart->getEndLoc()); + const Expr *BuilderExpr = Result.Nodes.getNodeAs<Expr>("builder"); + std::string BuilderText = GetText(*BuilderRange).str(); + if (BuilderExpr->getType()->isPointerType()) { + BuilderText = BuilderExpr->isImplicitCXXThis() + ? "*this" + : llvm::formatv("*{}", BuilderText).str(); } - Replace.Replacement = - llvm::formatv("{}::create({}{})", - GetText(CharSourceRange::getTokenRange( - LessToken->getEndLoc(), EndToken->getLastLoc())), - GetText(*BuilderRange), CallArgsStr); + StringRef OpType = GetText(CharSourceRange::getTokenRange( + LessToken->getEndLoc(), EndToken->getLastLoc())); + Replace.Replacement = llvm::formatv("{}::create({}{}", OpType, BuilderText, + hasArgs ? ", " : ""); return SmallVector<Edit, 1>({Replace}); }; @@ -114,19 +123,17 @@ RewriteRuleWith<std::string> useNewMlirOpBuilderCheckRule() { Stencil message = cat("use 'OpType::create(builder, ...)' instead of " "'builder.create<OpType>(...)'"); // Match a create call on an OpBuilder. - ast_matchers::internal::Matcher<Stmt> base = - cxxMemberCallExpr( - on(expr(hasType( - cxxRecordDecl(isSameOrDerivedFrom("::mlir::OpBuilder")))) - .bind("builder")), - callee(cxxMethodDecl(hasTemplateArgument(0, templateArgument()))), - callee(cxxMethodDecl(hasName("create")))) - .bind("call"); + auto BuilderType = cxxRecordDecl(isSameOrDerivedFrom("::mlir::OpBuilder")); + ast_matchers::internal::Matcher<Stmt> base = cxxMemberCallExpr( + on(expr(anyOf(hasType(BuilderType), hasType(pointsTo(BuilderType)))) + .bind("builder")), + callee(expr().bind("call")), + callee(cxxMethodDecl(hasTemplateArgument(0, templateArgument()))), + callee(cxxMethodDecl(hasName("create")))); return applyFirst( // Attempt rewrite given an lvalue builder, else just warn. {makeRule(cxxMemberCallExpr(unless(on(cxxTemporaryObjectExpr())), base), - rewrite(node("call"), node("builder"), callArgs("call")), - message), + rewrite(node("call"), node("builder")), message), makeRule(base, noopEdit(node("call")), message)}); } } // namespace diff --git a/clang-tools-extra/test/clang-tidy/checkers/llvm/use-new-mlir-op-builder.cpp b/clang-tools-extra/test/clang-tidy/checkers/llvm/use-new-mlir-op-builder.cpp index 0971a1611e3cb..1fbbf0a76c42a 100644 --- a/clang-tools-extra/test/clang-tidy/checkers/llvm/use-new-mlir-op-builder.cpp +++ b/clang-tools-extra/test/clang-tidy/checkers/llvm/use-new-mlir-op-builder.cpp @@ -2,6 +2,7 @@ namespace mlir { class Location {}; +class Value {}; class OpBuilder { public: template <typename OpTy, typename... Args> @@ -28,6 +29,13 @@ struct NamedOp { static NamedOp create(OpBuilder &builder, Location location, const char* name) { return NamedOp(name); } + Value getResult() { return Value(); } +}; +struct OperandOp { + OperandOp(Value val) {} + static OperandOp create(OpBuilder &builder, Location location, Value val) { + return OperandOp(val); + } }; } // namespace mlir @@ -40,6 +48,15 @@ void g(mlir::OpBuilder &b) { b.create<T>(b.getUnknownLoc(), "gaz"); } +class CustomBuilder : public mlir::ImplicitLocOpBuilder { +public: + mlir::NamedOp f(const char *name) { + // CHECK-MESSAGES: :[[@LINE+2]]:12: warning: use 'OpType::create(builder, ...)' + // CHECK-FIXES: NamedOp::create(*this, name); + return create<mlir::NamedOp>(name); + } +}; + void f() { mlir::OpBuilder builder; // CHECK-MESSAGES: :[[@LINE+2]]:3: warning: use 'OpType::create(builder, ...)' instead of 'builder.create<OpType>(...)' [llvm-use-new-mlir-op-builder] @@ -47,6 +64,8 @@ void f() { builder.create<mlir:: ModuleOp>(builder.getUnknownLoc()); using mlir::NamedOp; + using mlir::OperandOp; + // CHECK-MESSAGES: :[[@LINE+2]]:3: warning: use 'OpType::create(builder, ...)' instead of 'builder.create<OpType>(...)' [llvm-use-new-mlir-op-builder] // CHECK-FIXES: NamedOp::create(builder, builder.getUnknownLoc(), "baz") builder.create<NamedOp>(builder.getUnknownLoc(), "baz"); @@ -56,7 +75,7 @@ void f() { // CHECK-FIXES: builder.getUnknownLoc(), // CHECK-FIXES: "caz") builder. - create<NamedOp>( + create<NamedOp> ( builder.getUnknownLoc(), "caz"); @@ -67,10 +86,25 @@ void f() { mlir::ImplicitLocOpBuilder ib; // CHECK-MESSAGES: :[[@LINE+2]]:3: warning: use 'OpType::create(builder, ...)' instead of 'builder.create<OpType>(...)' [llvm-use-new-mlir-op-builder] - // CHECK-FIXES: mlir::ModuleOp::create(ib) + // CHECK-FIXES: mlir::ModuleOp::create(ib ) ib.create<mlir::ModuleOp>( ); // CHECK-MESSAGES: :[[@LINE+2]]:3: warning: use 'OpType::create(builder, ...)' instead of 'builder.create<OpType>(...)' [llvm-use-new-mlir-op-builder] // CHECK-FIXES: mlir::OpBuilder().create<mlir::ModuleOp>(builder.getUnknownLoc()); mlir::OpBuilder().create<mlir::ModuleOp>(builder.getUnknownLoc()); + + auto *p = &builder; + // CHECK-MESSAGES: :[[@LINE+2]]:3: warning: use 'OpType::create(builder, ...)' + // CHECK-FIXES: NamedOp::create(*p, builder.getUnknownLoc(), "eaz") + p->create<NamedOp>(builder.getUnknownLoc(), "eaz"); + + CustomBuilder cb; + cb.f("faz"); + + // CHECK-MESSAGES: :[[@LINE+4]]:3: warning: use 'OpType::create(builder, ...)' instead of 'builder.create<OpType>(...)' [llvm-use-new-mlir-op-builder] + // CHECK-FIXES: OperandOp::create(builder, builder.getUnknownLoc(), + // CHECK-MESSAGES: :[[@LINE+3]]:5: warning: use 'OpType::create(builder, ...)' instead of 'builder.create<OpType>(...)' [llvm-use-new-mlir-op-builder] + // CHECK-FIXES: NamedOp::create(builder, + builder.create<OperandOp>(builder.getUnknownLoc(), + builder.create<NamedOp>(builder.getUnknownLoc(), "gaz").getResult()); } >From b2fb3e853c93e49cf6f6a51a6bb4a1fddbc37cac Mon Sep 17 00:00:00 2001 From: Jacques Pienaar <[email protected]> Date: Thu, 18 Sep 2025 20:48:40 -0700 Subject: [PATCH 02/11] Apply suggestions from code review Co-authored-by: EugeneZelenko <[email protected]> --- .../clang-tidy/llvm/UseNewMLIROpBuilderCheck.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/clang-tools-extra/clang-tidy/llvm/UseNewMLIROpBuilderCheck.cpp b/clang-tools-extra/clang-tidy/llvm/UseNewMLIROpBuilderCheck.cpp index 8705c10f5c646..1b4360207b53b 100644 --- a/clang-tools-extra/clang-tidy/llvm/UseNewMLIROpBuilderCheck.cpp +++ b/clang-tools-extra/clang-tidy/llvm/UseNewMLIROpBuilderCheck.cpp @@ -88,7 +88,7 @@ EditGenerator rewrite(RangeSelector Call, RangeSelector Builder) { return llvm::make_error<llvm::StringError>(llvm::errc::invalid_argument, "unexpected end of file"); } - bool hasArgs = Arg->getKind() != clang::tok::r_paren; + const bool hasArgs = Arg->getKind() != clang::tok::r_paren; Expected<CharSourceRange> BuilderRange = Builder(Result); if (!BuilderRange) @@ -110,7 +110,7 @@ EditGenerator rewrite(RangeSelector Call, RangeSelector Builder) { ? "*this" : llvm::formatv("*{}", BuilderText).str(); } - StringRef OpType = GetText(CharSourceRange::getTokenRange( + const StringRef OpType = GetText(CharSourceRange::getTokenRange( LessToken->getEndLoc(), EndToken->getLastLoc())); Replace.Replacement = llvm::formatv("{}::create({}{}", OpType, BuilderText, hasArgs ? ", " : ""); >From 540e4af4df871e0d7ce06357fd8a942e6824b4ce Mon Sep 17 00:00:00 2001 From: Jacques Pienaar <[email protected]> Date: Tue, 23 Sep 2025 13:46:24 +0200 Subject: [PATCH 03/11] Update clang-tools-extra/test/clang-tidy/checkers/llvm/use-new-mlir-op-builder.cpp Co-authored-by: Victor Chernyakin <[email protected]> --- .../test/clang-tidy/checkers/llvm/use-new-mlir-op-builder.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/clang-tools-extra/test/clang-tidy/checkers/llvm/use-new-mlir-op-builder.cpp b/clang-tools-extra/test/clang-tidy/checkers/llvm/use-new-mlir-op-builder.cpp index 1fbbf0a76c42a..ea58a6c93e324 100644 --- a/clang-tools-extra/test/clang-tidy/checkers/llvm/use-new-mlir-op-builder.cpp +++ b/clang-tools-extra/test/clang-tidy/checkers/llvm/use-new-mlir-op-builder.cpp @@ -52,7 +52,7 @@ class CustomBuilder : public mlir::ImplicitLocOpBuilder { public: mlir::NamedOp f(const char *name) { // CHECK-MESSAGES: :[[@LINE+2]]:12: warning: use 'OpType::create(builder, ...)' - // CHECK-FIXES: NamedOp::create(*this, name); + // CHECK-FIXES: mlir::NamedOp::create(*this, name); return create<mlir::NamedOp>(name); } }; >From 87fc9c02176ac538a17ef6a6faba4eddfdb515b8 Mon Sep 17 00:00:00 2001 From: Jacques Pienaar <[email protected]> Date: Tue, 23 Sep 2025 14:03:37 +0200 Subject: [PATCH 04/11] Merge two cxxMethodDecl checks --- .../clang-tidy/llvm/UseNewMLIROpBuilderCheck.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/clang-tools-extra/clang-tidy/llvm/UseNewMLIROpBuilderCheck.cpp b/clang-tools-extra/clang-tidy/llvm/UseNewMLIROpBuilderCheck.cpp index 1b4360207b53b..09530ff69e404 100644 --- a/clang-tools-extra/clang-tidy/llvm/UseNewMLIROpBuilderCheck.cpp +++ b/clang-tools-extra/clang-tidy/llvm/UseNewMLIROpBuilderCheck.cpp @@ -128,8 +128,8 @@ RewriteRuleWith<std::string> useNewMlirOpBuilderCheckRule() { on(expr(anyOf(hasType(BuilderType), hasType(pointsTo(BuilderType)))) .bind("builder")), callee(expr().bind("call")), - callee(cxxMethodDecl(hasTemplateArgument(0, templateArgument()))), - callee(cxxMethodDecl(hasName("create")))); + callee(cxxMethodDecl(hasTemplateArgument(0, templateArgument()), + hasName("create")))); return applyFirst( // Attempt rewrite given an lvalue builder, else just warn. {makeRule(cxxMemberCallExpr(unless(on(cxxTemporaryObjectExpr())), base), >From 0ce25d1b77a4dfa4588355a18cd54f0aa99b1819 Mon Sep 17 00:00:00 2001 From: Jacques Pienaar <[email protected]> Date: Tue, 23 Sep 2025 14:05:26 +0200 Subject: [PATCH 05/11] Fix clang-tidy flags --- .../clang-tidy/llvm/UseNewMLIROpBuilderCheck.cpp | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/clang-tools-extra/clang-tidy/llvm/UseNewMLIROpBuilderCheck.cpp b/clang-tools-extra/clang-tidy/llvm/UseNewMLIROpBuilderCheck.cpp index 09530ff69e404..2dc2a7b971f09 100644 --- a/clang-tools-extra/clang-tidy/llvm/UseNewMLIROpBuilderCheck.cpp +++ b/clang-tools-extra/clang-tidy/llvm/UseNewMLIROpBuilderCheck.cpp @@ -88,7 +88,7 @@ EditGenerator rewrite(RangeSelector Call, RangeSelector Builder) { return llvm::make_error<llvm::StringError>(llvm::errc::invalid_argument, "unexpected end of file"); } - const bool hasArgs = Arg->getKind() != clang::tok::r_paren; + const bool HasArgs = Arg->getKind() != clang::tok::r_paren; Expected<CharSourceRange> BuilderRange = Builder(Result); if (!BuilderRange) @@ -113,7 +113,7 @@ EditGenerator rewrite(RangeSelector Call, RangeSelector Builder) { const StringRef OpType = GetText(CharSourceRange::getTokenRange( LessToken->getEndLoc(), EndToken->getLastLoc())); Replace.Replacement = llvm::formatv("{}::create({}{}", OpType, BuilderText, - hasArgs ? ", " : ""); + HasArgs ? ", " : ""); return SmallVector<Edit, 1>({Replace}); }; @@ -124,7 +124,7 @@ RewriteRuleWith<std::string> useNewMlirOpBuilderCheckRule() { "'builder.create<OpType>(...)'"); // Match a create call on an OpBuilder. auto BuilderType = cxxRecordDecl(isSameOrDerivedFrom("::mlir::OpBuilder")); - ast_matchers::internal::Matcher<Stmt> base = cxxMemberCallExpr( + ast_matchers::internal::Matcher<Stmt> Base = cxxMemberCallExpr( on(expr(anyOf(hasType(BuilderType), hasType(pointsTo(BuilderType)))) .bind("builder")), callee(expr().bind("call")), @@ -132,9 +132,9 @@ RewriteRuleWith<std::string> useNewMlirOpBuilderCheckRule() { hasName("create")))); return applyFirst( // Attempt rewrite given an lvalue builder, else just warn. - {makeRule(cxxMemberCallExpr(unless(on(cxxTemporaryObjectExpr())), base), + {makeRule(cxxMemberCallExpr(unless(on(cxxTemporaryObjectExpr())), Base), rewrite(node("call"), node("builder")), message), - makeRule(base, noopEdit(node("call")), message)}); + makeRule(Base, noopEdit(node("call")), message)}); } } // namespace >From 01d0e6f0288eb0e43dd4654b2d9ac66c671fb04e Mon Sep 17 00:00:00 2001 From: Jacques Pienaar <[email protected]> Date: Wed, 24 Sep 2025 09:10:55 +0200 Subject: [PATCH 06/11] Change bind to allow one callee descend. --- .../clang-tidy/llvm/UseNewMLIROpBuilderCheck.cpp | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/clang-tools-extra/clang-tidy/llvm/UseNewMLIROpBuilderCheck.cpp b/clang-tools-extra/clang-tidy/llvm/UseNewMLIROpBuilderCheck.cpp index 2dc2a7b971f09..edc7322c4e7ab 100644 --- a/clang-tools-extra/clang-tidy/llvm/UseNewMLIROpBuilderCheck.cpp +++ b/clang-tools-extra/clang-tidy/llvm/UseNewMLIROpBuilderCheck.cpp @@ -124,12 +124,13 @@ RewriteRuleWith<std::string> useNewMlirOpBuilderCheckRule() { "'builder.create<OpType>(...)'"); // Match a create call on an OpBuilder. auto BuilderType = cxxRecordDecl(isSameOrDerivedFrom("::mlir::OpBuilder")); - ast_matchers::internal::Matcher<Stmt> Base = cxxMemberCallExpr( - on(expr(anyOf(hasType(BuilderType), hasType(pointsTo(BuilderType)))) - .bind("builder")), - callee(expr().bind("call")), - callee(cxxMethodDecl(hasTemplateArgument(0, templateArgument()), - hasName("create")))); + ast_matchers::internal::Matcher<Stmt> Base = = + cxxMemberCallExpr( + on(expr(anyOf(hasType(BuilderType), hasType(pointsTo(BuilderType)))) + .bind("builder")), + callee(cxxMethodDecl(hasTemplateArgument(0, templateArgument()), + hasName("create")))) + .bind("call"); return applyFirst( // Attempt rewrite given an lvalue builder, else just warn. {makeRule(cxxMemberCallExpr(unless(on(cxxTemporaryObjectExpr())), Base), >From baf6916c0df0e477b3cf400f677ec59b3cd99792 Mon Sep 17 00:00:00 2001 From: Jacques Pienaar <[email protected]> Date: Wed, 24 Sep 2025 09:11:14 +0200 Subject: [PATCH 07/11] Fix typo --- clang-tools-extra/clang-tidy/llvm/UseNewMLIROpBuilderCheck.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/clang-tools-extra/clang-tidy/llvm/UseNewMLIROpBuilderCheck.cpp b/clang-tools-extra/clang-tidy/llvm/UseNewMLIROpBuilderCheck.cpp index edc7322c4e7ab..0e868b704a3f3 100644 --- a/clang-tools-extra/clang-tidy/llvm/UseNewMLIROpBuilderCheck.cpp +++ b/clang-tools-extra/clang-tidy/llvm/UseNewMLIROpBuilderCheck.cpp @@ -124,7 +124,7 @@ RewriteRuleWith<std::string> useNewMlirOpBuilderCheckRule() { "'builder.create<OpType>(...)'"); // Match a create call on an OpBuilder. auto BuilderType = cxxRecordDecl(isSameOrDerivedFrom("::mlir::OpBuilder")); - ast_matchers::internal::Matcher<Stmt> Base = = + ast_matchers::internal::Matcher<Stmt> Base = cxxMemberCallExpr( on(expr(anyOf(hasType(BuilderType), hasType(pointsTo(BuilderType)))) .bind("builder")), >From 195d5e0add60401a5d0b10ee51fc9e67240b2c43 Mon Sep 17 00:00:00 2001 From: Jacques Pienaar <[email protected]> Date: Wed, 22 Oct 2025 17:08:24 +0000 Subject: [PATCH 08/11] Address change in check matching --- .../checkers/llvm/use-new-mlir-op-builder.cpp | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/clang-tools-extra/test/clang-tidy/checkers/llvm/use-new-mlir-op-builder.cpp b/clang-tools-extra/test/clang-tidy/checkers/llvm/use-new-mlir-op-builder.cpp index bd5437deb6ced..28458140a486d 100644 --- a/clang-tools-extra/test/clang-tidy/checkers/llvm/use-new-mlir-op-builder.cpp +++ b/clang-tools-extra/test/clang-tidy/checkers/llvm/use-new-mlir-op-builder.cpp @@ -52,7 +52,7 @@ class CustomBuilder : public mlir::ImplicitLocOpBuilder { public: mlir::NamedOp f(const char *name) { // CHECK-MESSAGES: :[[@LINE+2]]:12: warning: use 'OpType::create(builder, ...)' - // CHECK-FIXES: mlir::NamedOp::create(*this, name); + // CHECK-FIXES: return mlir::NamedOp::create(*this, name); return create<mlir::NamedOp>(name); } }; @@ -70,9 +70,10 @@ void f() { // CHECK-FIXES: NamedOp::create(builder, builder.getUnknownLoc(), "baz"); builder.create<NamedOp>(builder.getUnknownLoc(), "baz"); - // CHECK-MESSAGES: :[[@LINE+3]]:3: warning: use 'OpType::create(builder, ...)' instead of 'builder.create<OpType>(...)' [llvm-use-new-mlir-op-builder] - // CHECK-FIXES: NamedOp::create(builder, builder.getUnknownLoc(), - // CHECK-FIXES: "caz"); + // CHECK-MESSAGES: :[[@LINE+4]]:3: warning: use 'OpType::create(builder, ...)' instead of 'builder.create<OpType>(...)' [llvm-use-new-mlir-op-builder] + // CHECK-FIXES: NamedOp::create(builder, + // CHECK-FIXES: builder.getUnknownLoc(), + // CHECK-FIXES: "caz"); builder. create<NamedOp> ( builder.getUnknownLoc(), @@ -85,7 +86,7 @@ void f() { mlir::ImplicitLocOpBuilder ib; // CHECK-MESSAGES: :[[@LINE+2]]:3: warning: use 'OpType::create(builder, ...)' instead of 'builder.create<OpType>(...)' [llvm-use-new-mlir-op-builder] - // CHECK-FIXES: mlir::ModuleOp::create(ib); + // CHECK-FIXES: mlir::ModuleOp::create(ib ); ib.create<mlir::ModuleOp>( ); // CHECK-MESSAGES: :[[@LINE+2]]:3: warning: use 'OpType::create(builder, ...)' instead of 'builder.create<OpType>(...)' [llvm-use-new-mlir-op-builder] @@ -94,7 +95,7 @@ void f() { auto *p = &builder; // CHECK-MESSAGES: :[[@LINE+2]]:3: warning: use 'OpType::create(builder, ...)' - // CHECK-FIXES: NamedOp::create(*p, builder.getUnknownLoc(), "eaz") + // CHECK-FIXES: NamedOp::create(*p, builder.getUnknownLoc(), "eaz"); p->create<NamedOp>(builder.getUnknownLoc(), "eaz"); CustomBuilder cb; @@ -103,7 +104,7 @@ void f() { // CHECK-MESSAGES: :[[@LINE+4]]:3: warning: use 'OpType::create(builder, ...)' instead of 'builder.create<OpType>(...)' [llvm-use-new-mlir-op-builder] // CHECK-FIXES: OperandOp::create(builder, builder.getUnknownLoc(), // CHECK-MESSAGES: :[[@LINE+3]]:5: warning: use 'OpType::create(builder, ...)' instead of 'builder.create<OpType>(...)' [llvm-use-new-mlir-op-builder] - // CHECK-FIXES: NamedOp::create(builder, + // CHECK-FIXES: NamedOp::create(builder, builder.getUnknownLoc(), "gaz").getResult()); builder.create<OperandOp>(builder.getUnknownLoc(), builder.create<NamedOp>(builder.getUnknownLoc(), "gaz").getResult()); } >From d3f671c787d3762fa2eef7e1625e3b5f43c69c21 Mon Sep 17 00:00:00 2001 From: Jacques Pienaar <[email protected]> Date: Thu, 23 Oct 2025 13:25:11 +0000 Subject: [PATCH 09/11] Fix bad merge via web diff --- .../clang-tidy/llvm/UseNewMLIROpBuilderCheck.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/clang-tools-extra/clang-tidy/llvm/UseNewMLIROpBuilderCheck.cpp b/clang-tools-extra/clang-tidy/llvm/UseNewMLIROpBuilderCheck.cpp index f1dd79f83d181..e7c62913ef1dc 100644 --- a/clang-tools-extra/clang-tidy/llvm/UseNewMLIROpBuilderCheck.cpp +++ b/clang-tools-extra/clang-tidy/llvm/UseNewMLIROpBuilderCheck.cpp @@ -123,6 +123,7 @@ RewriteRuleWith<std::string> useNewMlirOpBuilderCheckRule() { Stencil Message = cat("use 'OpType::create(builder, ...)' instead of " "'builder.create<OpType>(...)'"); // Match a create call on an OpBuilder. + auto BuilderType = cxxRecordDecl(isSameOrDerivedFrom("::mlir::OpBuilder")); ast_matchers::internal::Matcher<Stmt> Base = cxxMemberCallExpr( on(expr(anyOf(hasType(BuilderType), hasType(pointsTo(BuilderType)))) @@ -133,8 +134,7 @@ RewriteRuleWith<std::string> useNewMlirOpBuilderCheckRule() { return applyFirst( // Attempt rewrite given an lvalue builder, else just warn. {makeRule(cxxMemberCallExpr(unless(on(cxxTemporaryObjectExpr())), Base), - rewrite(node("call"), node("builder"), callArgs("call")), - Message), + rewrite(node("call"), node("builder")), Message), makeRule(Base, noopEdit(node("call")), Message)}); } } // namespace >From bfffb499d29a045839770bd80439b91f8a0c9eac Mon Sep 17 00:00:00 2001 From: Jacques Pienaar <[email protected]> Date: Fri, 24 Oct 2025 10:20:57 +0000 Subject: [PATCH 10/11] Switch out of anonymous to static functions --- .../clang-tidy/llvm/UseNewMLIROpBuilderCheck.cpp | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/clang-tools-extra/clang-tidy/llvm/UseNewMLIROpBuilderCheck.cpp b/clang-tools-extra/clang-tidy/llvm/UseNewMLIROpBuilderCheck.cpp index e7c62913ef1dc..0014153cceaa3 100644 --- a/clang-tools-extra/clang-tidy/llvm/UseNewMLIROpBuilderCheck.cpp +++ b/clang-tools-extra/clang-tidy/llvm/UseNewMLIROpBuilderCheck.cpp @@ -18,12 +18,11 @@ #include "llvm/Support/FormatVariadic.h" namespace clang::tidy::llvm_check { -namespace { using namespace ::clang::ast_matchers; using namespace ::clang::transformer; -EditGenerator rewrite(RangeSelector Call, RangeSelector Builder) { +static EditGenerator rewrite(RangeSelector Call, RangeSelector Builder) { // This is using an EditGenerator rather than ASTEdit as we want to warn even // if in macro. return [Call = std::move(Call), @@ -119,7 +118,7 @@ EditGenerator rewrite(RangeSelector Call, RangeSelector Builder) { }; } -RewriteRuleWith<std::string> useNewMlirOpBuilderCheckRule() { +static RewriteRuleWith<std::string> useNewMlirOpBuilderCheckRule() { Stencil Message = cat("use 'OpType::create(builder, ...)' instead of " "'builder.create<OpType>(...)'"); // Match a create call on an OpBuilder. @@ -137,7 +136,6 @@ RewriteRuleWith<std::string> useNewMlirOpBuilderCheckRule() { rewrite(node("call"), node("builder")), Message), makeRule(Base, noopEdit(node("call")), Message)}); } -} // namespace UseNewMlirOpBuilderCheck::UseNewMlirOpBuilderCheck(StringRef Name, ClangTidyContext *Context) >From b8788c4b049c159049bdfa47c9894276b700cad3 Mon Sep 17 00:00:00 2001 From: Jacques Pienaar <[email protected]> Date: Sun, 2 Nov 2025 16:22:27 +0200 Subject: [PATCH 11/11] Update test to address comments Webedit --- .../checkers/llvm/use-new-mlir-op-builder.cpp | 18 +++++++++++++----- 1 file changed, 13 insertions(+), 5 deletions(-) diff --git a/clang-tools-extra/test/clang-tidy/checkers/llvm/use-new-mlir-op-builder.cpp b/clang-tools-extra/test/clang-tidy/checkers/llvm/use-new-mlir-op-builder.cpp index 28458140a486d..3dbc2f4e0ca57 100644 --- a/clang-tools-extra/test/clang-tidy/checkers/llvm/use-new-mlir-op-builder.cpp +++ b/clang-tools-extra/test/clang-tidy/checkers/llvm/use-new-mlir-op-builder.cpp @@ -55,6 +55,13 @@ class CustomBuilder : public mlir::ImplicitLocOpBuilder { // CHECK-FIXES: return mlir::NamedOp::create(*this, name); return create<mlir::NamedOp>(name); } + + mlir::NamedOp g(const char *name) { + using mlir::NamedOp; + // CHECK-MESSAGES: :[[@LINE+2]]:12: warning: use 'OpType::create(builder, ...)' + // CHECK-FIXES: return NamedOp::create(*this, name); + return create<NamedOp>(name); + } }; void f() { @@ -100,11 +107,12 @@ void f() { CustomBuilder cb; cb.f("faz"); + cb.g("gaz"); - // CHECK-MESSAGES: :[[@LINE+4]]:3: warning: use 'OpType::create(builder, ...)' instead of 'builder.create<OpType>(...)' [llvm-use-new-mlir-op-builder] - // CHECK-FIXES: OperandOp::create(builder, builder.getUnknownLoc(), - // CHECK-MESSAGES: :[[@LINE+3]]:5: warning: use 'OpType::create(builder, ...)' instead of 'builder.create<OpType>(...)' [llvm-use-new-mlir-op-builder] - // CHECK-FIXES: NamedOp::create(builder, builder.getUnknownLoc(), "gaz").getResult()); + // CHECK-FIXES: OperandOp::create(builder, builder.getUnknownLoc(), + // CHECK-FIXES-NEXT: NamedOp::create(builder, builder.getUnknownLoc(), "gaz").getResult()); + // CHECK-MESSAGES: :[[@LINE+2]]:3: warning: use 'OpType::create(builder, ...)' instead of 'builder.create<OpType>(...)' [llvm-use-new-mlir-op-builder] + // CHECK-MESSAGES: :[[@LINE+2]]:5: warning: use 'OpType::create(builder, ...)' instead of 'builder.create<OpType>(...)' [llvm-use-new-mlir-op-builder] builder.create<OperandOp>(builder.getUnknownLoc(), - builder.create<NamedOp>(builder.getUnknownLoc(), "gaz").getResult()); + builder.create<NamedOp>(builder.getUnknownLoc(), "haz").getResult()); } _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
