[PATCH] D154066: [NFC][TableGen] Refactor the implementation of arguments

[Artem Belevich via Phabricator via llvm-commits]

tra accepted this revision.
tra added a comment.
This revision is now accepted and ready to land.

LGTM in general, with a nit.



================
Comment at: llvm/include/llvm/TableGen/Record.h:502
+  Init *getValue() const { return Value; }
+  virtual ArgumentInit *changeValue(Init *Value) = 0;
+
----------------
`changeValue()` does not seem to match what the function actually does. 

Change implies that we're modifying something already exsting. Based on the implementation, it appears that the function *creates* a new `ArgumentInit` with the current Index, and the new value. I think the name should reflect it. Perhaps `cloneWithValue()` would match the intent better?

Also, it appears to be implemented for the PositionalArgumentInit only. If it's needed for other types, then we may need a sensible default implementation. If it's not intended to be used in other circumstances, perhaps we can just construct the new PositionalArgumentInit explicitly in the `CheckTemplateArgValues` which is the only place where it's used, and drop the virtual function?



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D154066