Author: Jacques Pienaar Date: 2025-07-24T15:48:05+02:00 New Revision: 3feb6f971577701713034d3404b6737fe6462d43
URL: https://github.com/llvm/llvm-project/commit/3feb6f971577701713034d3404b6737fe6462d43 DIFF: https://github.com/llvm/llvm-project/commit/3feb6f971577701713034d3404b6737fe6462d43.diff LOG: [clang-tidy] Add MLIR check for old op builder usage. (#149148) Upstream is moving towards new create method invocation, add check to flag old usage that will be deprecated. --------- Co-authored-by: Baranov Victor <bar.victor.2...@gmail.com> Co-authored-by: EugeneZelenko <eugene.zele...@gmail.com> Added: clang-tools-extra/clang-tidy/llvm/UseNewMLIROpBuilderCheck.cpp clang-tools-extra/clang-tidy/llvm/UseNewMLIROpBuilderCheck.h clang-tools-extra/docs/clang-tidy/checks/llvm/use-new-mlir-op-builder.rst clang-tools-extra/test/clang-tidy/checkers/llvm/use-new-mlir-op-builder.cpp Modified: clang-tools-extra/clang-tidy/llvm/CMakeLists.txt clang-tools-extra/clang-tidy/llvm/LLVMTidyModule.cpp clang-tools-extra/docs/ReleaseNotes.rst clang-tools-extra/docs/clang-tidy/checks/list.rst Removed: ################################################################################ diff --git a/clang-tools-extra/clang-tidy/llvm/CMakeLists.txt b/clang-tools-extra/clang-tidy/llvm/CMakeLists.txt index 3232f6e2cafe5..4f1da43d3f1b7 100644 --- a/clang-tools-extra/clang-tidy/llvm/CMakeLists.txt +++ b/clang-tools-extra/clang-tidy/llvm/CMakeLists.txt @@ -11,11 +11,13 @@ add_clang_library(clangTidyLLVMModule STATIC PreferRegisterOverUnsignedCheck.cpp PreferStaticOverAnonymousNamespaceCheck.cpp TwineLocalCheck.cpp + UseNewMLIROpBuilderCheck.cpp LINK_LIBS clangTidy clangTidyReadabilityModule clangTidyUtils + clangTransformer DEPENDS omp_gen diff --git a/clang-tools-extra/clang-tidy/llvm/LLVMTidyModule.cpp b/clang-tools-extra/clang-tidy/llvm/LLVMTidyModule.cpp index 075453046f0a1..c7c61fd1649cc 100644 --- a/clang-tools-extra/clang-tidy/llvm/LLVMTidyModule.cpp +++ b/clang-tools-extra/clang-tidy/llvm/LLVMTidyModule.cpp @@ -18,6 +18,7 @@ #include "PreferRegisterOverUnsignedCheck.h" #include "PreferStaticOverAnonymousNamespaceCheck.h" #include "TwineLocalCheck.h" +#include "UseNewMLIROpBuilderCheck.h" namespace clang::tidy { namespace llvm_check { @@ -40,6 +41,8 @@ class LLVMModule : public ClangTidyModule { CheckFactories.registerCheck<readability::QualifiedAutoCheck>( "llvm-qualified-auto"); CheckFactories.registerCheck<TwineLocalCheck>("llvm-twine-local"); + CheckFactories.registerCheck<UseNewMlirOpBuilderCheck>( + "llvm-use-new-mlir-op-builder"); } ClangTidyOptions getModuleOptions() override { diff --git a/clang-tools-extra/clang-tidy/llvm/UseNewMLIROpBuilderCheck.cpp b/clang-tools-extra/clang-tidy/llvm/UseNewMLIROpBuilderCheck.cpp new file mode 100644 index 0000000000000..0b28ea2508977 --- /dev/null +++ b/clang-tools-extra/clang-tidy/llvm/UseNewMLIROpBuilderCheck.cpp @@ -0,0 +1,133 @@ +//===--- UseNewMLIROpBuilderCheck.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 "UseNewMLIROpBuilderCheck.h" +#include "clang/ASTMatchers/ASTMatchers.h" +#include "clang/Basic/LLVM.h" +#include "clang/Lex/Lexer.h" +#include "clang/Tooling/Transformer/RangeSelector.h" +#include "clang/Tooling/Transformer/RewriteRule.h" +#include "clang/Tooling/Transformer/SourceCode.h" +#include "clang/Tooling/Transformer/Stencil.h" +#include "llvm/Support/Error.h" +#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, + RangeSelector CallArgs) { + // 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) + -> 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(); + + // This will result in just a warning and no edit. + 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}); + } + + // 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->getEndLoc() >= CallRange->getEnd()) + return std::optional<Token>(); + return clang::Lexer::findNextToken(CurrentToken->getLocation(), SM, + LangOpts); + }; + std::optional<Token> LessToken = + clang::Lexer::findNextToken(Begin, SM, LangOpts); + while (LessToken && LessToken->getKind() != clang::tok::less) { + LessToken = NextToken(LessToken); + } + if (!LessToken) { + 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; + GreaterToken = NextToken(GreaterToken)) { + EndToken = GreaterToken; + } + if (!EndToken) { + return llvm::make_error<llvm::StringError>(llvm::errc::invalid_argument, + "missing '>' token"); + } + + 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) { + return clang::Lexer::getSourceText(Range, SM, LangOpts); + }; + + 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.Replacement = + llvm::formatv("{}::create({}{})", + GetText(CharSourceRange::getTokenRange( + LessToken->getEndLoc(), EndToken->getLastLoc())), + GetText(*BuilderRange), CallArgsStr); + + return SmallVector<Edit, 1>({Replace}); + }; +} + +RewriteRuleWith<std::string> useNewMlirOpBuilderCheckRule() { + return makeRule( + cxxMemberCallExpr( + on(expr(hasType( + cxxRecordDecl(isSameOrDerivedFrom("::mlir::OpBuilder")))) + .bind("builder")), + callee(cxxMethodDecl(hasTemplateArgument(0, templateArgument()))), + callee(cxxMethodDecl(hasName("create")))) + .bind("call"), + rewrite(node("call"), node("builder"), callArgs("call")), + cat("use 'OpType::create(builder, ...)' instead of " + "'builder.create<OpType>(...)'")); +} +} // namespace + +UseNewMlirOpBuilderCheck::UseNewMlirOpBuilderCheck(StringRef Name, + ClangTidyContext *Context) + : TransformerClangTidyCheck(useNewMlirOpBuilderCheckRule(), Name, Context) { +} + +} // 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 new file mode 100644 index 0000000000000..813a23c564782 --- /dev/null +++ b/clang-tools-extra/clang-tidy/llvm/UseNewMLIROpBuilderCheck.h @@ -0,0 +1,29 @@ +//===--- UseNewMLIROpBuilderCheck.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_LLVM_USENEWMLIROPBUILDERCHECK_H +#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_LLVM_USENEWMLIROPBUILDERCHECK_H + +#include "../utils/TransformerClangTidyCheck.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 { +public: + UseNewMlirOpBuilderCheck(StringRef Name, ClangTidyContext *Context); + + bool isLanguageVersionSupported(const LangOptions &LangOpts) const override { + return getLangOpts().CPlusPlus; + } +}; + +} // namespace clang::tidy::llvm_check + +#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_LLVM_USENEWMLIROPBUILDERCHECK_H diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst index edab04730cb25..bde4ddec50ff3 100644 --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -96,6 +96,12 @@ Improvements to clang-tidy New checks ^^^^^^^^^^ +- New :doc:`llvm-mlir-op-builder + <clang-tidy/checks/llvm/use-new-mlir-op-builder>` check. + + Checks for uses of MLIR's old/to be deprecated ``OpBuilder::create<T>`` form + and suggests using ``T::create`` instead. + New check aliases ^^^^^^^^^^^^^^^^^ diff --git a/clang-tools-extra/docs/clang-tidy/checks/list.rst b/clang-tools-extra/docs/clang-tidy/checks/list.rst index 0cffbd323caa2..20a43274f9788 100644 --- a/clang-tools-extra/docs/clang-tidy/checks/list.rst +++ b/clang-tools-extra/docs/clang-tidy/checks/list.rst @@ -247,6 +247,7 @@ Clang-Tidy Checks :doc:`linuxkernel-must-check-errs <linuxkernel/must-check-errs>`, :doc:`llvm-header-guard <llvm/header-guard>`, :doc:`llvm-include-order <llvm/include-order>`, "Yes" + :doc:`llvm-use-new-mlir-op-builder <llvm/use-new-mlir-op-builder>`, "Yes" :doc:`llvm-namespace-comment <llvm/namespace-comment>`, :doc:`llvm-prefer-isa-or-dyn-cast-in-conditionals <llvm/prefer-isa-or-dyn-cast-in-conditionals>`, "Yes" :doc:`llvm-prefer-register-over-unsigned <llvm/prefer-register-over-unsigned>`, "Yes" diff --git a/clang-tools-extra/docs/clang-tidy/checks/llvm/use-new-mlir-op-builder.rst b/clang-tools-extra/docs/clang-tidy/checks/llvm/use-new-mlir-op-builder.rst new file mode 100644 index 0000000000000..bb7427d4a4fbc --- /dev/null +++ b/clang-tools-extra/docs/clang-tidy/checks/llvm/use-new-mlir-op-builder.rst @@ -0,0 +1,21 @@ +.. title:: clang-tidy - llvm-mlir-op-builder + +llvm-mlir-op-builder +==================== + +Checks for uses of MLIR's old/to be deprecated ``OpBuilder::create<T>`` form +and suggests using ``T::create`` instead. + +Example +------- + +.. code-block:: c++ + + builder.create<FooOp>(builder.getUnknownLoc(), "baz"); + + +Transforms to: + +.. code-block:: c++ + + FooOp::create(builder, builder.getUnknownLoc(), "baz"); 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 new file mode 100644 index 0000000000000..57e026c10bf53 --- /dev/null +++ b/clang-tools-extra/test/clang-tidy/checkers/llvm/use-new-mlir-op-builder.cpp @@ -0,0 +1,72 @@ +// RUN: %check_clang_tidy --match-partial-fixes %s llvm-use-new-mlir-op-builder %t + +namespace mlir { +class Location {}; +class OpBuilder { +public: + template <typename OpTy, typename... Args> + OpTy create(Location location, Args &&...args) { + return OpTy(args...); + } + Location getUnknownLoc() { return Location(); } +}; +class ImplicitLocOpBuilder : public OpBuilder { +public: + template <typename OpTy, typename... Args> + OpTy create(Args &&...args) { + return OpTy(args...); + } +}; +struct ModuleOp { + ModuleOp() {} + static ModuleOp create(OpBuilder &builder, Location location) { + return ModuleOp(); + } +}; +struct NamedOp { + NamedOp(const char* name) {} + static NamedOp create(OpBuilder &builder, Location location, const char* name) { + return NamedOp(name); + } +}; +} // namespace mlir + +#define ASSIGN(A, B, C, D) C A = B.create<C>(B.getUnknownLoc(), D) + +template <typename T> +void g(mlir::OpBuilder &b) { + // CHECK-MESSAGES: :[[@LINE+2]]:3: warning: use 'OpType::create(builder, ...)' instead of 'builder.create<OpType>(...)' [llvm-use-new-mlir-op-builder] + // CHECK-FIXES: T::create(b, b.getUnknownLoc(), "gaz") + b.create<T>(b.getUnknownLoc(), "gaz"); +} + +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] + // CHECK-FIXES: mlir:: ModuleOp::create(builder, builder.getUnknownLoc()) + builder.create<mlir:: ModuleOp>(builder.getUnknownLoc()); + + using mlir::NamedOp; + // 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"); + + // 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(), + "caz"); + + // CHECK-MESSAGES: :[[@LINE+1]]:3: warning: use 'OpType::create(builder, ...)' instead of 'builder.create<OpType>(...)' [llvm-use-new-mlir-op-builder] + ASSIGN(op, builder, NamedOp, "daz"); + + g<NamedOp>(builder); + + 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) + ib.create<mlir::ModuleOp>( ); +} _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
