On Wed, Dec 08, 2021 at 09:15:05AM -0500, Jason Merrill wrote:
> On 12/7/21 19:25, Marek Polacek wrote:
> > On Mon, Dec 06, 2021 at 04:44:06PM -0500, Jason Merrill wrote:
> > > Please also make this change to cp_parser_sizeof_operand, and add tests
> > > involving sizeof/alignof in array bounds. OK with that change.
> >
> > Turns out we reject sizeof(auto(4)) because cp_parser_type_id_1 errors
> > "invalid use of auto". So I've added a hack to make it work; auto(x)
> > is *not* a type-id, so reject that parse and let it be parsed as an
> > expression.
> >
> > FWIW, I don't think we need to clear
> > auto_is_implicit_function_template_parm_p
> > in cp_parser_sizeof_operand for parameters like int[sizeof(auto(10))]
> > because
> > the auto is in a declarator and auto_is_... will have been cleared already
> > in
> > cp_parser_parameter_declaration before parsing the declarator. But I've
> > added
> > it anyway, maybe there are other cases where it matters.
> >
> > Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk?
> >
> > -- >8 --
> > In C++23, auto(x) is valid, so decltype(auto(x)) should also be valid,
> > so
> >
> > void f(decltype(auto(0)));
> >
> > should be just as
> >
> > void f(int);
> >
> > but currently, everytime we see 'auto' in a parameter-declaration-clause,
> > we try to synthesize_implicit_template_parm for it, creating a new template
> > parameter list. The code above actually has us calling s_i_t_p twice;
> > once from cp_parser_decltype_expr -> cp_parser_postfix_expression which
> > fails and then again from cp_parser_decltype_expr -> cp_parser_expression.
> > So it looks like we have f<auto, auto> and we accept ill-formed code.
> >
> > This shows that we need to be more careful about synthesizing the
> > implicit template parameter. [dcl.spec.auto.general] says that "A
> > placeholder-type-specifier of the form type-constraintopt auto can be
> > used as a decl-specifier of the decl-specifier-seq of a
> > parameter-declaration of a function declaration or lambda-expression..."
> > so this patch turns off auto_is_... after we've parsed the
> > decl-specifier-seq.
> >
> > That doesn't quite cut it yet though, because we also need to handle an
> > auto nested in the decl-specifier:
> >
> > void f(decltype(new auto{0}));
> >
> > therefore the cp_parser_decltype change.
> >
> > To accept "sizeof(auto{10})", the cp_parser_type_id_1 hunk rejects the
> > current parse if it sees an auto followed by a ( or {.
>
> The problem here doesn't seem specific to the ( or {, but that we're giving
> a hard error in tentative parsing context; I think we want to guard that
> error with cp_parser_simulate_error like we do a few lines earlier for class
> template placeholders.
I agree that that's generally the approach that makes sense, but in this
case it regresses our diagnostic :(. For example,
int i = *(auto *) 0;
would give
q.C:1:11: error: expected primary-expression before ‘auto’
1 | int i = *(auto *) 0;
| ^~~~
q.C:1:11: error: expected ‘)’ before ‘auto’
1 | int i = *(auto *) 0;
| ~^~~~
| )
instead of the current
q.C:1:11: error: invalid use of ‘auto’
1 | int i = *(auto *) 0;
| ^~~~
We just reject the parse in cp_parser_type_id_1 and then give an error in
cp_parser_primary_expression:
cp_parser_error (parser, "expected primary-expression");
I suppose I could add 'case RID_AUTO' to cp_parser_primary_expression and
issue an error there, but that doesn't understand decltype(auto) etc, and
still issues multiple error messages.
Or, maybe it would be OK to actually go with the cp_parser_simulate_error
approach and accept that about 5 tests produce somewhat worse diagnostic.
What's your preference?
> > The second hunk broke lambda-generic-85713-2.C but I think the error we
> > issue with this patch is in fact correct, and clang++ agrees.
>
> I don't think this is the second hunk anymore. :)
Ah, fixed.
Marek
