[Mlir-commits] [mlir] eea1e50 - [mlir-tblgen] trim method body to empty with only spaces to avoid crash (#139568)

[llvmlistbot at llvm.org]

Author: drazi
Date: 2025-05-13T10:03:06+02:00
New Revision: eea1e50ac281d667992aa74c1819cd80b8eb634f

URL: https://github.com/llvm/llvm-project/commit/eea1e50ac281d667992aa74c1819cd80b8eb634f
DIFF: https://github.com/llvm/llvm-project/commit/eea1e50ac281d667992aa74c1819cd80b8eb634f.diff

LOG: [mlir-tblgen] trim method body to empty with only spaces to avoid crash (#139568)

method body or default impl must be true empty. Even they contain only
spaces, ``mlir-tblgen`` considers they are non-empty and generates
invalid code lead to segment fault. It's very hard to debug.

```c++
    InterfaceMethod<
      ...
      /*methodBody=*/  [{ }],    // This must be true empty. Leaving a space here can lead to segment fault which is hard to figure out why
      /*defaultImpl=*/ [{
        ...
      }]
```

This PR trim spaces when method body or default implementation of
interface method is not empty. Now ``mlir-tblgen`` generates valid code
even when they contain only spaces.

---------

Co-authored-by: Fung Xie <ftse at nvidia.com>
Co-authored-by: Mehdi Amini <joker.eph at gmail.com>

Added: 
    mlir/test/mlir-tblgen/method-body-with-only-spaces.td

Modified: 
    mlir/lib/TableGen/Interfaces.cpp

Removed: 
    


################################################################################
diff  --git a/mlir/lib/TableGen/Interfaces.cpp b/mlir/lib/TableGen/Interfaces.cpp
index dc9a74c4e8a90..ff1518dea1f22 100644
--- a/mlir/lib/TableGen/Interfaces.cpp
+++ b/mlir/lib/TableGen/Interfaces.cpp
@@ -51,13 +51,15 @@ bool InterfaceMethod::isStatic() const {
 
 // Return the body for this method if it has one.
 std::optional<StringRef> InterfaceMethod::getBody() const {
-  auto value = def->getValueAsString("body");
+  // Trim leading and trailing spaces from the default implementation.
+  auto value = def->getValueAsString("body").trim();
   return value.empty() ? std::optional<StringRef>() : value;
 }
 
 // Return the default implementation for this method if it has one.
 std::optional<StringRef> InterfaceMethod::getDefaultImplementation() const {
-  auto value = def->getValueAsString("defaultBody");
+  // Trim leading and trailing spaces from the default implementation.
+  auto value = def->getValueAsString("defaultBody").trim();
   return value.empty() ? std::optional<StringRef>() : value;
 }
 

diff  --git a/mlir/test/mlir-tblgen/method-body-with-only-spaces.td b/mlir/test/mlir-tblgen/method-body-with-only-spaces.td
new file mode 100644
index 0000000000000..daa0067e062d5
--- /dev/null
+++ b/mlir/test/mlir-tblgen/method-body-with-only-spaces.td
@@ -0,0 +1,45 @@
+// RUN: mlir-tblgen --gen-type-interface-decls -I %S/../../include %s | FileCheck %s
+
+include "mlir/IR/OpBase.td"
+
+def TestEmptyMethodBodyTypeInterface : TypeInterface<"TestEmptyMethodBodyTypeInterface"> {
+  let cppNamespace = "::TestEmptyMethodBodyTypeInterface";
+  let description = [{
+    Treat method body with only spaces as empty.
+  }];
+  let methods = [
+    InterfaceMethod<
+      /*desc=*/        [{ Trim spaces of method body and default implementation }],
+      /*returnType=*/  "StringRef",
+      /*methodName=*/  "trimSpaces",
+      /*args=*/        (ins),
+      // CHECK-LABEL: StringRef detail::TestEmptyMethodBodyTypeInterfaceInterfaceTraits::Model<ConcreteType>::trimSpaces
+      // CHECK-SAME: {
+      // CHECK-NEXT: return (::llvm::cast<ConcreteType>(tablegen_opaque_val)).trimSpaces();
+      // CHECK-NEXT: }
+      /*methodBody=*/  [{  }],
+      /*defaultImpl=*/ [{ return StringRef(); }]
+    >
+  ];
+}
+
+def TestEmptyDefaultImplTypeInterface : TypeInterface<"TestEmptyDefaultImplTypeInterface"> {
+  let cppNamespace = "::TestEmptyDefaultImplTypeInterface";
+  let description = [{
+    Treat default implementation with only spaces as empty.
+  }];
+
+  let methods = [
+    InterfaceMethod<
+      /*desc=*/        [{ Trim spaces of default implementation }],
+      /*returnType=*/  "StringRef",
+      /*methodName=*/  "trimSpaces",
+      /*args=*/        (ins),
+      /*methodBody=*/  [{ return StringRef(); }],
+      // COM: Don't generate default implementation
+      // CHECK-NOT: StringRef detail::TestEmptyDefaultImplTypeInterfaceInterfaceTraits::ExternalModel<ConcreteModel, ConcreteType>::trimSpaces
+      /*defaultImpl=*/ [{
+      }]
+    >
+  ];
+}