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

[chr1sj0nes]

Summary

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

[chr1sj0nes]

Use bytes, not str, to return C++ opaque attributes to Python, and add value_bytes for string attributes.

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

[chr1sj0nes]

Thanks for the review. Please can somebody submit the change? I don't think I have permissions.

mlir/lib/Bindings/Python/IRAttributes.cpp

c.def_property_readonly(
           MlirStringRef stringRef = mlirOpaqueAttrGetDialectNamespace(self);
           return py::str(stringRef.data, stringRef.length);
         },
         "Returns the dialect namespace for the Opaque attribute as a string");
     c.def_property_readonly(
         "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`");
   }
 };
 
 class PyStringAttribute : public PyConcreteAttribute<PyStringAttribute> {
 public:
   static constexpr IsAFunctionTy isaFunction = mlirAttributeIsAString;
   static constexpr const char *pyClassName = "StringAttr";
   using PyConcreteAttribute::PyConcreteAttribute;
c.def_static(
         "Gets a uniqued string attribute associated to a type");
     c.def_property_readonly(
         "value",
         [](PyStringAttribute &self) {
           MlirStringRef stringRef = mlirStringAttrGetValue(self);
           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`");
   }
 };
 
 // TODO: Support construction of string elements.
 class PyDenseElementsAttribute
     : public PyConcreteAttribute<PyDenseElementsAttribute> {
 public:
   static constexpr IsAFunctionTy isaFunction = mlirAttributeIsADenseElements;

mlir/test/python/ir/attributes.py

 # CHECK-LABEL: TEST: testOpaqueAttr
 @run
 def testOpaqueAttr():
   with Context() as ctx:
     ctx.allow_unregistered_dialects = True
     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.
     # CHECK: default_get: #foobar<123>
     print(
         "default_get:",
         OpaqueAttr.get("foobar", bytes("123", "utf-8"), NoneType.get()))
 
 
 # CHECK-LABEL: TEST: testStringAttr
 @run
 def testStringAttr():
   with Context() as ctx:
     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"
     print("default_get:", StringAttr.get("foobar"))
     # CHECK: typed_get: "12345" : i32
     print("typed_get:", StringAttr.get_typed(
         IntegerType.get_signless(32), "12345"))
 

mlir/test/python/ir/operation.py

def testOperationAttributes():
   fattr = FloatAttr(op.attributes["other.attribute"])
   sattr = StringAttr(op.attributes["dependent"])
   # CHECK: Attribute type i8, value 1
   print(f"Attribute type {iattr.type}, value {iattr.value}")
   # CHECK: Attribute type f64, value 3.0
   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")
   # CHECK-DAG: NamedAttribute(other.attribute=3.000000e+00 : f64)
   # CHECK-DAG: NamedAttribute(some.attribute=1 : i8)
   for attr in op.attributes:
     print(str(attr))