[clang-tidy] Add MLIR check for old op builder usage.

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

@@ -7,6 +7,7 @@ add_clang_library(clangTidyLLVMModule STATIC
   HeaderGuardCheck.cpp
   IncludeOrderCheck.cpp
   LLVMTidyModule.cpp
+  MLIROpBuilderCheck.cpp
   PreferIsaOrDynCastInConditionalsCheck.cpp
   PreferRegisterOverUnsignedCheck.cpp
   PreferStaticOverAnonymousNamespaceCheck.cpp
@@ -16,6 +17,7 @@ add_clang_library(clangTidyLLVMModule STATIC
   clangTidy
   clangTidyReadabilityModule
   clangTidyUtils
+  clangTransformer
 
   DEPENDS
   omp_gen

clang-tools-extra/clang-tidy/llvm/LLVMTidyModule.cpp

@@ -14,6 +14,7 @@
 #include "../readability/QualifiedAutoCheck.h"
 #include "HeaderGuardCheck.h"
 #include "IncludeOrderCheck.h"
+#include "MLIROpBuilderCheck.h"
 #include "PreferIsaOrDynCastInConditionalsCheck.h"
 #include "PreferRegisterOverUnsignedCheck.h"
 #include "PreferStaticOverAnonymousNamespaceCheck.h"
@@ -29,6 +30,8 @@ class LLVMModule : public ClangTidyModule {
         "llvm-else-after-return");
     CheckFactories.registerCheck<LLVMHeaderGuardCheck>("llvm-header-guard");
     CheckFactories.registerCheck<IncludeOrderCheck>("llvm-include-order");
+    CheckFactories.registerCheck<MlirOpBuilderCheck>(
+        "llvm-use-new-mlir-op-builder");
     CheckFactories.registerCheck<readability::NamespaceCommentCheck>(
         "llvm-namespace-comment");
     CheckFactories.registerCheck<PreferIsaOrDynCastInConditionalsCheck>(

clang-tools-extra/clang-tidy/llvm/MLIROpBuilderCheck.cpp

@@ -0,0 +1,132 @@
+//===--- MLIROpBuilderCheck.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 "MLIROpBuilderCheck.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> mlirOpBuilderCheckRule() {
+  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
+
+MlirOpBuilderCheck::MlirOpBuilderCheck(StringRef Name,
+                                       ClangTidyContext *Context)
+    : TransformerClangTidyCheck(mlirOpBuilderCheckRule(), Name, Context) {}
+
+} // namespace clang::tidy::llvm_check

clang-tools-extra/clang-tidy/llvm/MLIROpBuilderCheck.h

@@ -0,0 +1,29 @@
+//===--- MLIROpBuilderCheck.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_MLIROPBUILDERCHECK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_LLVM_MLIROPBUILDERCHECK_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 MlirOpBuilderCheck : public utils::TransformerClangTidyCheck {
+public:
+  MlirOpBuilderCheck(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_MLIROPBUILDERCHECK_H

clang-tools-extra/docs/ReleaseNotes.rst

@@ -91,6 +91,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
 ^^^^^^^^^^^^^^^^^
 

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"

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");

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>(   );
+}