[Mlir-commits] [mlir] [mlir][EmitC] Disallow string attributes as initial values (PR #75310)

[Marius Brehler]

================
@@ -50,6 +50,27 @@ void mlir::emitc::buildTerminatedBody(OpBuilder &builder, Location loc) {
   builder.create<emitc::YieldOp>(loc);
 }
 
+/// Check that the type of the initial value is compatible with the operations
+/// result type.
+static LogicalResult verifyInitializationAttribute(Operation *op,
+                                                   Attribute value) {
+  assert(op->getNumResults() == 1 && "operation must have 1 result");
+
+  if (llvm::isa<emitc::OpaqueAttr>(value))
+    return success();
+
+  if (llvm::isa<StringAttr>(value))
+    return op->emitOpError()
+           << "string attributes are not supported, use #emitc.opaque instead";
+  Type resultType = op->getResult(0).getType();
+  Type attrType = cast<TypedAttr>(value).getType();
+  if (!llvm::isa<NoneType>(attrType) && resultType != attrType)
+    return op->emitOpError()
+           << "requires attribute's type (" << attrType
+           << ") to match op's result type (" << resultType << ")";
----------------
marbre wrote:

As I am on travel and without looking to much into the details... one quick question: I assume that ` %c0 = "emitc.constant"(){value = #emitc.opaque<"NULL">} : () -> !emitc.ptr<i32>` would be valid? If yes, the error message that the attribute's type must match the op's result type might be misleading. If I don't miss something this is only valid for non opaque types. Please correct me if I am wrong, but if not there might be an option to improve the error message.

https://github.com/llvm/llvm-project/pull/75310