Skip to content

Commit 9f53354

Browse files
[mlir][python] Remove __str__ from bindings of StringAttr.
This reverts a feature introduced in commit 2a5d497. The goal of that commit was to allow `StringAttr`s to by used transparently wherever Python `str`s are expected. But, as the tests in https://reviews.llvm.org/D159182 reveal, pybind11 doesn't do this conversion based on `__str__` automatically, unlike for the other types introduced in the commit above. At the same time, changing `__str__` breaks the symmetry with other attributes of `print(attr)` printing the assembly of the attribute, so the change probably has more disadvantages than advantages. Reviewed By: springerm, rkayaith Differential Revision: https://reviews.llvm.org/D159255
1 parent dd48a9b commit 9f53354

File tree

3 files changed

+18
-32
lines changed

3 files changed

+18
-32
lines changed

mlir/lib/Bindings/Python/IRAttributes.cpp

Lines changed: 7 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -596,23 +596,20 @@ class PyStringAttribute : public PyConcreteAttribute<PyStringAttribute> {
596596
},
597597
py::arg("type"), py::arg("value"),
598598
"Gets a uniqued string attribute associated to a type");
599-
c.def_property_readonly("value", toPyStr,
600-
"Returns the value of the string attribute");
599+
c.def_property_readonly(
600+
"value",
601+
[](PyStringAttribute &self) {
602+
MlirStringRef stringRef = mlirStringAttrGetValue(self);
603+
return py::str(stringRef.data, stringRef.length);
604+
},
605+
"Returns the value of the string attribute");
601606
c.def_property_readonly(
602607
"value_bytes",
603608
[](PyStringAttribute &self) {
604609
MlirStringRef stringRef = mlirStringAttrGetValue(self);
605610
return py::bytes(stringRef.data, stringRef.length);
606611
},
607612
"Returns the value of the string attribute as `bytes`");
608-
c.def("__str__", toPyStr,
609-
"Converts the value of the string attribute to a Python str");
610-
}
611-
612-
private:
613-
static py::str toPyStr(PyStringAttribute &self) {
614-
MlirStringRef stringRef = mlirStringAttrGetValue(self);
615-
return py::str(stringRef.data, stringRef.length);
616613
}
617614
};
618615

mlir/test/python/dialects/builtin.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -134,13 +134,13 @@ def testBuildFuncOp():
134134
),
135135
visibility="nested",
136136
)
137-
# CHECK: Name is: some_func
137+
# CHECK: Name is: "some_func"
138138
print("Name is: ", f.name)
139139

140140
# CHECK: Type is: (tensor<2x3x4xf32>, tensor<2x3x4xf32>) -> tensor<2x3x4xf32>
141141
print("Type is: ", f.type)
142142

143-
# CHECK: Visibility is: nested
143+
# CHECK: Visibility is: "nested"
144144
print("Visibility is: ", f.visibility)
145145

146146
try:

mlir/test/python/ir/attributes.py

Lines changed: 9 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ def testParsePrint():
2121
assert t.context is ctx
2222
ctx = None
2323
gc.collect()
24-
# CHECK: hello
24+
# CHECK: "hello"
2525
print(str(t))
2626
# CHECK: StringAttr("hello")
2727
print(repr(t))
@@ -290,25 +290,14 @@ def testStringAttr():
290290
sattr = StringAttr(Attribute.parse('"stringattr"'))
291291
# CHECK: sattr value: stringattr
292292
print("sattr value:", sattr.value)
293-
# CHECK: sattr value_bytes: b'stringattr'
294-
print("sattr value_bytes:", sattr.value_bytes)
295-
# CHECK: sattr str: stringattr
296-
print("sattr str:", str(sattr))
297-
298-
typed_sattr = StringAttr(Attribute.parse('"stringattr" : i32'))
299-
# CHECK: typed_sattr value: stringattr
300-
print("typed_sattr value:", typed_sattr.value)
301-
# CHECK: typed_sattr str: stringattr
302-
print("typed_sattr str:", str(typed_sattr))
293+
# CHECK: sattr value: b'stringattr'
294+
print("sattr value:", sattr.value_bytes)
303295

304296
# Test factory methods.
305-
# CHECK: default_get: StringAttr("foobar")
306-
print("default_get:", repr(StringAttr.get("foobar")))
307-
# CHECK: typed_get: StringAttr("12345" : i32)
308-
print(
309-
"typed_get:",
310-
repr(StringAttr.get_typed(IntegerType.get_signless(32), "12345")),
311-
)
297+
# CHECK: default_get: "foobar"
298+
print("default_get:", StringAttr.get("foobar"))
299+
# CHECK: typed_get: "12345" : i32
300+
print("typed_get:", StringAttr.get_typed(IntegerType.get_signless(32), "12345"))
312301

313302

314303
# CHECK-LABEL: TEST: testNamedAttr
@@ -317,8 +306,8 @@ def testNamedAttr():
317306
with Context():
318307
a = Attribute.parse('"stringattr"')
319308
named = a.get_named("foobar") # Note: under the small object threshold
320-
# CHECK: attr: StringAttr("stringattr")
321-
print("attr:", repr(named.attr))
309+
# CHECK: attr: "stringattr"
310+
print("attr:", named.attr)
322311
# CHECK: name: foobar
323312
print("name:", named.name)
324313
# CHECK: named: NamedAttribute(foobar="stringattr")

0 commit comments

Comments
 (0)