From 62bf6c2e1083b8ddc698f5c8b99d6c3614c9b56a Mon Sep 17 00:00:00 2001
From: Chris Jones <cjfj@deepmind.com>
Date: Thu, 13 Apr 2023 17:05:26 +0200
Subject: [PATCH] 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
---
 mlir/lib/Bindings/Python/IRAttributes.cpp | 11 +++++++++--
 mlir/test/python/ir/attributes.py         |  4 +++-
 mlir/test/python/ir/operation.py          |  2 ++
 3 files changed, 14 insertions(+), 3 deletions(-)

diff --git a/mlir/lib/Bindings/Python/IRAttributes.cpp b/mlir/lib/Bindings/Python/IRAttributes.cpp
index d252044..4a43ffb 100644
--- a/mlir/lib/Bindings/Python/IRAttributes.cpp
+++ b/mlir/lib/Bindings/Python/IRAttributes.cpp
@@ -490,9 +490,9 @@ public:
         "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 @@ public:
           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 1e1589d..6aad943 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 941420e..2088e16 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")
-- 
2.7.4