On 12/8/21 13:32, Marek Polacek wrote:
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?
Hmm.
auto( could be the beginning of e.g. auto(*)(), which is also a type-id,
and might trip your assert instead of giving an error.
So I think the latter is the way to go.
I wonder about some time establishing a pattern in the parser that if a
tentative parse results in error_mark_node without simulating an error,
we repeat the same parse again to get the desired semantic error. But
that's a big project, not something to address this bug.
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
