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

Lei Zhang via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Feb 13 09:01:29 PST 2020


antiagainst added a comment.

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