fix: add shouldBeQualifiedFlag and new printing utils for corresponding cases

Author: rebel-jueonpark

mlir/include/mlir/IR/OpImplementation.h

@@ -20,6 +20,7 @@
 #include "llvm/ADT/Twine.h"
 #include "llvm/Support/SMLoc.h"
 #include <optional>
+#include <type_traits>
 
 namespace {
 // reference https://stackoverflow.com/a/16000226
@@ -203,6 +204,43 @@ class AsmPrinter {
     *this << attrOrType;
   }
 
+  /// Print the provided attribute or type ensuring that any dialect-specific
+  /// prefixes (e.g. `#dialect.mnemonic`) are retained. This is used in
+  /// contexts, such as structured property dictionaries, where the fully
+  /// qualified form is required for disambiguation.
+  template <typename AttrOrType,
+            std::enable_if_t<std::is_convertible_v<AttrOrType, Attribute>> *
+                sfinae = nullptr>
+  void printQualifiedAttrOrType(AttrOrType attrOrType) {
+    printAttribute(attrOrType);
+  }
+
+  template <typename AttrOrType,
+            std::enable_if_t<std::is_convertible_v<AttrOrType, Type> &&
+                             !std::is_convertible_v<AttrOrType, Attribute>> *
+                sfinae = nullptr>
+  void printQualifiedAttrOrType(AttrOrType attrOrType) {
+    printType(attrOrType);
+  }
+
+  template <typename ElementT,
+            std::enable_if_t<std::is_convertible_v<ElementT, Attribute> ||
+                             std::is_convertible_v<ElementT, Type>> *sfinae =
+                nullptr>
+  void printQualifiedAttrOrType(ArrayRef<ElementT> attrOrTypes) {
+    llvm::interleaveComma(attrOrTypes, getStream(), [&](ElementT element) {
+      printQualifiedAttrOrType(element);
+    });
+  }
+
+  template <typename AttrOrType,
+            std::enable_if_t<!std::is_convertible_v<AttrOrType, Attribute> &&
+                             !std::is_convertible_v<AttrOrType, Type>> *
+                sfinae = nullptr>
+  void printQualifiedAttrOrType(AttrOrType attrOrType) {
+    *this << attrOrType;
+  }
+
   /// Print the given attribute without its type. The corresponding parser must
   /// provide a valid type for the attribute.
   virtual void printAttributeWithoutType(Attribute attr);

mlir/test/mlir-tblgen/op-format.mlir

@@ -352,14 +352,14 @@ module attributes {test.internal = #test.internal<key = 8, value = 9>} {
 
 //-----
 
-// CHECK: module attributes {test.external = #test.external<internal = <key = 1, value = 2>>} {
+// CHECK: module attributes {test.external = #test.external<internal = #test.internal<key = 1, value = 2>>} {
 // CHECK-NEXT: }
 module attributes {test.external = #test.external<internal = #test.internal<key = 1, value = 2>>} {
 }
 
 //-----
 
-// CHECK: module attributes {test.external_array = #test.external_array<[internals = <key = 1, value = 2>, <key = 8, value = 9>]>} {
+// CHECK: module attributes {test.external_array = #test.external_array<[internals = #test.internal<key = 1, value = 2>, #test.internal<key = 8, value = 9>]>} {
 // CHECK-NEXT: }
 module attributes {test.external_array = #test.external_array<[internals = #test.internal<key = 1, value = 2>, #test.internal<key = 8, value = 9>]>} {
 }

mlir/tools/mlir-tblgen/AttrOrTypeFormatGen.cpp

@@ -141,6 +141,15 @@ class StructDirective
 
 } // namespace
 
+/// Return true if the provided parameter should always be printed in qualified
+/// form (i.e., with dialect/type prefixes). Attribute and type parameters fall
+/// into this category to avoid ambiguity when nested within structured
+/// properties.
+static bool shouldPrintQualified(ParameterElement *param) {
+  StringRef cppType = param->getParam().getCppType();
+  return cppType.contains("Attr") || cppType.contains("Type");
+}
+
 //===----------------------------------------------------------------------===//
 // Format Strings
 //===----------------------------------------------------------------------===//
@@ -155,7 +164,8 @@ static const char *const defaultParameterPrinter =
 
 /// Qualified printer for attribute or type parameters: it does not elide
 /// dialect and mnemonic.
-static const char *const qualifiedParameterPrinter = "$_printer << $_self";
+static const char *const qualifiedParameterPrinter =
+  "$_printer.printQualifiedAttrOrType($_self)";
 
 /// Print an error when failing to parse an element.
 ///
@@ -849,13 +859,13 @@ static void guardOnAnyOptional(FmtContext &ctx, MethodBody &os,
 }
 
 void DefFormat::genCommaSeparatedPrinter(
-    ArrayRef<FormatElement *> args, FmtContext &ctx, MethodBody &os,
+    ArrayRef<FormatElement *> params, FmtContext &ctx, MethodBody &os,
     function_ref<void(FormatElement *)> extra) {
   // Emit a space if necessary, but only if the struct is present.
   if (shouldEmitSpace || !lastWasPunctuation) {
-    bool allOptional = llvm::all_of(args, formatIsOptional);
+    bool allOptional = llvm::all_of(params, formatIsOptional);
     if (allOptional)
-      guardOnAnyOptional(ctx, os, args);
+      guardOnAnyOptional(ctx, os, params);
     os << tgfmt("$_printer << ' ';\n", &ctx);
     if (allOptional)
       os.unindent() << "}\n";
@@ -864,14 +874,16 @@ void DefFormat::genCommaSeparatedPrinter(
   // The first printed element does not need to emit a comma.
   os << "{\n";
   os.indent() << "bool _firstPrinted = true;\n";
-  for (FormatElement *arg : args) {
+  for (FormatElement *arg : params) {
     ParameterElement *param = getEncapsulatedParameterElement(arg);
     if (param->isOptional()) {
       param->genPrintGuard(ctx, os << "if (") << ") {\n";
       os.indent();
     }
     os << tgfmt("if (!_firstPrinted) $_printer << \", \";\n", &ctx);
     os << "_firstPrinted = false;\n";
+    if (param && shouldPrintQualified(param))
+      param->setShouldBeQualified();
     extra(arg);
     shouldEmitSpace = false;
     lastWasPunctuation = true;