https://github.com/localspook created https://github.com/llvm/llvm-project/pull/196596
None >From 509f27d61887e6df161838b3fa5eb94f2de7c749 Mon Sep 17 00:00:00 2001 From: Victor Chernyakin <[email protected]> Date: Fri, 8 May 2026 10:50:43 -0700 Subject: [PATCH] [clang-tidy] Speed up/rewrite `llvm-use-new-mlir-op-builder` --- .../llvm/UseNewMLIROpBuilderCheck.cpp | 169 ++++++------------ .../llvm/UseNewMLIROpBuilderCheck.h | 8 +- .../checkers/llvm/use-new-mlir-op-builder.cpp | 6 +- 3 files changed, 59 insertions(+), 124 deletions(-) diff --git a/clang-tools-extra/clang-tidy/llvm/UseNewMLIROpBuilderCheck.cpp b/clang-tools-extra/clang-tidy/llvm/UseNewMLIROpBuilderCheck.cpp index ca3e778e573c1..66371f7b2a58f 100644 --- a/clang-tools-extra/clang-tidy/llvm/UseNewMLIROpBuilderCheck.cpp +++ b/clang-tools-extra/clang-tidy/llvm/UseNewMLIROpBuilderCheck.cpp @@ -7,137 +7,70 @@ //===----------------------------------------------------------------------===// #include "UseNewMLIROpBuilderCheck.h" -#include "../utils/LexerUtils.h" #include "clang/ASTMatchers/ASTMatchers.h" -#include "clang/Basic/LLVM.h" -#include "clang/Tooling/Transformer/RangeSelector.h" -#include "clang/Tooling/Transformer/RewriteRule.h" -#include "clang/Tooling/Transformer/Stencil.h" -#include "llvm/Support/Error.h" -#include "llvm/Support/FormatVariadic.h" +#include "clang/Lex/Lexer.h" -namespace clang::tidy::llvm_check { - -using namespace ::clang::ast_matchers; -using namespace ::clang::transformer; +using namespace clang::ast_matchers; -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), - Builder = std::move(Builder)](const MatchFinder::MatchResult &Result) - -> Expected<SmallVector<transformer::Edit, 1>> { - Expected<CharSourceRange> CallRange = Call(Result); - if (!CallRange) - return CallRange.takeError(); - SourceManager &SM = *Result.SourceManager; - const LangOptions &LangOpts = Result.Context->getLangOpts(); - SourceLocation Begin = CallRange->getBegin(); +namespace clang::tidy::llvm_check { - // This will result in just a warning and no edit. - const bool InMacro = CallRange->getBegin().isMacroID(); - if (InMacro) { - while (SM.isMacroArgExpansion(Begin)) - Begin = SM.getImmediateExpansionRange(Begin).getBegin(); - Edit WarnOnly; - WarnOnly.Kind = EditKind::Range; - WarnOnly.Range = CharSourceRange::getCharRange(Begin, Begin); - return SmallVector<Edit, 1>({WarnOnly}); - } +void UseNewMlirOpBuilderCheck::registerMatchers(MatchFinder *Finder) { + // Match a create call on an OpBuilder. + const auto BuilderType = + cxxRecordDecl(isSameOrDerivedFrom("::mlir::OpBuilder")); + Finder->addMatcher( + cxxMemberCallExpr( + callee(cxxMethodDecl(hasTemplateArgument(0, templateArgument()), + hasName("create"))), + on(expr(anyOf(hasType(BuilderType), hasType(pointsTo(BuilderType)))) + .bind("builder"))) + .bind("call"), + this); +} - // This will try to extract the template argument as written so that the - // rewritten code looks closest to original. - auto NextToken = [&](std::optional<Token> CurrentToken) { - if (!CurrentToken) - return CurrentToken; - if (CurrentToken->is(tok::eof)) - return std::optional<Token>(); - return utils::lexer::findNextTokenSkippingComments( - CurrentToken->getLocation(), SM, LangOpts); - }; - std::optional<Token> LessToken = - utils::lexer::findNextTokenSkippingComments(Begin, SM, LangOpts); - while (LessToken && LessToken->getKind() != tok::less) - LessToken = NextToken(LessToken); - if (!LessToken) { - return llvm::make_error<llvm::StringError>(llvm::errc::invalid_argument, - "missing '<' token"); - } +void UseNewMlirOpBuilderCheck::check(const MatchFinder::MatchResult &Result) { + const auto *Call = Result.Nodes.getNodeAs<CXXMemberCallExpr>("call"); + const auto *Builder = Result.Nodes.getNodeAs<Expr>("builder"); - std::optional<Token> EndToken = NextToken(LessToken); - std::optional<Token> GreaterToken = NextToken(EndToken); - for (; GreaterToken && GreaterToken->getKind() != tok::greater; - GreaterToken = NextToken(GreaterToken)) { - EndToken = GreaterToken; - } - if (!EndToken) { - return llvm::make_error<llvm::StringError>(llvm::errc::invalid_argument, - "missing '>' token"); - } + const DiagnosticBuilder Diag = + diag(Result.SourceManager->getExpansionLoc(Call->getBeginLoc()), + "use 'OpType::create(builder, ...)' instead of " + "'builder.create<OpType>(...)'"); - std::optional<Token> ArgStart = NextToken(GreaterToken); - if (!ArgStart || ArgStart->getKind() != 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"); - } - const bool HasArgs = Arg->getKind() != tok::r_paren; + if (Call->getBeginLoc().isMacroID()) + return; - Expected<CharSourceRange> BuilderRange = Builder(Result); - if (!BuilderRange) - return BuilderRange.takeError(); + // Only attempt the rewrite if given an lvalue builder. + if (isa<CXXTemporaryObjectExpr>(Builder)) + return; - // Helper for concatting below. - auto GetText = [&](const CharSourceRange &Range) { - return Lexer::getSourceText(Range, SM, LangOpts); - }; + const auto *Callee = dyn_cast<MemberExpr>(Call->getCallee()); + if (!Callee) + return; - Edit Replace; - Replace.Kind = EditKind::Range; - 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(); - } - const StringRef OpType = GetText(CharSourceRange::getTokenRange( - LessToken->getEndLoc(), EndToken->getLastLoc())); - Replace.Replacement = llvm::formatv("{}::create({}{}", OpType, BuilderText, - HasArgs ? ", " : ""); + Diag << FixItHint::CreateRemoval( + {Call->getBeginLoc(), Callee->getLAngleLoc()}) + << FixItHint::CreateReplacement(Callee->getRAngleLoc(), "::create"); - return SmallVector<Edit, 1>({Replace}); - }; -} + std::string BuilderArg; + if (Builder->isImplicitCXXThis()) { + BuilderArg = "*this"; + } else { + if (Callee->isArrow()) + BuilderArg += '*'; -static RewriteRuleWith<std::string> useNewMlirOpBuilderCheckRule() { - const 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")); - const 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), - rewrite(node("call"), node("builder")), Message), - makeRule(Base, noopEdit(node("call")), Message)}); -} + BuilderArg += Lexer::getSourceText( + CharSourceRange::getTokenRange(Builder->getSourceRange()), + *Result.SourceManager, getLangOpts()); + } -UseNewMlirOpBuilderCheck::UseNewMlirOpBuilderCheck(StringRef Name, - ClangTidyContext *Context) - : TransformerClangTidyCheck(useNewMlirOpBuilderCheckRule(), Name, Context) { + if (Call->getNumArgs() == 0) { + Diag << FixItHint::CreateInsertion(Call->getRParenLoc(), BuilderArg); + } else { + BuilderArg += ", "; + Diag << FixItHint::CreateInsertion(Call->getArg(0)->getBeginLoc(), + BuilderArg); + } } } // namespace clang::tidy::llvm_check diff --git a/clang-tools-extra/clang-tidy/llvm/UseNewMLIROpBuilderCheck.h b/clang-tools-extra/clang-tidy/llvm/UseNewMLIROpBuilderCheck.h index 28c7477a18f7f..c24a6f8cdf5c8 100644 --- a/clang-tools-extra/clang-tidy/llvm/UseNewMLIROpBuilderCheck.h +++ b/clang-tools-extra/clang-tidy/llvm/UseNewMLIROpBuilderCheck.h @@ -9,15 +9,17 @@ #ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_LLVM_USENEWMLIROPBUILDERCHECK_H #define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_LLVM_USENEWMLIROPBUILDERCHECK_H -#include "../utils/TransformerClangTidyCheck.h" +#include "../ClangTidyCheck.h" namespace clang::tidy::llvm_check { /// Checks for uses of MLIR's old/to be deprecated `OpBuilder::create<T>` form /// and suggests using `T::create` instead. -class UseNewMlirOpBuilderCheck : public utils::TransformerClangTidyCheck { +class UseNewMlirOpBuilderCheck : public ClangTidyCheck { public: - UseNewMlirOpBuilderCheck(StringRef Name, ClangTidyContext *Context); + using ClangTidyCheck::ClangTidyCheck; + void registerMatchers(ast_matchers::MatchFinder *Finder) override; + void check(const ast_matchers::MatchFinder::MatchResult &Result) override; bool isLanguageVersionSupported(const LangOptions &LangOpts) const override { return LangOpts.CPlusPlus; 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 c4a1d8d66cdeb..9b20ccf98d1e9 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 @@ -78,8 +78,8 @@ void f() { builder.create<NamedOp>(builder.getUnknownLoc(), "baz"); // 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: NamedOp::create ( + // CHECK-FIXES: builder, builder.getUnknownLoc(), // CHECK-FIXES: "caz"); builder. create<NamedOp> ( @@ -93,7 +93,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] _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
