[Mlir-commits] [mlir] [mlir] Support better printing for mutually recursive types (PR #112160)

[Shoaib Meenai]

================
@@ -525,7 +529,7 @@ void TestRecursiveAliasType::print(AsmPrinter &printer) const {
       printer.tryStartCyclicPrint(*this);
----------------
smeenai wrote:

That's a good point. The only concern I have is that the existing `tryStartCyclicPrint` implementation is used to signal that you *must* print a forward declaration, whereas this new check signals that you *may* print a forward declaration. E.g. `TestRecursiveAliasType::parse` has this logic currently:

```cpp
  // If this type already has been parsed above in the stack, expect just the
  // name.
  if (failed(cyclicParse)) {                                                                                                                                                                                                                                                                                                                                                
    if (failed(parser.parseGreater()))
      return Type();
    return rec;
  }
```

With the current implementation, both of the following are valid:

```
# Spell out the mutually recursive type fully
!t1 = !test.test_rec_alias<t1, !test.test_rec_alias<t2, !test.test_rec_alias<t1>>>
!t2 = !test.test_rec_alias<t2, !t1>

# Forward-declare the mutually recursive type
!t1 = !test.test_rec_alias<t1, !test.test_rec_alias<t2>>
!t2 = !test.test_rec_alias<t2, !t1>
```

If we change `tryStartCyclicPrint`, the first one would fail to parse. We could make the new behavior opt-in via a flag, which preserves behavior initially but still doesn't let you distinguish the two cases when you opt-in. We could just accept the new behavior, which I'm warming up to because it limits the number of redundant type definitions which you have to check for consistency. Or we could have a higher-level API that still lets you tell the two cases apart if a dialect needs it. (ClangIR currently prints a self-reference differently from any other incomplete type, but that might be a historical artifact and not an intentional design decision.) I'm fine with any of those options, and I'd appreciate your opinions on them.

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