[PATCH] D72429: [mlir] Change the syntax of AffineMapAttr and IntegerSetAttr to avoid conflicts with function types.

[Uday Bondhugula via Phabricator via llvm-commits]

bondhugula added a comment.

In D72429#1811324 <https://reviews.llvm.org/D72429#1811324>, @rriddle wrote:

> In D72429#1811319 <https://reviews.llvm.org/D72429#1811319>, @bondhugula wrote:
>
> > How about just using a space after the keyword instead of angular brackets? The angular bracket right before/after the parenthesis looks a bit clumsy, and there is no specialization being implied on the
>
>
> I don't see it as a specialization, it is a grouping mechanism that clearly delineate the scope of the attribute. We use this in many other places and doesn't imply that. I personally find the space to create a clumsier syntax as it makes it more difficult for me to tell what is actually part of the syntax. For example, there are many cases where we end up something like `() : () ()` or `(...) -> (...) (...)`. For me it is much easier to read


Hmm... I didn't see the delineation to be an issue because wherever the map typically appears, it either ends a line (in outlined definitions), has a trailing comma or '>', or often has a non-empty operand list following (%x, %y).

> Is there some other delimiter grouping that would be less clumsy for you? `{}` perhaps? `set{() : ()}()`/`map{() -> ()}()`?

I think the angular brackets are better or as good as any other delimiter.

> 
> 
>> map/set. It's a character less with the space FWIW. And we could drop affine_ as well - those are the only maps and sets, and it prevents repetition at several places (like affine.apply affine_map, affine.if > affine_set ...,  affine.for %i = affine_map...).
> 
> Happy to drop the 'affine_', but it should be noted that this only removed ambiguity for the affine dialect. There are other users of these attributes that don't have this problem.

Note that the ones I listed above are the most common users; those that appear as attributes on ops without a custom form are far fewer, and those aren't an issue as well when outlined due to the #map and #set prefix.

This is all really a bikeshed.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D72429/new/

https://reviews.llvm.org/D72429