Store a MlirIdentifier instead of a MlirStringRef in MlirNameAttribute

[mehdi_amini]

Summary

This mirror the C++ API for NamedAttribute, and has the advantage or
internalizing earlier in the Context and not requiring the caller to
keep the StringRef alive beyong this call.

[stellaraccident]

Thanks - there was a TODO somewhere for this sharp edge

mlir/include/mlir-c/IR.h

 #undef DEFINE_C_API_STRUCT
 
 /** Named MLIR attribute.
  *
  * A named attribute is essentially a (name, attribute) pair where the name is
  * a string.
  */
 struct MlirNamedAttribute {
-  MlirStringRef name;
+  MlirIdentifier name;
   MlirAttribute attribute;
 };
 typedef struct MlirNamedAttribute MlirNamedAttribute;
 
 //===----------------------------------------------------------------------===//
 // Context API.
 //===----------------------------------------------------------------------===//
 

 MLIR_CAPI_EXPORTED void mlirAttributePrint(MlirAttribute attr,
                                            MlirStringCallback callback,
                                            void *userData);
 
 /// Prints the attribute to the standard error stream.
 MLIR_CAPI_EXPORTED void mlirAttributeDump(MlirAttribute attr);
 
 /// Associates an attribute with the name. Takes ownership of neither.
