hubert.reinterpretcast added a comment.
In https://reviews.llvm.org/D33339#759125, @rsmith wrote:
> Should I assume our "misplaced ellipsis" diagnostic requires that we
> disambiguate the ill-formed ellipsis-after-declarator-id as a declarator in
> some cases? If so, do we have tests for that somewhere?
Tests for the diagnostics are in `clang/test/FixIt/fixit-cxx0x.cpp` and
`clang/test/Parser/cxx11-templates.cpp`.
The `check-all` target passes even if the ellipsis-after-declarator-id
disambiguation as a declarator is removed entirely. The disambiguation process
is not needed involved in many cases, and when it is, can choose the
declarative interpretation for other reasons.
I find the most interesting cases affected by the patch here to be those
involving disambiguation between the declaration of a function template and
that of a variable template.
The misplaced-ellipsis diagnostic will still trigger (if we accept non-trailing
stray ellipses) on a case like
template <typename ...T> int f(T (t) ...(int), int (x));
which, although it works, it not very motivating.
Removing the `(int)` turns it into a possibly-valid variable template
declaration.
Removing the parentheses around `t` (whether or not the `(int)` is there) makes
the parser decide to parse as a function declaration (and the
misplaced-ellipsis diagnostic is triggered) regardless of the treatment of
"stray" ellipses.
The logic implemented here does not generate the misplaced-ellipsis diagnostic
for
template <typename ...T> int f(T (t) ..., int x);
So, on the whole, the stray ellipsis treatment is both too complicated and not
complicated enough.
================
Comment at: include/clang/Parse/Parser.h:2138
+ TPResult TryParseDeclarator(bool mayBeAbstract, bool mayHaveIdentifier =
true,
+ bool mayHaveTrailingStrayEllipsis = true);
TPResult
----------------
rsmith wrote:
> Given that this flag is enabling parsing of things that are *not* valid
> declarators, I think the default should be the other way around. If some
> calling code believes it's safe to parse a trailing ellipsis as part of a
> declarator, it should be explicitly opting into that.
Okay; will do.
================
Comment at: lib/Parse/ParseTentative.cpp:944
+ (mayHaveTrailingStrayEllipsis ||
+ !(NextToken().is(tok::r_paren) || NextToken().is(tok::comma))))
ConsumeToken();
----------------
rsmith wrote:
> hubert.reinterpretcast wrote:
> > The check for what qualifies as "trailing" is not the strongest, but I find
> > it to be the most straightforward change that should do the job. One
> > alternative is to track whether an ellipsis was consumed within the current
> > loop iteration.
> Use `!NextToken().isOneOf(tok::r_paren, tok::comma)` here.
Will do.
https://reviews.llvm.org/D33339
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
