[Mlir-commits] [mlir] [mlir] Make `TypedStrAttr` actually enforce the string type. (PR #124770)

[Ingo Müller]

https://github.com/ingomueller-net created https://github.com/llvm/llvm-project/pull/124770

The tablgen definition `TypedStrAttr` is an attribute constraints that is meant to restrict the type of a `StringAttr` to the type given as parameter. However, the definition did not previously restrict the type; any `StringAttr` was accepted. This PR makes the definition actually enforce the type.

To test the constraints, the PR also changes the test op that was previously used to test this constraint such that the enforced type is `AnyInteger` instead of `AnyType`. The latter allowed any type, so not enforcing that constraint had no observable effect. The PR then adds a test case with a wrong type and ensures that diagnostics are produced.

>From 12b9d9bb56dd01464cbb8e58037429ae5f9f7162 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Ingo=20M=C3=BCller?= <ingomueller at google.com>
Date: Tue, 28 Jan 2025 13:55:12 +0000
Subject: [PATCH] [mlir] Make `TypedStrAttr` actually enforce the string type.
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

The tablgen definition `TypedStrAttr` is an attribute constraints that
is meant to restrict the type of a `StringAttr` to the type given as
parameter. However, the definition did not previously restrict the type;
any `StringAttr` was accepted. This PR makes the definition actually
enforce the type.

To test the constraints, the PR also changes the test op that was
previously used to test this constraint such that the enforced type is
`AnyInteger` instead of `AnyType`. The latter allowed any type, so not
enforcing that constraint had no observable effect. The PR then adds a
test case with a wrong type and ensures that diagnostics are produced.

Signed-off-by: Ingo Müller <ingomueller at google.com>
---
 mlir/include/mlir/IR/CommonAttrConstraints.td |  6 ++++--
 mlir/test/IR/attribute.mlir                   | 16 ++++++++++++----
 mlir/test/lib/Dialect/Test/TestOps.td         |  2 +-
 3 files changed, 17 insertions(+), 7 deletions(-)

diff --git a/mlir/include/mlir/IR/CommonAttrConstraints.td b/mlir/include/mlir/IR/CommonAttrConstraints.td
index 17ca82c510f8a2..4cd6c84db61df8 100644
--- a/mlir/include/mlir/IR/CommonAttrConstraints.td
+++ b/mlir/include/mlir/IR/CommonAttrConstraints.td
@@ -343,8 +343,10 @@ def SymbolNameAttr : StringBasedAttr<CPred<"::llvm::isa<::mlir::StringAttr>($_se
 
 // String attribute that has a specific value type.
 class TypedStrAttr<Type ty>
-    : StringBasedAttr<CPred<"::llvm::isa<::mlir::StringAttr>($_self)">,
-                            "string attribute"> {
+    : StringBasedAttr<
+        SubstLeaves<"$_self", "::mlir::cast<StringAttr>($_self).getType()",
+                    ty.predicate>,
+        "string attribute of " # ty.summary> {
   let valueType = ty;
 }
 
diff --git a/mlir/test/IR/attribute.mlir b/mlir/test/IR/attribute.mlir
index 0085d64ae82b6b..048dd06d71096f 100644
--- a/mlir/test/IR/attribute.mlir
+++ b/mlir/test/IR/attribute.mlir
@@ -416,10 +416,18 @@ func.func @non_type_in_type_array_attr_fail() {
 // Test StringAttr with custom type
 //===----------------------------------------------------------------------===//
 
-// CHECK-LABEL: func @string_attr_custom_type
-func.func @string_attr_custom_type() {
-  // CHECK: "string_data" : !foo.string
-  test.string_attr_with_type "string_data" : !foo.string
+// CHECK-LABEL: func @string_attr_custom_type_valid
+func.func @string_attr_custom_type_valid() {
+  // CHECK: "string_data" : i64
+  test.string_attr_with_type "string_data" : i64
+  return
+}
+
+// -----
+
+func.func @string_attr_custom_type_invalid() {
+  // expected-error @+1 {{'attr' failed to satisfy constraint: string attribute of integer}}
+  test.string_attr_with_type "string_data" : f32
   return
 }
 
diff --git a/mlir/test/lib/Dialect/Test/TestOps.td b/mlir/test/lib/Dialect/Test/TestOps.td
index f37573c1351cec..84b7f34d37d0f0 100644
--- a/mlir/test/lib/Dialect/Test/TestOps.td
+++ b/mlir/test/lib/Dialect/Test/TestOps.td
@@ -194,7 +194,7 @@ def TypeArrayAttrWithDefaultOp : TEST_Op<"type_array_attr_with_default"> {
 }
 
 def TypeStringAttrWithTypeOp : TEST_Op<"string_attr_with_type"> {
-  let arguments = (ins TypedStrAttr<AnyType>:$attr);
+  let arguments = (ins TypedStrAttr<AnyInteger>:$attr);
   let assemblyFormat = "$attr attr-dict";
 }