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