[PATCH] D74143: [MLIR] Adding attribute setters generation for table gen

Alexandre Eichenberger via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Feb 18 08:17:10 PST 2020


AlexEichenberger added a comment.

@ antiagainst

What do you suggest I do? Would you like me to introduce the get and set, and leave the older methods for deprecation? Or only add the setter with "set" in the name?

In D74143#1874608 <https://reviews.llvm.org/D74143#1874608>, @antiagainst wrote:

> I'm concerned about the method name here. Although we can rely on compiler symbol resolution, it's still quite confusing to see two attributes with the same name but one for getters and one for setters. If we also think about the getter method returning "unwrapped" values, that's three.. So if I have a `some_stuff` attribute, we have `some_stuff()`, `some_stuffAttr()`, and `some_stuffAttr(some-params)`. It's also quite sad to see the mix of different naming conventions here. And I think the number of attribute methods is just gonna to grow. It would be far better if we have `getSomeStuff()`, `getSomeStuffAttr()`, and `setSomeStuff()`. FWIW, I still think having proper naming convention and method prefix/postfix control at dialect level is the better way to go given that we don't mandate how attributes should be named for ops.





Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74143





More information about the llvm-commits mailing list