diff mbox series

c++: wrong error with variadic concept [PR105268]

Message ID 20220414202419.420912-1-polacek@redhat.com
State New
Headers show
Series c++: wrong error with variadic concept [PR105268] | expand

Commit Message

Marek Polacek April 14, 2022, 8:24 p.m. UTC
Here we issue a wrong error for the

  template<typename T = S<C_many<int>>> void g();

line in the testcase.  I surmise that's because we mistakenly parse
C_many<int> as a placeholder-type-specifier, and things go wrong from
there.  We are in a default argument so we should reject parsing C_many<int>
as a placeholder-type-specifier, which would mean creating a new parameter.
We want C_many<int> to be a concept-id instead.

It's interesting to see why the same problem didn't occur for C_one<int>.
In that case, cp_parser_placeholder_type_specifier -> finish_type_constraints
-> build_type_constraint -> build_concept_check -> build_standard_check ->
coerce_template_parms fails the parse here:

 8916   nargs = inner_args ? NUM_TMPL_ARGS (inner_args) : 0;
 8917   if ((nargs - variadic_args_p > nparms && !variadic_p)
 8918       || (nargs < nparms - variadic_p
 8919           && require_all_args
 8920           && !variadic_args_p
 8921           && (!use_default_args
 8922               || (TREE_VEC_ELT (parms, nargs) != error_mark_node
 8923                   && !TREE_PURPOSE (TREE_VEC_ELT (parms, nargs))))))
 8924     {
 8925     bad_nargs:
 ...
 8943       return error_mark_node;

because nargs is 2 (the targs are <WILDCARD_DECL, int>) while nparms is
1 (for the one 'typename' in the tparam list of C_one).  But for
C_many<int> variadic_p is true so we don't return error_mark_node but
<type_argument_pack>.

This patch does not issue any error for the !tentative case because
I didn't figure out how to trigger that.

Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk?

	PR c++/105268

gcc/cp/ChangeLog:

	* parser.cc (cp_parser_placeholder_type_specifier): Return
	error_mark_node when trying to build up a constrained parameter in
	a default argument.

gcc/testsuite/ChangeLog:

	* g++.dg/concepts/variadic6.C: New test.
---
 gcc/cp/parser.cc                          |  7 ++++++-
 gcc/testsuite/g++.dg/concepts/variadic6.C | 20 ++++++++++++++++++++
 2 files changed, 26 insertions(+), 1 deletion(-)
 create mode 100644 gcc/testsuite/g++.dg/concepts/variadic6.C


base-commit: 82536fbb8a7d150b829650378e0ba07dad5c8fb8

Comments

Jason Merrill April 15, 2022, 2:32 a.m. UTC | #1
On 4/14/22 16:24, Marek Polacek wrote:
> Here we issue a wrong error for the
> 
>    template<typename T = S<C_many<int>>> void g();
> 
> line in the testcase.  I surmise that's because we mistakenly parse
> C_many<int> as a placeholder-type-specifier, and things go wrong from
> there.  We are in a default argument so we should reject parsing C_many<int>
> as a placeholder-type-specifier, which would mean creating a new parameter.
> We want C_many<int> to be a concept-id instead.
> 
> It's interesting to see why the same problem didn't occur for C_one<int>.
> In that case, cp_parser_placeholder_type_specifier -> finish_type_constraints
> -> build_type_constraint -> build_concept_check -> build_standard_check ->
> coerce_template_parms fails the parse here:
> 
>   8916   nargs = inner_args ? NUM_TMPL_ARGS (inner_args) : 0;
>   8917   if ((nargs - variadic_args_p > nparms && !variadic_p)
>   8918       || (nargs < nparms - variadic_p
>   8919           && require_all_args
>   8920           && !variadic_args_p
>   8921           && (!use_default_args
>   8922               || (TREE_VEC_ELT (parms, nargs) != error_mark_node
>   8923                   && !TREE_PURPOSE (TREE_VEC_ELT (parms, nargs))))))
>   8924     {
>   8925     bad_nargs:
>   ...
>   8943       return error_mark_node;
> 
> because nargs is 2 (the targs are <WILDCARD_DECL, int>) while nparms is
> 1 (for the one 'typename' in the tparam list of C_one).  But for
> C_many<int> variadic_p is true so we don't return error_mark_node but
> <type_argument_pack>.
> 
> This patch does not issue any error for the !tentative case because
> I didn't figure out how to trigger that.

Then I'd add an assert that tentative is true.  OK with that change.

> Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk?
> 
> 	PR c++/105268
> 
> gcc/cp/ChangeLog:
> 
> 	* parser.cc (cp_parser_placeholder_type_specifier): Return
> 	error_mark_node when trying to build up a constrained parameter in
> 	a default argument.
> 
> gcc/testsuite/ChangeLog:
> 
> 	* g++.dg/concepts/variadic6.C: New test.
> ---
>   gcc/cp/parser.cc                          |  7 ++++++-
>   gcc/testsuite/g++.dg/concepts/variadic6.C | 20 ++++++++++++++++++++
>   2 files changed, 26 insertions(+), 1 deletion(-)
>   create mode 100644 gcc/testsuite/g++.dg/concepts/variadic6.C
> 
> diff --git a/gcc/cp/parser.cc b/gcc/cp/parser.cc
> index bfd16e1ef62..dfb613168b6 100644
> --- a/gcc/cp/parser.cc
> +++ b/gcc/cp/parser.cc
> @@ -20041,7 +20041,12 @@ cp_parser_placeholder_type_specifier (cp_parser *parser, location_t loc,
>     /* In a template parameter list, a type-parameter can be introduced
>        by type-constraints alone.  */
>     if (processing_template_parmlist && !placeholder)
> -    return build_constrained_parameter (con, proto, args);
> +    {
> +      /* In a default argument we may not be creating new parameters.  */
> +      if (parser->local_variables_forbidden_p & LOCAL_VARS_FORBIDDEN)
> +	return error_mark_node;
> +      return build_constrained_parameter (con, proto, args);
> +    }
>   
>     /* Diagnose issues placeholder issues.  */
>     if (!flag_concepts_ts
> diff --git a/gcc/testsuite/g++.dg/concepts/variadic6.C b/gcc/testsuite/g++.dg/concepts/variadic6.C
> new file mode 100644
> index 00000000000..0b386b0cd6d
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/concepts/variadic6.C
> @@ -0,0 +1,20 @@
> +// PR c++/105268
> +// { dg-do compile { target concepts } }
> +
> +template<typename> concept C_one = true;
> +template<typename...> concept C_many = true;
> +
> +template<bool B> struct S { };
> +
> +template<typename T = S<C_one<int>>> void f();
> +template<typename T = S<C_many<int>>> void g();
> +
> +void
> +fn (auto a = S<C_one<int>>{})
> +{
> +}
> +
> +void
> +fn2 (auto a = S<C_many<int>>{})
> +{
> +}
> 
> base-commit: 82536fbb8a7d150b829650378e0ba07dad5c8fb8
diff mbox series

Patch

diff --git a/gcc/cp/parser.cc b/gcc/cp/parser.cc
index bfd16e1ef62..dfb613168b6 100644
--- a/gcc/cp/parser.cc
+++ b/gcc/cp/parser.cc
@@ -20041,7 +20041,12 @@  cp_parser_placeholder_type_specifier (cp_parser *parser, location_t loc,
   /* In a template parameter list, a type-parameter can be introduced
      by type-constraints alone.  */
   if (processing_template_parmlist && !placeholder)
-    return build_constrained_parameter (con, proto, args);
+    {
+      /* In a default argument we may not be creating new parameters.  */
+      if (parser->local_variables_forbidden_p & LOCAL_VARS_FORBIDDEN)
+	return error_mark_node;
+      return build_constrained_parameter (con, proto, args);
+    }
 
   /* Diagnose issues placeholder issues.  */
   if (!flag_concepts_ts
diff --git a/gcc/testsuite/g++.dg/concepts/variadic6.C b/gcc/testsuite/g++.dg/concepts/variadic6.C
new file mode 100644
index 00000000000..0b386b0cd6d
--- /dev/null
+++ b/gcc/testsuite/g++.dg/concepts/variadic6.C
@@ -0,0 +1,20 @@ 
+// PR c++/105268
+// { dg-do compile { target concepts } }
+
+template<typename> concept C_one = true;
+template<typename...> concept C_many = true;
+
+template<bool B> struct S { };
+
+template<typename T = S<C_one<int>>> void f();
+template<typename T = S<C_many<int>>> void g();
+
+void
+fn (auto a = S<C_one<int>>{})
+{
+}
+
+void
+fn2 (auto a = S<C_many<int>>{})
+{
+}