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

[River Riddle via Phabricator via llvm-commits]

rriddle added a comment.

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 something like: `set<() : ()> ()` or `map<() -> ()> ()`. Is there some other delimiter grouping that would be less clumsy for you? `{}` perhaps? `set{() : ()}()`/`map{() -> ()}()`?

> 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.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72429