[Mlir-commits] [mlir] [mlir][TableGen] Verify compatibility of tblgen::Method properties (PR #147979)

[Andrei Golubev]

https://github.com/andrey-golubev updated https://github.com/llvm/llvm-project/pull/147979

>From cbf0fe0a67ff85154770325e8b03702342f2260f Mon Sep 17 00:00:00 2001
From: "Golubev, Andrey" <andrey.golubev at intel.com>
Date: Thu, 10 Jul 2025 14:20:47 +0000
Subject: [PATCH 1/2] [mlir][TableGen] Check tblgen::Method properties for
 compatibility

Following a recent discovery of a method being defined both "inline" and
"declaration" (declaration implying no definition), verify the method
properties in general to fail early in the development and avoid
accidental bugs (especially for "opt-in" features).
---
 mlir/include/mlir/TableGen/Class.h | 14 +++++++++++--
 mlir/lib/TableGen/Class.cpp        | 32 ++++++++++++++++++++++++++++++
 2 files changed, 44 insertions(+), 2 deletions(-)

diff --git a/mlir/include/mlir/TableGen/Class.h b/mlir/include/mlir/TableGen/Class.h
index f750a34a3b2ba..8000c07b52b2e 100644
--- a/mlir/include/mlir/TableGen/Class.h
+++ b/mlir/include/mlir/TableGen/Class.h
@@ -332,13 +332,19 @@ class Method : public ClassDeclarationBase<ClassDeclaration::Method> {
       : properties(properties),
         methodSignature(std::forward<RetTypeT>(retType),
                         std::forward<NameT>(name), std::forward<Args>(args)...),
-        methodBody(properties & Declaration) {}
+        methodBody(properties & Declaration) {
+    assert(methodPropertiesAreCompatible(properties) &&
+           "invalid combination of properties specified");
+  }
   /// Create a method with a return type, a name, method properties, and a list
   /// of parameters.
   Method(StringRef retType, StringRef name, Properties properties,
          std::initializer_list<MethodParameter> params)
       : properties(properties), methodSignature(retType, name, params),
-        methodBody(properties & Declaration) {}
+        methodBody(properties & Declaration) {
+    assert(methodPropertiesAreCompatible(properties) &&
+           "invalid combination of properties specified");
+  }
 
   // Define move constructor and assignment operator to prevent copying.
   Method(Method &&) = default;
@@ -402,6 +408,10 @@ class Method : public ClassDeclarationBase<ClassDeclaration::Method> {
   MethodBody methodBody;
   /// Deprecation message if the method is deprecated.
   std::optional<std::string> deprecationMessage;
+
+  /// Utility method to verify method properties correctness.
+  [[maybe_unused]] static bool
+  methodPropertiesAreCompatible(Properties properties);
 };
 
 /// This enum describes C++ inheritance visibility.
diff --git a/mlir/lib/TableGen/Class.cpp b/mlir/lib/TableGen/Class.cpp
index c65f67d50a47d..81f1aee73a7f0 100644
--- a/mlir/lib/TableGen/Class.cpp
+++ b/mlir/lib/TableGen/Class.cpp
@@ -159,6 +159,38 @@ void Method::writeDefTo(raw_indented_ostream &os, StringRef namePrefix) const {
   os << "}\n\n";
 }
 
+bool Method::methodPropertiesAreCompatible(Properties properties) {
+  const bool isStatic = (properties & Method::Static);
+  const bool isConstructor = (properties & Method::Constructor);
+  // const bool isPrivate = (properties & Method::Private);
+  const bool isDeclaration = (properties & Method::Declaration);
+  const bool isInline = (properties & Method::Inline);
+  const bool isConstexprValue = (properties & Method::ConstexprValue);
+  const bool isConst = (properties & Method::Const);
+
+  // Note: assert to immediately fail and thus simplify debugging.
+  if (isStatic && isConstructor) {
+    assert(false && "constructor cannot be static");
+    return false;
+  }
+  if (isConstructor && isConst) { // albeit constexpr is fine
+    assert(false && "constructor cannot be const");
+    return false;
+  }
+  if (isDeclaration && isInline) {
+    assert(false &&
+           "declaration implies no definition and thus cannot be inline");
+    return false;
+  }
+  if (isDeclaration && isConstexprValue) {
+    assert(false &&
+           "declaration implies no definition and thus cannot be constexpr");
+    return false;
+  }
+
+  return true;
+}
+
 //===----------------------------------------------------------------------===//
 // Constructor definitions
 //===----------------------------------------------------------------------===//

>From 5548348f75a454e94d1a8fc4292abc46079e5ff2 Mon Sep 17 00:00:00 2001
From: "Golubev, Andrey" <andrey.golubev at intel.com>
Date: Fri, 11 Jul 2025 07:49:54 +0000
Subject: [PATCH 2/2] Convert ctor-level assert into report_fatal_error

---
 mlir/include/mlir/TableGen/Class.h | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/mlir/include/mlir/TableGen/Class.h b/mlir/include/mlir/TableGen/Class.h
index 8000c07b52b2e..349ea54954feb 100644
--- a/mlir/include/mlir/TableGen/Class.h
+++ b/mlir/include/mlir/TableGen/Class.h
@@ -333,8 +333,10 @@ class Method : public ClassDeclarationBase<ClassDeclaration::Method> {
         methodSignature(std::forward<RetTypeT>(retType),
                         std::forward<NameT>(name), std::forward<Args>(args)...),
         methodBody(properties & Declaration) {
-    assert(methodPropertiesAreCompatible(properties) &&
-           "invalid combination of properties specified");
+    if (!methodPropertiesAreCompatible(properties)) {
+      llvm::report_fatal_error(
+          "Invalid combination of method properties specified");
+    }
   }
   /// Create a method with a return type, a name, method properties, and a list
   /// of parameters.
@@ -342,8 +344,10 @@ class Method : public ClassDeclarationBase<ClassDeclaration::Method> {
          std::initializer_list<MethodParameter> params)
       : properties(properties), methodSignature(retType, name, params),
         methodBody(properties & Declaration) {
-    assert(methodPropertiesAreCompatible(properties) &&
-           "invalid combination of properties specified");
+    if (!methodPropertiesAreCompatible(properties)) {
+      llvm::report_fatal_error(
+          "Invalid combination of method properties specified");
+    }
   }
 
   // Define move constructor and assignment operator to prevent copying.