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

[Jacques Pienaar via Phabricator via llvm-commits]

jpienaar added a comment.

In D74143#1865419 <https://reviews.llvm.org/D74143#1865419>, @mehdi_amini wrote:

> Can this be tested in-tree?


+1, thanks



================
Comment at: mlir/tools/mlir-tblgen/OpDefinitionsGen.cpp:393
+  FmtContext fctx;
+  fctx.withBuilder("mlir::Builder(this->getContext())");
+  auto emitAttrWithStorageType = [&](StringRef name, Attribute attr) {
----------------
AlexEichenberger wrote:
> rriddle wrote:
> > What is the context used for?
> It is used in the same context as the getters. In our case, we use this functionality in a shape inference pass. Is it at risk to vanish before a subsequent pass?
To rephrase the question: fctx does not seemed to be used anywhere here and so could be removed.


================
Comment at: mlir/tools/mlir-tblgen/OpDefinitionsGen.cpp:388-390
+// Generate raw named setter type. This is a wrapper class that allows
+// setting to the attributes via setters instead of having to use
+// the string interface for better compile time verification.
----------------
Reflow?

  // Generate raw named setter type. This is a wrapper class that allows setting
  // the attributes via setters instead of having to use the string interface
  // for better compile time verification.

This comment may also be more relevant inside: e.g., there are potentially multiple setters, as with getters, and this is one specific to raw setters.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74143