-MLIR_CAPI_EXPORTED MlirNamedAttribute mlirNamedAttributeGet(MlirStringRef name,
+MLIR_CAPI_EXPORTED MlirNamedAttribute mlirNamedAttributeGet(MlirIdentifier name,
                                                             MlirAttribute attr);
 
 //===----------------------------------------------------------------------===//
 // Identifier API.
 //===----------------------------------------------------------------------===//
 
 /// Gets an identifier with the given string value.
 MLIR_CAPI_EXPORTED MlirIdentifier mlirIdentifierGet(MlirContext context,

mlir/lib/Bindings/Python/IRModules.cpp

mlirOperationStateAddResults(&state, mlirResults.size(),
                                  mlirResults.data());
   if (!mlirAttributes.empty()) {
     // Note that the attribute names directly reference bytes in
     // mlirAttributes, so that vector must not be changed from here
     // on.
     llvm::SmallVector<MlirNamedAttribute, 4> mlirNamedAttributes;
     mlirNamedAttributes.reserve(mlirAttributes.size());
     for (auto &it : mlirAttributes)
-      mlirNamedAttributes.push_back(
-          mlirNamedAttributeGet(toMlirStringRef(it.first), it.second));
+      mlirNamedAttributes.push_back(mlirNamedAttributeGet(
+          mlirIdentifierGet(mlirAttributeGetContext(it.second),
+                            toMlirStringRef(it.first)),
+          it.second));
     mlirOperationStateAddAttributes(&state, mlirNamedAttributes.size(),
                                     mlirNamedAttributes.data());
   }
   if (!mlirSuccessors.empty())
     mlirOperationStateAddSuccessors(&state, mlirSuccessors.size(),
                                     mlirSuccessors.data());
   if (regions) {
     llvm::SmallVector<MlirRegion, 4> mlirRegions;

 }
 
 //------------------------------------------------------------------------------
 // PyNamedAttribute.
 //------------------------------------------------------------------------------
 
 PyNamedAttribute::PyNamedAttribute(MlirAttribute attr, std::string ownedName)
     : ownedName(new std::string(std::move(ownedName))) {
-  namedAttr = mlirNamedAttributeGet(toMlirStringRef(*this->ownedName), attr);
+  namedAttr = mlirNamedAttributeGet(
+      mlirIdentifierGet(mlirAttributeGetContext(attr),
+                        toMlirStringRef(*this->ownedName)),
+      attr);
 }
 
 //------------------------------------------------------------------------------
 // PyType.
 //------------------------------------------------------------------------------
 
 bool PyType::operator==(const PyType &other) {
   return mlirTypeEqual(type, other.type);
public:
 
   PyNamedAttribute dunderGetItemIndexed(intptr_t index) {
     if (index < 0 || index >= dunderLen()) {
       throw SetPyError(PyExc_IndexError,
                        "attempt to access out of bounds attribute");
     }
     MlirNamedAttribute namedAttr =
         mlirOperationGetAttribute(operation->get(), index);
-    return PyNamedAttribute(namedAttr.attribute,
-                            std::string(namedAttr.name.data));
+    return PyNamedAttribute(
+        namedAttr.attribute,
+        std::string(mlirIdentifierStr(namedAttr.name).data));
   }
 
   void dunderSetItem(const std::string &name, PyAttribute attr) {
     mlirOperationSetAttributeByName(operation->get(), toMlirStringRef(name),
                                     attr);
   }
 
   void dunderDelItem(const std::string &name) {
void mlir::python::populateIRSubmodule(py::module &m) {
   //----------------------------------------------------------------------------
   // Mapping of PyNamedAttribute
   //----------------------------------------------------------------------------
   py::class_<PyNamedAttribute>(m, "NamedAttribute")
       .def("__repr__",
            [](PyNamedAttribute &self) {
              PyPrintAccumulator printAccum;
              printAccum.parts.append("NamedAttribute(");
-             printAccum.parts.append(self.namedAttr.name.data);
+             printAccum.parts.append(
+                 mlirIdentifierStr(self.namedAttr.name).data);
              printAccum.parts.append("=");
              mlirAttributePrint(self.namedAttr.attribute,
                                 printAccum.getCallback(),
                                 printAccum.getUserData());
              printAccum.parts.append(")");
              return printAccum.join();
            })
       .def_property_readonly(
           "name",
           [](PyNamedAttribute &self) {
-            return py::str(self.namedAttr.name.data,
-                           self.namedAttr.name.length);
+            return py::str(mlirIdentifierStr(self.namedAttr.name).data,
+                           mlirIdentifierStr(self.namedAttr.name).length);
           },
           "The name of the NamedAttribute binding")
       .def_property_readonly(
           "attr",
           [](PyNamedAttribute &self) {
             // TODO: When named attribute is removed/refactored, also remove
             // this constructor (it does an inefficient table lookup).
             auto contextRef = PyMlirContext::forContext(

mlir/lib/CAPI/IR/BuiltinAttributes.cpp

 intptr_t mlirDictionaryAttrGetNumElements(MlirAttribute attr) {
   return static_cast<intptr_t>(unwrap(attr).cast<DictionaryAttr>().size());
 }
 
 MlirNamedAttribute mlirDictionaryAttrGetElement(MlirAttribute attr,
                                                 intptr_t pos) {
   NamedAttribute attribute =
       unwrap(attr).cast<DictionaryAttr>().getValue()[pos];
-  return {wrap(attribute.first.strref()), wrap(attribute.second)};
+  return {wrap(attribute.first), wrap(attribute.second)};
 }
 
 MlirAttribute mlirDictionaryAttrGetElementByName(MlirAttribute attr,
                                                  MlirStringRef name) {
   return wrap(unwrap(attr).cast<DictionaryAttr>().get(unwrap(name)));
 }
 
 //===----------------------------------------------------------------------===//

mlir/lib/CAPI/IR/IR.cpp

 }
 
 intptr_t mlirOperationGetNumAttributes(MlirOperation op) {
   return static_cast<intptr_t>(unwrap(op)->getAttrs().size());
 }
 
 MlirNamedAttribute mlirOperationGetAttribute(MlirOperation op, intptr_t pos) {
   NamedAttribute attr = unwrap(op)->getAttrs()[pos];
-  return MlirNamedAttribute{wrap(attr.first.strref()), wrap(attr.second)};
+  return MlirNamedAttribute{wrap(attr.first), wrap(attr.second)};
 }
 
 MlirAttribute mlirOperationGetAttributeByName(MlirOperation op,
                                               MlirStringRef name) {
   return wrap(unwrap(op)->getAttr(unwrap(name)));
 }
 
 void mlirOperationSetAttributeByName(MlirOperation op, MlirStringRef name,

 void mlirAttributePrint(MlirAttribute attr, MlirStringCallback callback,
                         void *userData) {
   detail::CallbackOstream stream(callback, userData);
   unwrap(attr).print(stream);
 }
 
 void mlirAttributeDump(MlirAttribute attr) { unwrap(attr).dump(); }
 
-MlirNamedAttribute mlirNamedAttributeGet(MlirStringRef name,
+MlirNamedAttribute mlirNamedAttributeGet(MlirIdentifier name,
                                          MlirAttribute attr) {
   return MlirNamedAttribute{name, attr};
 }
 
 //===----------------------------------------------------------------------===//
 // Identifier API.
 //===----------------------------------------------------------------------===//
 

mlir/test/CAPI/ir.c

MlirModule makeAndDumpAdd(MlirContext ctx, MlirLocation location) {
   mlirRegionAppendOwnedBlock(funcBodyRegion, funcBody);
 
   MlirAttribute funcTypeAttr = mlirAttributeParseGet(
       ctx,
       mlirStringRefCreateFromCString("(memref<?xf32>, memref<?xf32>) -> ()"));
   MlirAttribute funcNameAttr =
       mlirAttributeParseGet(ctx, mlirStringRefCreateFromCString("\"add\""));
   MlirNamedAttribute funcAttrs[] = {
-      mlirNamedAttributeGet(mlirStringRefCreateFromCString("type"),
-                            funcTypeAttr),
-      mlirNamedAttributeGet(mlirStringRefCreateFromCString("sym_name"),
-                            funcNameAttr)};
+      mlirNamedAttributeGet(
+          mlirIdentifierGet(ctx, mlirStringRefCreateFromCString("type")),
+      mlirNamedAttributeGet(
+          mlirIdentifierGet(ctx, mlirStringRefCreateFromCString("sym_name")),
   MlirOperationState funcState =
       mlirOperationStateGet(mlirStringRefCreateFromCString("func"), location);
   mlirOperationStateAddAttributes(&funcState, 2, funcAttrs);
   mlirOperationStateAddOwnedRegions(&funcState, 1, &funcBodyRegion);
   MlirOperation func = mlirOperationCreate(&funcState);
   mlirBlockInsertOwnedOperation(moduleBody, 0, func);
 
   MlirType indexType =
       mlirTypeParseGet(ctx, mlirStringRefCreateFromCString("index"));
   MlirAttribute indexZeroLiteral =
       mlirAttributeParseGet(ctx, mlirStringRefCreateFromCString("0 : index"));
   MlirNamedAttribute indexZeroValueAttr = mlirNamedAttributeGet(
-      mlirStringRefCreateFromCString("value"), indexZeroLiteral);
+      mlirIdentifierGet(ctx, mlirStringRefCreateFromCString("value")),
+      indexZeroLiteral);
   MlirOperationState constZeroState = mlirOperationStateGet(
       mlirStringRefCreateFromCString("std.constant"), location);
   mlirOperationStateAddResults(&constZeroState, 1, &indexType);
   mlirOperationStateAddAttributes(&constZeroState, 1, &indexZeroValueAttr);
   MlirOperation constZero = mlirOperationCreate(&constZeroState);
   mlirBlockAppendOwnedOperation(funcBody, constZero);
 
   MlirValue funcArg0 = mlirBlockGetArgument(funcBody, 0);
   MlirValue constZeroValue = mlirOperationGetResult(constZero, 0);
   MlirValue dimOperands[] = {funcArg0, constZeroValue};
   MlirOperationState dimState = mlirOperationStateGet(
       mlirStringRefCreateFromCString("std.dim"), location);
   mlirOperationStateAddOperands(&dimState, 2, dimOperands);
   mlirOperationStateAddResults(&dimState, 1, &indexType);
   MlirOperation dim = mlirOperationCreate(&dimState);
   mlirBlockAppendOwnedOperation(funcBody, dim);
 
   MlirRegion loopBodyRegion = mlirRegionCreate();
   MlirBlock loopBody = mlirBlockCreate(/*nArgs=*/1, &indexType);
   mlirRegionAppendOwnedBlock(loopBodyRegion, loopBody);
 
   MlirAttribute indexOneLiteral =
       mlirAttributeParseGet(ctx, mlirStringRefCreateFromCString("1 : index"));
   MlirNamedAttribute indexOneValueAttr = mlirNamedAttributeGet(
-      mlirStringRefCreateFromCString("value"), indexOneLiteral);
+      mlirIdentifierGet(ctx, mlirStringRefCreateFromCString("value")),
+      indexOneLiteral);
   MlirOperationState constOneState = mlirOperationStateGet(
       mlirStringRefCreateFromCString("std.constant"), location);
   mlirOperationStateAddResults(&constOneState, 1, &indexType);
   mlirOperationStateAddAttributes(&constOneState, 1, &indexOneValueAttr);
   MlirOperation constOne = mlirOperationCreate(&constOneState);
   mlirBlockAppendOwnedOperation(funcBody, constOne);
 
   MlirValue dimValue = mlirOperationGetResult(dim, 0);
static void printFirstOfEach(MlirContext ctx, MlirOperation operation) {
   // Get the attribute by index.
   MlirNamedAttribute namedAttr0 = mlirOperationGetAttribute(operation, 0);
   fprintf(stderr, "Get attr 0: ");
   mlirAttributePrint(namedAttr0.attribute, printToStderr, NULL);
   fprintf(stderr, "\n");
   // CHECK: Get attr 0: 0 : index
 
   // Now re-get the attribute by name.
-  MlirAttribute attr0ByName =
-      mlirOperationGetAttributeByName(operation, namedAttr0.name);
+  MlirAttribute attr0ByName = mlirOperationGetAttributeByName(
+      operation, mlirIdentifierStr(namedAttr0.name));
   fprintf(stderr, "Get attr 0 by name: ");
   mlirAttributePrint(attr0ByName, printToStderr, NULL);
   fprintf(stderr, "\n");
   // CHECK: Get attr 0 by name: 0 : index
 
   // Get a non-existing attribute and assert that it is null (sanity).
   fprintf(stderr, "does_not_exist is null: %d\n",
           mlirAttributeIsNull(mlirOperationGetAttributeByName(