[PATCH v4] c++: Handle auto(x) in parameter-declaration-clause [PR103401]
Jason Merrill
jason@redhat.com
Thu Dec 9 15:55:44 GMT 2021
On 12/8/21 18:23, Marek Polacek wrote:
> On Wed, Dec 08, 2021 at 03:09:00PM -0500, Jason Merrill wrote:
>> 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.
>
> Ah, yes.
>
>> So I think the latter is the way to go.
>
> Patch attached.
>
> Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk?
OK.
>> 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.
>
> Interesting. How would we handle e.g. the case when sizeof gets something
> that isn't a valid type-id, so we reject the parse, try to parse it as an
> expression, but that fails too -- parse again as a type-id, this time with
> errors? It will probably be hard to say if it's more of a type-id or more
> of an expression. I hope it would improve our parsing of template arguments
> where we sometimes just print "parse error". :]
I was thinking of situations where the argument is a syntactically valid
type-id that violates some constraint, as in this case, so we would
never try to parse as an expression.
> -- >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 only gives a
> hard error when we're not parsing tentatively.
>
> The cp_parser_parameter_declaration hunk broke lambda-generic-85713-2.C but
> I think the error we issue with this patch is in fact correct, and clang++
> agrees.
>
> The r11-1913 change is OK: we need to make sure that we see '(auto)' after
> decltype to go ahead with 'decltype(auto)'.
>
> PR c++/103401
>
> gcc/cp/ChangeLog:
>
> * parser.c (cp_parser_decltype): Clear
> auto_is_implicit_function_template_parm_p.
> (cp_parser_type_id_1): Give errors only when !cp_parser_simulate_error.
> (cp_parser_parameter_declaration): Clear
> auto_is_implicit_function_template_parm_p after parsing the
> decl-specifier-seq.
> (cp_parser_sizeof_operand): Clear
> auto_is_implicit_function_template_parm_p.
>
> gcc/testsuite/ChangeLog:
>
> * g++.dg/cpp1y/lambda-generic-85713-2.C: Add dg-error.
> * g++.dg/cpp1y/pr60054.C: Adjust dg-error.
> * g++.dg/cpp1y/pr60332.C: Likewise.
> * g++.dg/cpp2a/concepts-pr84979-2.C: Likewise.
> * g++.dg/cpp2a/concepts-pr84979-3.C: Likewise.
> * g++.dg/cpp2a/concepts-pr84979.C: Likewise.
> * g++.dg/cpp23/auto-fncast7.C: New test.
> * g++.dg/cpp23/auto-fncast8.C: New test.
> * g++.dg/cpp23/auto-fncast9.C: New test.
> ---
> gcc/cp/parser.c | 38 +++++++++++++----
> .../g++.dg/cpp1y/lambda-generic-85713-2.C | 2 +-
> gcc/testsuite/g++.dg/cpp1y/pr60054.C | 4 +-
> gcc/testsuite/g++.dg/cpp1y/pr60332.C | 3 +-
> gcc/testsuite/g++.dg/cpp23/auto-fncast7.C | 9 ++++
> gcc/testsuite/g++.dg/cpp23/auto-fncast8.C | 42 +++++++++++++++++++
> gcc/testsuite/g++.dg/cpp23/auto-fncast9.C | 17 ++++++++
> .../g++.dg/cpp2a/concepts-pr84979-2.C | 12 +++---
> .../g++.dg/cpp2a/concepts-pr84979-3.C | 12 +++---
> gcc/testsuite/g++.dg/cpp2a/concepts-pr84979.C | 2 +-
> 10 files changed, 116 insertions(+), 25 deletions(-)
> create mode 100644 gcc/testsuite/g++.dg/cpp23/auto-fncast7.C
> create mode 100644 gcc/testsuite/g++.dg/cpp23/auto-fncast8.C
> create mode 100644 gcc/testsuite/g++.dg/cpp23/auto-fncast9.C
>
> diff --git a/gcc/cp/parser.c b/gcc/cp/parser.c
> index 6f273bfe21f..de464afdb54 100644
> --- a/gcc/cp/parser.c
> +++ b/gcc/cp/parser.c
> @@ -16432,6 +16432,16 @@ cp_parser_decltype (cp_parser *parser)
> = parser->greater_than_is_operator_p;
> parser->greater_than_is_operator_p = true;
>
> + /* Don't synthesize an implicit template type parameter here. This
> + could happen with C++23 code like
> +
> + void f(decltype(new auto{0}));
> +
> + where we want to deduce the auto right away so that the parameter
> + is of type 'int *'. */
> + auto cleanup = make_temp_override
> + (parser->auto_is_implicit_function_template_parm_p, false);
> +
> /* Do not actually evaluate the expression. */
> ++cp_unevaluated_operand;
>
> @@ -24144,22 +24154,22 @@ cp_parser_type_id_1 (cp_parser *parser, cp_parser_flags flags,
> /* OK */;
> else
> {
> - location_t loc = type_specifier_seq.locations[ds_type_spec];
> - if (tree tmpl = CLASS_PLACEHOLDER_TEMPLATE (auto_node))
> + if (!cp_parser_simulate_error (parser))
> {
> - if (!cp_parser_simulate_error (parser))
> + location_t loc = type_specifier_seq.locations[ds_type_spec];
> + if (tree tmpl = CLASS_PLACEHOLDER_TEMPLATE (auto_node))
> {
> error_at (loc, "missing template arguments after %qT",
> auto_node);
> inform (DECL_SOURCE_LOCATION (tmpl), "%qD declared here",
> tmpl);
> }
> + else if (parser->in_template_argument_list_p)
> + error_at (loc, "%qT not permitted in template argument",
> + auto_node);
> + else
> + error_at (loc, "invalid use of %qT", auto_node);
> }
> - else if (parser->in_template_argument_list_p)
> - error_at (loc, "%qT not permitted in template argument",
> - auto_node);
> - else
> - error_at (loc, "invalid use of %qT", auto_node);
> return error_mark_node;
> }
> }
> @@ -24668,6 +24678,15 @@ cp_parser_parameter_declaration (cp_parser *parser,
> &decl_specifiers,
> &declares_class_or_enum);
>
> + /* [dcl.spec.auto.general]: "A placeholder-type-specifier of the form
> + type-constraint opt auto can be used as a decl-specifier of the
> + decl-specifier-seq of a parameter-declaration of a function declaration
> + or lambda-expression..." but we must not synthesize an implicit template
> + type parameter in its declarator. That is, in "void f(auto[auto{10}]);"
> + we want to synthesize only the first auto. */
> + auto cleanup = make_temp_override
> + (parser->auto_is_implicit_function_template_parm_p, false);
> +
> /* Complain about missing 'typename' or other invalid type names. */
> if (!decl_specifiers.any_type_specifiers_p
> && cp_parser_parse_and_diagnose_invalid_type_name (parser))
> @@ -32369,6 +32388,9 @@ cp_parser_sizeof_operand (cp_parser* parser, enum rid keyword)
> = parser->non_integral_constant_expression_p;
> parser->integral_constant_expression_p = false;
>
> + auto cleanup = make_temp_override
> + (parser->auto_is_implicit_function_template_parm_p, false);
> +
> /* Do not actually evaluate the expression. */
> ++cp_unevaluated_operand;
> ++c_inhibit_evaluation_warnings;
> diff --git a/gcc/testsuite/g++.dg/cpp1y/lambda-generic-85713-2.C b/gcc/testsuite/g++.dg/cpp1y/lambda-generic-85713-2.C
> index 8fb8dfdeaf0..dbc9e8c732c 100644
> --- a/gcc/testsuite/g++.dg/cpp1y/lambda-generic-85713-2.C
> +++ b/gcc/testsuite/g++.dg/cpp1y/lambda-generic-85713-2.C
> @@ -2,6 +2,6 @@
> // { dg-do compile { target c++14 } }
>
> auto l4 = [](auto v, auto (&array (int)) [5]) -> int { return v + array[0]; };
> -auto l5 = [](auto v, auto (&array (auto)) [5]) -> int { return v + array[0]; };
> +auto l5 = [](auto v, auto (&array (auto)) [5]) -> int { return v + array[0]; }; // { dg-error ".auto. parameter not permitted in this context" }
> auto l6 = [](auto v, auto (&array (int int)) [5]) -> int { return v + array[0]; }; // { dg-error "two or more data types" }
> auto l7 = [](auto v, auto (&array (int auto)) [5]) -> int { return v + array[0]; }; // { dg-error "two or more data types" }
> diff --git a/gcc/testsuite/g++.dg/cpp1y/pr60054.C b/gcc/testsuite/g++.dg/cpp1y/pr60054.C
> index f51120f6425..0d4925afbf2 100644
> --- a/gcc/testsuite/g++.dg/cpp1y/pr60054.C
> +++ b/gcc/testsuite/g++.dg/cpp1y/pr60054.C
> @@ -6,6 +6,6 @@ template<typename T> decltype(T{}) fooB(T);
>
> void bar()
> {
> - fooA((auto*)0); // { dg-error "invalid use" }
> - fooB((auto*)0); // { dg-error "invalid use" }
> + fooA((auto*)0); // { dg-error "expected" }
> + fooB((auto*)0); // { dg-error "expected" }
> }
> diff --git a/gcc/testsuite/g++.dg/cpp1y/pr60332.C b/gcc/testsuite/g++.dg/cpp1y/pr60332.C
> index e75ab8584e3..f3a7980b380 100644
> --- a/gcc/testsuite/g++.dg/cpp1y/pr60332.C
> +++ b/gcc/testsuite/g++.dg/cpp1y/pr60332.C
> @@ -3,4 +3,5 @@
>
> void foo();
>
> -auto f = (auto(*)())(&foo); // { dg-error "invalid" }
> +auto f = (auto(*)())(&foo); // { dg-error "expected" }
> +// { dg-error "only available" "" { target c++20_down } .-1 }
> diff --git a/gcc/testsuite/g++.dg/cpp23/auto-fncast7.C b/gcc/testsuite/g++.dg/cpp23/auto-fncast7.C
> new file mode 100644
> index 00000000000..763164f3e5b
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/cpp23/auto-fncast7.C
> @@ -0,0 +1,9 @@
> +// PR c++/103401
> +// { dg-do compile { target c++23 } }
> +
> +void f(decltype(auto(0))) { }
> +
> +int main()
> +{
> + f<int,int>(0); // { dg-error "no matching function" }
> +}
> diff --git a/gcc/testsuite/g++.dg/cpp23/auto-fncast8.C b/gcc/testsuite/g++.dg/cpp23/auto-fncast8.C
> new file mode 100644
> index 00000000000..9fb7b9c2516
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/cpp23/auto-fncast8.C
> @@ -0,0 +1,42 @@
> +// PR c++/103401
> +// { dg-do compile { target c++23 } }
> +
> +void f1 (decltype(auto(0)));
> +void f2 (decltype(auto{0}));
> +void f3 (int = auto(42));
> +void f4 (int = auto{42});
> +void f5 (decltype(auto(0)) = auto(42));
> +void f6 (auto (x));
> +void f7 (int[auto(10)]);
> +void f8 (int[auto{10}]);
> +void f9 (auto[auto{10}]);
> +void f10 (auto);
> +void f11 (int x, decltype(x) y);
> +void f12 (int[sizeof(auto{10})]);
> +void f13 (int[sizeof(auto(10))]);
> +void f14 (int[__extension__ alignof(auto{10})]);
> +void f15 (int[__extension__ alignof(auto(10))]);
> +
> +void
> +g ()
> +{
> + int a[2];
> + f1 (1);
> + f2 (1);
> + f3 ();
> + f3 (1);
> + f4 ();
> + f4 (1);
> + f5 ();
> + f5 (1);
> + f6 ('a');
> + f7 (&a[0]);
> + f8 (&a[0]);
> + f9 (&a[0]);
> + f10 (1);
> + f11 (1, 2);
> + f12 (&a[0]);
> + f13 (&a[0]);
> + f14 (&a[0]);
> + f15 (&a[0]);
> +}
> diff --git a/gcc/testsuite/g++.dg/cpp23/auto-fncast9.C b/gcc/testsuite/g++.dg/cpp23/auto-fncast9.C
> new file mode 100644
> index 00000000000..12a0dcece75
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/cpp23/auto-fncast9.C
> @@ -0,0 +1,17 @@
> +// PR c++/103401
> +// { dg-do compile { target c++23 } }
> +
> +void f1(decltype(new auto{0}));
> +void f2(decltype(new int{0}));
> +
> +void
> +g ()
> +{
> + int i;
> + void f3(decltype(new auto{0}));
> + void f4(decltype(new int{0}));
> + f1 (&i);
> + f2 (&i);
> + f3 (&i);
> + f4 (&i);
> +}
> diff --git a/gcc/testsuite/g++.dg/cpp2a/concepts-pr84979-2.C b/gcc/testsuite/g++.dg/cpp2a/concepts-pr84979-2.C
> index 025bbf3bb93..75f8e40ca58 100644
> --- a/gcc/testsuite/g++.dg/cpp2a/concepts-pr84979-2.C
> +++ b/gcc/testsuite/g++.dg/cpp2a/concepts-pr84979-2.C
> @@ -4,9 +4,9 @@
> template <typename T>
> void foo1(T& t) {
> typename T::template C<void> tcv = t;
> - typename T::template C<auto> u = tcv; // { dg-error "not permitted" "" { target c++20 } }
> - T::template C<auto>::f (tcv, u); // { dg-error "incomplete|not permitted" }
> - (typename T::template D<auto> (t)); // { dg-error "invalid|not permitted|unable" }
> + typename T::template C<auto> u = tcv; // { dg-error "" "" { target c++20 } }
> + T::template C<auto>::f (tcv, u); // { dg-error "" }
> + (typename T::template D<auto> (t)); // { dg-error "" }
> // { dg-warning "only available" "" { target c++17_down } .-1 }
> }
>
> @@ -23,9 +23,9 @@ struct T1 {
> template <typename T>
> void foo2(T& t) {
> typename T::template C<void> tcv = t;
> - typename T::template C<auto> u = tcv; // { dg-error "not permitted" "" { target c++20 } }
> - T::template C<auto>::f (tcv, u); // { dg-error "incomplete|not permitted" }
> - T::template D<auto> (t); // { dg-error "invalid|not permitted" }
> + typename T::template C<auto> u = tcv; // { dg-error "" "" { target c++20 } }
> + T::template C<auto>::f (tcv, u); // { dg-error "" }
> + T::template D<auto> (t); // { dg-error "" }
> }
>
> struct T2 {
> diff --git a/gcc/testsuite/g++.dg/cpp2a/concepts-pr84979-3.C b/gcc/testsuite/g++.dg/cpp2a/concepts-pr84979-3.C
> index 80a388462eb..1c1a41c0fa2 100644
> --- a/gcc/testsuite/g++.dg/cpp2a/concepts-pr84979-3.C
> +++ b/gcc/testsuite/g++.dg/cpp2a/concepts-pr84979-3.C
> @@ -8,9 +8,9 @@
> template <typename T>
> void foo1(T& t) {
> typename T::template C<void> tcv = t;
> - typename T::template C<auto> u = tcv; // { dg-error "not permitted" "" { target c++20 } }
> - T::template C<auto>::f (tcv, u); // { dg-error "incomplete|not permitted" }
> - (typename T::template D<auto> (t)); // { dg-error "invalid|not permitted|no class" }
> + typename T::template C<auto> u = tcv; // { dg-error "" "" { target c++20 } }
> + T::template C<auto>::f (tcv, u); // { dg-error "" }
> + (typename T::template D<auto> (t)); // { dg-error "" }
> // { dg-warning "only available" "" { target c++17_down } .-1 }
> }
>
> @@ -27,9 +27,9 @@ struct T1 {
> template <typename T>
> void foo2(T& t) {
> typename T::template C<void> tcv = t;
> - typename T::template C<auto> u = tcv; // { dg-error "not permitted" "" { target c++20 } }
> - T::template C<auto>::f (tcv, u); // { dg-error "incomplete|not permitted" }
> - T::template D<auto> (t); // { dg-error "yields a type|not permitted" }
> + typename T::template C<auto> u = tcv; // { dg-error "" "" { target c++20 } }
> + T::template C<auto>::f (tcv, u); // { dg-error "" }
> + T::template D<auto> (t); // { dg-error "" }
> }
>
> struct T2 {
> diff --git a/gcc/testsuite/g++.dg/cpp2a/concepts-pr84979.C b/gcc/testsuite/g++.dg/cpp2a/concepts-pr84979.C
> index 81555eb4554..a83601527bb 100644
> --- a/gcc/testsuite/g++.dg/cpp2a/concepts-pr84979.C
> +++ b/gcc/testsuite/g++.dg/cpp2a/concepts-pr84979.C
> @@ -5,5 +5,5 @@ template<typename> void foo() {}
>
> void bar()
> {
> - foo<auto>(); // { dg-error "not permitted|invalid|no matching function" }
> + foo<auto>(); // { dg-error "" }
> }
>
> base-commit: b77968a70537429b4f548f90c369d26e6b6943cc
>
More information about the Gcc-patches
mailing list