[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"
@@ -40,6 +41,7 @@ class LLVMModule : public ClangTidyModule {
     CheckFactories.registerCheck<readability::QualifiedAutoCheck>(
         "llvm-qualified-auto");
     CheckFactories.registerCheck<TwineLocalCheck>("llvm-twine-local");
+    CheckFactories.registerCheck<MlirOpBuilderCheck>("llvm-mlir-op-builder");
   }
 
   ClangTidyOptions getModuleOptions() override {

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

@@ -0,0 +1,103 @@
+//===--- 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"
+
+namespace clang::tidy::llvm_check {
+namespace {
+
+using namespace ::clang::ast_matchers; // NOLINT: Too many names.
+using namespace ::clang::transformer;  // NOLINT: Too many names.
+
+class TypeAsWrittenStencil : public StencilInterface {
+public:
+  explicit TypeAsWrittenStencil(std::string S) : Id(std::move(S)) {}
+  std::string toString() const override {
+    return (llvm::Twine("TypeAsWritten(\"") + Id + "\")").str();
+  }
+
+  llvm::Error eval(const MatchFinder::MatchResult &match,
+                   std::string *result) const override {
+    llvm::Expected<CharSourceRange> n = node(Id)(match);
+    if (!n)
+      return n.takeError();
+    const SourceRange SrcRange = n->getAsRange();
+    if (SrcRange.isInvalid()) {
+      return llvm::make_error<llvm::StringError>(llvm::errc::invalid_argument,
+                                                 "SrcRange is invalid");
+    }
+    const CharSourceRange Range = n->getTokenRange(SrcRange);
+    auto NextToken = [&](std::optional<Token> Token) {
+      if (!Token)
+        return Token;
+      return clang::Lexer::findNextToken(Token->getLocation(),
+                                         *match.SourceManager,
+                                         match.Context->getLangOpts());
+    };
+    std::optional<Token> LessToken = clang::Lexer::findNextToken(
+        Range.getBegin(), *match.SourceManager, match.Context->getLangOpts());
+    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");
+    }
+    *result += clang::tooling::getText(
+        CharSourceRange::getTokenRange(LessToken->getEndLoc(),
+                                       EndToken->getLastLoc()),
+        *match.Context);
+    return llvm::Error::success();
+  }
+  std::string Id;
+};
+
+Stencil typeAsWritten(StringRef Id) {
+  // Using this instead of `describe` so that we get the exact same spelling.
+  return std::make_shared<TypeAsWrittenStencil>(std::string(Id));
+}
+
+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"),
+      changeTo(cat(typeAsWritten("call"), "::create(", 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 `OpBuilder::create<T>` 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

@@ -96,6 +96,12 @@ Improvements to clang-tidy
 New checks
 ^^^^^^^^^^
 
+- New :doc:`llvm-mlir-op-builder
+  <clang-tidy/checks/llvm/mlir-op-builder>` check.
+
+  Flags usage of old form of invoking create on MLIR's ``OpBuilder`` and
+  suggests new form.
+
 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-mlir-op-builder <llvm/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/mlir-op-builder.rst

@@ -0,0 +1,21 @@
+.. title:: clang-tidy - llvm-mlir-op-builder
+
+llvm-mlir-op-builder
+====================
+
+Flags usage of old form of invoking create on MLIR's ``OpBuilder`` and suggests
+new form.
+
+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/mlir_op_builder.cpp

@@ -0,0 +1,51 @@
+// RUN: %check_clang_tidy --match-partial-fixes %s llvm-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
+
+void f() {
+  mlir::OpBuilder builder;
+  // CHECK-MESSAGES: :[[@LINE+2]]:3: warning: Use OpType::create(builder, ...) instead of builder.create<OpType>(...) [llvm-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-mlir-op-builder]
+  // CHECK-FIXES: NamedOp::create(builder, builder.getUnknownLoc(), "baz")
+  builder.create<NamedOp>(builder.getUnknownLoc(), "baz");
+
+  mlir::ImplicitLocOpBuilder ib;
+  // Note: extra space in the case where there is no other arguments. Could be
+  // improved, but also clang-format will do that just post.
+  // CHECK-MESSAGES: :[[@LINE+2]]:3: warning: Use OpType::create(builder, ...) instead of builder.create<OpType>(...) [llvm-mlir-op-builder]
+  // CHECK-FIXES: mlir::ModuleOp::create(ib )
+  ib.create<mlir::ModuleOp>();
+}