[Mlir-commits] [mlir] Enable printing newlines and indents in attribute and type printers (PR #87948)

[Łukasz Mielicki]

lmielick wrote:

> > In the proposal it's the attribute printer's implementation choice and it is transparent to the parent operation,
> 
> Right, this is the part I'm not convinced about :)
> 
> I expressed it above already I believe: [#87948 (comment)](https://github.com/llvm/llvm-project/pull/87948#issuecomment-2075833954)

Well, for the use cases I can think of, yes it would be always desired to have the newlines if attribute printer chooses so.
We implemented that because in our case some elaborate attributes were completely unreadable without newlines.
It does compose with op printer, as NLs are contained within attribute syntax.
We also avoid potential surprises in parsing/tools that could stem from newlines being optional.

> > if the operation uses newlines semantically
> 
> Are there precedent for doing this? I don't remember seeing this, and don't even remember we added support for this actually.

Not sure it was ever used semantically for ops. I'm guessing it's ultimately decision of the pretty printer. Not necessarily to my taste, but semantic NLs could be helpful at least for prototyping. NLs in attribute syntax do not seem to impair readability of the ops in our dialect, as the scope of attribute is very clear.

> > We would need to take that into account in attribute printers themselves. Ignoring printNewline/increaseIndent on the attribute depending on context does not seem right.
> 
> I believe the printer itself could elide calls to `printNewline` under the hood.

What I'm worried about is that the such control mechanism may open a can of worms. Not sure how we inform the printer also not sure whether should that be per op or perhaps a knob controlled externally like command line?
We can certainly update the PR based on more direction.

At least in our use case, the elaborate attributes are specific to a handful of ops and we have not encountered any issues with unconditional newlines in these.

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