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

[Wang Pengcheng via Phabricator via llvm-commits]

wangpc added inline comments.


================
Comment at: llvm/include/llvm/TableGen/Record.h:502
+  Init *getValue() const { return Value; }
+  virtual ArgumentInit *changeValue(Init *Value) = 0;
+
----------------
tra wrote:
> `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?
> 
This virtual function will be overrided by `PositionalArgumentInit`, and `NamedArgumentInit` in D152998.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D154066