[Mlir-commits] [mlir] 62bf6c2 - Use `bytes`, not `str`, to return C++ strings to Python.

[Alex Zinenko]

Author: Chris Jones
Date: 2023-04-13T17:09:19+02:00
New Revision: 62bf6c2e1083b8ddc698f5c8b99d6c3614c9b56a

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

LOG: Use `bytes`, not `str`, to return C++ strings to Python.

`str` must be valid UTF-8, which is not guaranteed for C++ strings.

Reviewed By: ftynse

Differential Revision: https://reviews.llvm.org/D147818

Added: 
    

Modified: 
    mlir/lib/Bindings/Python/IRAttributes.cpp
    mlir/test/python/ir/attributes.py
    mlir/test/python/ir/operation.py

Removed: 
    


################################################################################
diff  --git a/mlir/lib/Bindings/Python/IRAttributes.cpp b/mlir/lib/Bindings/Python/IRAttributes.cpp
index d252044c8e65..4a43ffbc0f3e 100644
--- a/mlir/lib/Bindings/Python/IRAttributes.cpp
+++ b/mlir/lib/Bindings/Python/IRAttributes.cpp
@@ -490,9 +490,9 @@ class PyOpaqueAttribute : public PyConcreteAttribute<PyOpaqueAttribute> {
         "data",
         [](PyOpaqueAttribute &self) {
           MlirStringRef stringRef = mlirOpaqueAttrGetData(self);
-          return py::str(stringRef.data, stringRef.length);
+          return py::bytes(stringRef.data, stringRef.length);
         },
-        "Returns the data for the Opaqued attributes as a string");
+        "Returns the data for the Opaqued attributes as `bytes`");
   }
 };
 
@@ -528,6 +528,13 @@ class PyStringAttribute : public PyConcreteAttribute<PyStringAttribute> {
           return py::str(stringRef.data, stringRef.length);
         },
         "Returns the value of the string attribute");
+    c.def_property_readonly(
+        "value_bytes",
+        [](PyStringAttribute &self) {
+          MlirStringRef stringRef = mlirStringAttrGetValue(self);
+          return py::bytes(stringRef.data, stringRef.length);
+        },
+        "Returns the value of the string attribute as `bytes`");
   }
 };
 

diff  --git a/mlir/test/python/ir/attributes.py b/mlir/test/python/ir/attributes.py
index 1e1589d6d5f4..6aad94317e6f 100644
--- a/mlir/test/python/ir/attributes.py
+++ b/mlir/test/python/ir/attributes.py
@@ -248,7 +248,7 @@ def testOpaqueAttr():
     oattr = OpaqueAttr(Attribute.parse("#pytest_dummy.dummyattr<>"))
     # CHECK: oattr value: pytest_dummy
     print("oattr value:", oattr.dialect_namespace)
-    # CHECK: oattr value: dummyattr<>
+    # CHECK: oattr value: b'dummyattr<>'
     print("oattr value:", oattr.data)
 
     # Test factory methods.
@@ -265,6 +265,8 @@ def testStringAttr():
     sattr = StringAttr(Attribute.parse('"stringattr"'))
     # CHECK: sattr value: stringattr
     print("sattr value:", sattr.value)
+    # CHECK: sattr value: b'stringattr'
+    print("sattr value:", sattr.value_bytes)
 
     # Test factory methods.
     # CHECK: default_get: "foobar"

diff  --git a/mlir/test/python/ir/operation.py b/mlir/test/python/ir/operation.py
index 941420e8d1ff..2088e1633c66 100644
--- a/mlir/test/python/ir/operation.py
+++ b/mlir/test/python/ir/operation.py
@@ -516,6 +516,8 @@ def testOperationAttributes():
   print(f"Attribute type {fattr.type}, value {fattr.value}")
   # CHECK: Attribute value text
   print(f"Attribute value {sattr.value}")
+  # CHECK: Attribute value b'text'
+  print(f"Attribute value {sattr.value_bytes}")
 
   # We don't know in which order the attributes are stored.
   # CHECK-DAG: NamedAttribute(dependent="text")