steffenlarsen added inline comments.
================
Comment at: clang/lib/Parse/ParseDecl.cpp:447
Actions.CorrectDelayedTyposInExpr(ParseAssignmentExpression()));
+
+ if (Tok.is(tok::ellipsis)) {
----------------
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?
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
