[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