erichkeane added inline comments.
================
Comment at: clang/lib/Parse/ParseDecl.cpp:447
Actions.CorrectDelayedTyposInExpr(ParseAssignmentExpression()));
+
+ if (Tok.is(tok::ellipsis)) {
----------------
steffenlarsen wrote:
> erichkeane wrote:
> > steffenlarsen wrote:
> > > erichkeane wrote:
> > > > steffenlarsen wrote:
> > > > > erichkeane wrote:
> > > > > > So I was thinking about this overnight... I wonder if this logic is
> > > > > > inverted here? Aaron can better answer, but I wonder if we should
> > > > > > be instead detecting when we are on the 'last' parameter and it is
> > > > > > one of these `VariadicExprArgument` things (that accept a pack),
> > > > > > then dropping the parser into a loop of:
> > > > > >
> > > > > > while (Tok.is(tok::comma)){
> > > > > > Tok.Consume();
> > > > > > ArgExpr = CorrectTypos(ParseAssignmentExpr(...));
> > > > > > }
> > > > > > // finally, consume the closing paren.
> > > > > >
> > > > > > WDYT?
> > > > > The parsing of attribute arguments is already this lenient. It does
> > > > > not actually care how many arguments the attribute says it takes
> > > > > (unless it takes a type and then it will only currently supply _one_
> > > > > argument). It simply consumes as many expressions it can before
> > > > > reaching the end parenthesis, then leaves the attributes to consume
> > > > > them in `clang/lib/Sema/SemaDeclAttr.cpp`.
> > > > > As a result we do not need any additional work here to handle
> > > > > variadic arguments, but it also means that we have to introduce the
> > > > > restrictions that we can only allow packs in the last argument and
> > > > > allow only that argument to be variadic, as otherwise we have no way
> > > > > of knowing when and where the packs pass between argument boundaries.
> > > > My point is, I don't think THIS function should be handling the
> > > > ellipsis, the expression parsing functions we send the parsing of each
> > > > component off to should do that.
> > > >
> > > > We unfortunately cannot move the entirety of this parsing section to do
> > > > that, since 'identifier'/'type', and 'Function' argument types end up
> > > > needing special handling, but it would be nice if our 'expr' types
> > > > would all be able to do this, and I suggested earlier that we could
> > > > start with the `VariadicExprArgument` to make this an easier patch that
> > > > didn't have to deal with the issues that come with that.
> > > >
> > > > That said, it would be nice if we could get CLOSE to that.
> > > Sorry. I am still not sure I am following. Do you mean the consumption of
> > > pack expressions should be moved into the expression parsing so other
> > > places (not just attributes) would accept it? If so, I am not opposed to
> > > the idea though I have no idea which expressions these would be nor the
> > > full implications of such a change. For these we at least have the
> > > isolation of having to explicitly support the packs in the given
> > > attributes.
> > I'm saying the other expression parsers should ALREADY properly handle
> > packs, and we should just use those.
> I think I see what you mean. There is the `Parser::ParseExpressionList` which
> may fit the bill, though it seems to also accept initializer lists. But since
> it is an expression the attributes can decide if it is a valid expression for
> them, so maybe that is not a problem?
Hmm... I don't have a good idea of that, Aaron has better visibility into that
sort of thing.
Part of the advantage to doing it the way I suggest is that it ends up being a
single call (that is, the parser should parse the commas!), so we should only
have to consume the closing paren.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D114439/new/
https://reviews.llvm.org/D114439
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
