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

[Mehdi AMINI via Phabricator via llvm-commits]

mehdi_amini added a comment.

LGTM, if @antiagainst approves.



================
Comment at: mlir/test/mlir-tblgen/op-attribute.td:64
+// DEF-NEXT:   this->setAttr("cAttr", attr);
+
 // Test build methods
----------------
AlexEichenberger wrote:
> mehdi_amini wrote:
> > It is a bit annoying that a setters isn't prefixed with `set`: in general I rather have method starting with a verb expressing their actions. (This is also the general style in MLIR I believe).
> > 
> > That said we already emit the getters without a `get` similarly, so this is in line with what is already there.
> > 
> Would you like me to add the set, e.g. setPadAttr(...) or leave the proposal as is?
I'd leave it as-is for now, as it is aligned with the getter.
It is also a problem with the style (how do we make it CamelCase after the set?)

Seems like it goes beyond this revision, @antiagainst do you agree?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74143