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:
> > > 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.
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
