[PATCH] D152998: [TableGen][RISCV] Support named arguments

[Philip Reames via Phabricator via llvm-commits]

reames added a comment.

I think this is generally excellent idea.  The overall structure seems to make sense here, but some of the details of TGParser.cpp lost me in a way where I can't give meaningful review feedback.

I'd like to suggest that you split this patch.  A pre-patch which does nothing but add the ArgumentInit class *for positional arguments only* would go a long way to reducing the verbose, but uninteresting parts of this diff.  It would also be entirely NFC, and thus easier to test and review on that basis.  Once that landed - I am offering to review it - this could be rebased, and attention focused on the more semantically useful bits.

On a similar note, I think the extraction of resolveClassArguments and resolveMultiClassArguments could be done as a standalone NFC.  Doing so would eliminate the mixture of code motion and modification in the current patch.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D152998