[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