[PATCH] D152998: [TableGen] Support named arguments

Tue Jul 11 01:08:22 PDT 2023

wangpc marked 6 inline comments as done.
wangpc added inline comments.

================
Comment at: llvm/include/llvm/TableGen/Record.h:516
 // Represent a positional argument.
 class PositionalArgumentInit : public ArgumentInit, public FoldingSetNode {
   unsigned Index;
----------------
reames wrote:
> I'm not 100% on this, but I don't think sub-classing is the right model here.  I think this code would be a lot easier to follow with a single ArgumentInit class with an optional index, an optional name, and a value.  A single isPositionalArg() method then let's you dispatch in the few places you need to.
The original patch was just like what you said when I started this patch. The complexity won't disappear, it just moves from sub-typing to branches I think.

================
Comment at: llvm/test/TableGen/named-arguments.td:99
+
+#ifdef ERROR1
+// ERROR1: Argument "d" doesn't exist
----------------
MaskRay wrote:
> I wonder whether tablegen parsing doesn't have error recovery so that we need an invocation for every test...
> 
> In clang we test many errors in one invocation...
Yes, there is no error recovery for TableGen parser…
The parser is a really simple recursive descent parser for LL(1) grammar.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D152998