[Mlir-commits] [mlir] [mlir][ODS] Populate properties in legacy aggregate builders (PR #219194)

[Mehdi Amini]

================
@@ -2695,6 +2714,82 @@ void OpEmitter::genSeparateArgParamBuilder() {
   }
 }
 
+void OpEmitter::genLegacyPropertiesBuilderHelper() {
+  if (!emitHelper.hasNonEmptyPropertiesStruct())
+    return;
+
+  SmallVector<StringRef> inherentNames;
+  for (const ConstArgument &attrOrProperty : getAttrOrProperties()) {
+    if (const auto *namedAttr =
+            dyn_cast_if_present<const AttributeMetadata *>(attrOrProperty))
+      inherentNames.push_back(namedAttr->attrName);
+    else
+      inherentNames.push_back(
+          cast<const NamedProperty *>(attrOrProperty)->name);
+  }
+  if (emitHelper.getOperandSegmentsSize()) {
+    inherentNames.push_back(legacyOperandSegmentAttrName);
+  }
+  if (emitHelper.getResultSegmentsSize()) {
+    inherentNames.push_back(legacyResultSegmentAttrName);
+  }
+  llvm::sort(inherentNames);
+  inherentNames.erase(llvm::unique(inherentNames), inherentNames.end());
+
+  auto *method = opClass.addStaticMethod<Method::Private>(
+      "void", "buildPropertiesAndDiscardableAttributes",
+      MethodParameter("::mlir::OperationState &", builderOpState),
+      MethodParameter("::llvm::ArrayRef<::mlir::NamedAttribute>",
+                      "attributes"));
+  ERROR_IF_PRUNED(method, "buildPropertiesAndDiscardableAttributes", op);
+  MethodBody &body = method->body();
+  body << "  Properties &properties = " << builderOpStateProperties << ";\n"
+       << "  populateDefaultProperties(" << builderOpState
+       << ".name, properties);\n"
+       << "  ::llvm::SmallVector<::mlir::NamedAttribute> "
+          "inherentAttributes;\n"
+       << "  for (const ::mlir::NamedAttribute &attr : attributes) {\n"
+       << "    ::llvm::StringRef name = attr.getName().getValue();\n"
+       << "    if (";
+  llvm::interleave(
+      inherentNames,
+      [&](StringRef name) { body << "name == \"" << name << "\""; },
+      [&] { body << " || "; });
+  body << ")\n"
+       << "      inherentAttributes.push_back(attr);\n"
+       << "    else\n"
+       << "      " << builderOpState << ".addAttribute(attr.getName(), "
+       << "attr.getValue());\n"
+       << "  }\n"
+       << "  if (inherentAttributes.empty())\n"
+       << "    return;\n"
+       << "  if (::mlir::failed(setPropertiesFromAttr(\n"
----------------
joker-eph wrote:

Agreed. setPropertiesFromAttr is currently the only generic generated conversion path that preserves conversion failures; setInherentAttr is void and isn’t sufficient here. The DictionaryAttr bridge seems like the least awkward compatibility path until properties have a systematic unfreeze operation. I can add a TODO there though?

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