c++: normalization of non-templated return-type-req [PR100946]

Message ID 20210609193414.2908539-1-ppalka@redhat.com
State New
Headers show
Series
  • c++: normalization of non-templated return-type-req [PR100946]
Related show

Commit Message

Marek Polacek via Gcc-patches June 9, 2021, 7:34 p.m.
Here the satisfaction cache is conflating the satisfaction value of the
two return-type-requirements because the corresponding constrained
'auto's have level 2, but they capture an empty current_template_parms.
This ultimately causes the satisfaction cache to think the type
constraint doesn't depend on the deduced type of the expression.

When normalizing the constraints on an 'auto', the assumption made by
normalize_placeholder_type_constraints is that the level of the 'auto'
is one greater than the captured current_template_parms, an assumption
which is not holding here.  To fix this, this patch adds a dummy level
to current_template_parms to match processing_template_decl when parsing
a non-templated return-type-requirement.  This patch also makes us
verify this assumption upon creation of a constrained 'auto'.

Bootstrapped and regtested on x86_64-pc-linux-gnu, does this look OK for
trunk/11?

	PR c++/100946

gcc/cp/ChangeLog:

	* parser.c (cp_parser_compound_requirement): When parsing a
	non-templated return-type-requirement, add a dummy level
	to current_template_parms.
	* pt.c (make_constrained_placeholder_type): Verify the depth
	of current_template_parms is consistent with the level of
	the 'auto'.

gcc/testsuite/ChangeLog:

	* g++.dg/cpp2a/concepts-return-req3.C: New test.
---
 gcc/cp/parser.c                                   | 12 ++++++++++++
 gcc/cp/pt.c                                       |  8 ++++++++
 gcc/testsuite/g++.dg/cpp2a/concepts-return-req3.C |  6 ++++++
 3 files changed, 26 insertions(+)
 create mode 100644 gcc/testsuite/g++.dg/cpp2a/concepts-return-req3.C

-- 
2.32.0.rc2

Comments

Marek Polacek via Gcc-patches June 10, 2021, 7:07 p.m. | #1
On 6/9/21 3:34 PM, Patrick Palka wrote:
> Here the satisfaction cache is conflating the satisfaction value of the

> two return-type-requirements because the corresponding constrained

> 'auto's have level 2, but they capture an empty current_template_parms.

> This ultimately causes the satisfaction cache to think the type

> constraint doesn't depend on the deduced type of the expression.

> 

> When normalizing the constraints on an 'auto', the assumption made by

> normalize_placeholder_type_constraints is that the level of the 'auto'

> is one greater than the captured current_template_parms, an assumption

> which is not holding here.  To fix this, this patch adds a dummy level

> to current_template_parms to match processing_template_decl when parsing

> a non-templated return-type-requirement.  This patch also makes us

> verify this assumption upon creation of a constrained 'auto'.

> 

> Bootstrapped and regtested on x86_64-pc-linux-gnu, does this look OK for

> trunk/11?

> 

> 	PR c++/100946

> 

> gcc/cp/ChangeLog:

> 

> 	* parser.c (cp_parser_compound_requirement): When parsing a

> 	non-templated return-type-requirement, add a dummy level

> 	to current_template_parms.

> 	* pt.c (make_constrained_placeholder_type): Verify the depth

> 	of current_template_parms is consistent with the level of

> 	the 'auto'.

> 

> gcc/testsuite/ChangeLog:

> 

> 	* g++.dg/cpp2a/concepts-return-req3.C: New test.

> ---

>   gcc/cp/parser.c                                   | 12 ++++++++++++

>   gcc/cp/pt.c                                       |  8 ++++++++

>   gcc/testsuite/g++.dg/cpp2a/concepts-return-req3.C |  6 ++++++

>   3 files changed, 26 insertions(+)

>   create mode 100644 gcc/testsuite/g++.dg/cpp2a/concepts-return-req3.C

> 

> diff --git a/gcc/cp/parser.c b/gcc/cp/parser.c

> index d59a829d0b9..8278a5608ae 100644

> --- a/gcc/cp/parser.c

> +++ b/gcc/cp/parser.c

> @@ -29181,6 +29181,18 @@ cp_parser_compound_requirement (cp_parser *parser)

>         cp_lexer_consume_token (parser->lexer);

>         cp_token *tok = cp_lexer_peek_token (parser->lexer);

>   

> +      auto ctp = make_temp_override (current_template_parms);

> +      if (!current_template_parms)

> +	{

> +	  /* We're parsing a return-type-requirement within a non-templated

> +	     requires-expression.  Update current_template_parms to agree with

> +	     processing_template_decl so that the normalization context that's

> +	     captured by the corresponding constrained 'auto' is sensible.  */

> +	  gcc_checking_assert (processing_template_decl == 1);

> +	  current_template_parms

> +	    = build_tree_list (size_int (1), make_tree_vec (0));

> +	}


How about handling this in normalization, rather than in the parser?

If you set current_template_parms here, when is it cleared?

>         bool saved_result_type_constraint_p = parser->in_result_type_constraint_p;

>         parser->in_result_type_constraint_p = true;

>         /* C++20 allows either a type-id or a type-constraint. Parsing

> diff --git a/gcc/cp/pt.c b/gcc/cp/pt.c

> index b0155a9c370..05679b12973 100644

> --- a/gcc/cp/pt.c

> +++ b/gcc/cp/pt.c

> @@ -28131,6 +28131,14 @@ make_constrained_placeholder_type (tree type, tree con, tree args)

>     expr = build_concept_check (expr, type, args, tf_warning_or_error);

>     --processing_template_decl;

>   

> +  /* Verify the normalization context is consistent with the level of

> +     this 'auto'.  */

> +  if (TEMPLATE_TYPE_LEVEL (type) == 1)

> +    gcc_checking_assert (!current_template_parms);

> +  else

> +    gcc_checking_assert (1 + TMPL_PARMS_DEPTH (current_template_parms)

> +			 == TEMPLATE_TYPE_LEVEL (type));

> +

>     PLACEHOLDER_TYPE_CONSTRAINTS_INFO (type)

>       = build_tree_list (current_template_parms, expr);

>   

> diff --git a/gcc/testsuite/g++.dg/cpp2a/concepts-return-req3.C b/gcc/testsuite/g++.dg/cpp2a/concepts-return-req3.C

> new file mode 100644

> index 00000000000..a546c6457be

> --- /dev/null

> +++ b/gcc/testsuite/g++.dg/cpp2a/concepts-return-req3.C

> @@ -0,0 +1,6 @@

> +// PR c++/100946

> +// { dg-do compile { target c++20 } }

> +

> +template<class T> concept C = __is_same(T, int);

> +static_assert(requires { { 0 } -> C; });

> +static_assert(requires { { true } -> C; }); // { dg-error "failed" }

>
Marek Polacek via Gcc-patches June 10, 2021, 8:17 p.m. | #2
On Thu, 10 Jun 2021, Jason Merrill wrote:

> On 6/9/21 3:34 PM, Patrick Palka wrote:

> > Here the satisfaction cache is conflating the satisfaction value of the

> > two return-type-requirements because the corresponding constrained

> > 'auto's have level 2, but they capture an empty current_template_parms.

> > This ultimately causes the satisfaction cache to think the type

> > constraint doesn't depend on the deduced type of the expression.

> > 

> > When normalizing the constraints on an 'auto', the assumption made by

> > normalize_placeholder_type_constraints is that the level of the 'auto'

> > is one greater than the captured current_template_parms, an assumption

> > which is not holding here.  To fix this, this patch adds a dummy level

> > to current_template_parms to match processing_template_decl when parsing

> > a non-templated return-type-requirement.  This patch also makes us

> > verify this assumption upon creation of a constrained 'auto'.

> > 

> > Bootstrapped and regtested on x86_64-pc-linux-gnu, does this look OK for

> > trunk/11?

> > 

> > 	PR c++/100946

> > 

> > gcc/cp/ChangeLog:

> > 

> > 	* parser.c (cp_parser_compound_requirement): When parsing a

> > 	non-templated return-type-requirement, add a dummy level

> > 	to current_template_parms.

> > 	* pt.c (make_constrained_placeholder_type): Verify the depth

> > 	of current_template_parms is consistent with the level of

> > 	the 'auto'.

> > 

> > gcc/testsuite/ChangeLog:

> > 

> > 	* g++.dg/cpp2a/concepts-return-req3.C: New test.

> > ---

> >   gcc/cp/parser.c                                   | 12 ++++++++++++

> >   gcc/cp/pt.c                                       |  8 ++++++++

> >   gcc/testsuite/g++.dg/cpp2a/concepts-return-req3.C |  6 ++++++

> >   3 files changed, 26 insertions(+)

> >   create mode 100644 gcc/testsuite/g++.dg/cpp2a/concepts-return-req3.C

> > 

> > diff --git a/gcc/cp/parser.c b/gcc/cp/parser.c

> > index d59a829d0b9..8278a5608ae 100644

> > --- a/gcc/cp/parser.c

> > +++ b/gcc/cp/parser.c

> > @@ -29181,6 +29181,18 @@ cp_parser_compound_requirement (cp_parser *parser)

> >         cp_lexer_consume_token (parser->lexer);

> >         cp_token *tok = cp_lexer_peek_token (parser->lexer);

> >   +      auto ctp = make_temp_override (current_template_parms);

> > +      if (!current_template_parms)

> > +	{

> > +	  /* We're parsing a return-type-requirement within a non-templated

> > +	     requires-expression.  Update current_template_parms to agree with

> > +	     processing_template_decl so that the normalization context that's

> > +	     captured by the corresponding constrained 'auto' is sensible.  */

> > +	  gcc_checking_assert (processing_template_decl == 1);

> > +	  current_template_parms

> > +	    = build_tree_list (size_int (1), make_tree_vec (0));

> > +	}

> 

> How about handling this in normalization, rather than in the parser?


That works nicely too, how does the below look?  Tested
x86_64-pc-linux-gnu.

> 

> If you set current_template_parms here, when is it cleared?


The 'ctp' temp_override would take care of that.

-- >8 --

Subject: [PATCH] c++: normalization of non-templated return-type-req
 [PR100946]

Here the satisfaction cache is conflating the satisfaction value of the
two return-type-requirements because the corresponding constrained
'auto's have level 2, but they capture an empty current_template_parms.
This ultimately causes the satisfaction cache to think the type
constraint doesn't depend on the deduced type of the expression.

When normalizing the constraints on an 'auto', the assumption made by
normalize_placeholder_type_constraints is that the level of the 'auto'
is one greater than the depth of the captured current_template_parms, an
assumption which is not holding here.  This patch relaxes makes
normalize_placeholder_type_constraints adjust the normalization context
appropriately in this situation.

	PR c++/100946

gcc/cp/ChangeLog:

	* constraint.cc (normalize_placeholder_type_constraints): When
	normalizing a non-templated return-type-requirement, add a dummy
	level to initial_parms.

gcc/testsuite/ChangeLog:

	* g++.dg/cpp2a/concepts-return-req3.C: New test.
---
 gcc/cp/constraint.cc                              | 9 +++++++++
 gcc/testsuite/g++.dg/cpp2a/concepts-return-req3.C | 6 ++++++
 2 files changed, 15 insertions(+)
 create mode 100644 gcc/testsuite/g++.dg/cpp2a/concepts-return-req3.C

diff --git a/gcc/cp/constraint.cc b/gcc/cp/constraint.cc
index 03ce8eb9ff2..74b16d27101 100644
--- a/gcc/cp/constraint.cc
+++ b/gcc/cp/constraint.cc
@@ -3065,6 +3065,15 @@ normalize_placeholder_type_constraints (tree t, bool diag)
      scope for this placeholder type; use them as the initial template
      parameters for normalization.  */
   tree initial_parms = TREE_PURPOSE (ci);
+
+  if (!initial_parms && TEMPLATE_TYPE_LEVEL (t) == 2)
+    /* This is a return-type-requirement of a non-templated requires-expression,
+       which are parsed under processing_template_decl == 1 and empty
+       current_template_parms; hence the 'auto' has level 2 and initial_parms
+       is empty.  Fix up initial_parms to be consistent with the value of
+       processing_template_decl whence the 'auto' was created.  */
+    initial_parms = build_tree_list (size_int (1), make_tree_vec (0));
+
   /* The 'auto' itself is used as the first argument in its own constraints,
      and its level is one greater than its template depth.  So in order to
      capture all used template parameters, we need to add an extra level of
diff --git a/gcc/testsuite/g++.dg/cpp2a/concepts-return-req3.C b/gcc/testsuite/g++.dg/cpp2a/concepts-return-req3.C
new file mode 100644
index 00000000000..a546c6457be
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp2a/concepts-return-req3.C
@@ -0,0 +1,6 @@
+// PR c++/100946
+// { dg-do compile { target c++20 } }
+
+template<class T> concept C = __is_same(T, int);
+static_assert(requires { { 0 } -> C; });
+static_assert(requires { { true } -> C; }); // { dg-error "failed" }
-- 
2.32.0.rc2
Marek Polacek via Gcc-patches June 10, 2021, 8:24 p.m. | #3
On 6/10/21 4:17 PM, Patrick Palka wrote:
> On Thu, 10 Jun 2021, Jason Merrill wrote:

> 

>> On 6/9/21 3:34 PM, Patrick Palka wrote:

>>> Here the satisfaction cache is conflating the satisfaction value of the

>>> two return-type-requirements because the corresponding constrained

>>> 'auto's have level 2, but they capture an empty current_template_parms.

>>> This ultimately causes the satisfaction cache to think the type

>>> constraint doesn't depend on the deduced type of the expression.

>>>

>>> When normalizing the constraints on an 'auto', the assumption made by

>>> normalize_placeholder_type_constraints is that the level of the 'auto'

>>> is one greater than the captured current_template_parms, an assumption

>>> which is not holding here.  To fix this, this patch adds a dummy level

>>> to current_template_parms to match processing_template_decl when parsing

>>> a non-templated return-type-requirement.  This patch also makes us

>>> verify this assumption upon creation of a constrained 'auto'.

>>>

>>> Bootstrapped and regtested on x86_64-pc-linux-gnu, does this look OK for

>>> trunk/11?

>>>

>>> 	PR c++/100946

>>>

>>> gcc/cp/ChangeLog:

>>>

>>> 	* parser.c (cp_parser_compound_requirement): When parsing a

>>> 	non-templated return-type-requirement, add a dummy level

>>> 	to current_template_parms.

>>> 	* pt.c (make_constrained_placeholder_type): Verify the depth

>>> 	of current_template_parms is consistent with the level of

>>> 	the 'auto'.

>>>

>>> gcc/testsuite/ChangeLog:

>>>

>>> 	* g++.dg/cpp2a/concepts-return-req3.C: New test.

>>> ---

>>>    gcc/cp/parser.c                                   | 12 ++++++++++++

>>>    gcc/cp/pt.c                                       |  8 ++++++++

>>>    gcc/testsuite/g++.dg/cpp2a/concepts-return-req3.C |  6 ++++++

>>>    3 files changed, 26 insertions(+)

>>>    create mode 100644 gcc/testsuite/g++.dg/cpp2a/concepts-return-req3.C

>>>

>>> diff --git a/gcc/cp/parser.c b/gcc/cp/parser.c

>>> index d59a829d0b9..8278a5608ae 100644

>>> --- a/gcc/cp/parser.c

>>> +++ b/gcc/cp/parser.c

>>> @@ -29181,6 +29181,18 @@ cp_parser_compound_requirement (cp_parser *parser)

>>>          cp_lexer_consume_token (parser->lexer);

>>>          cp_token *tok = cp_lexer_peek_token (parser->lexer);

>>>    +      auto ctp = make_temp_override (current_template_parms);

>>> +      if (!current_template_parms)

>>> +	{

>>> +	  /* We're parsing a return-type-requirement within a non-templated

>>> +	     requires-expression.  Update current_template_parms to agree with

>>> +	     processing_template_decl so that the normalization context that's

>>> +	     captured by the corresponding constrained 'auto' is sensible.  */

>>> +	  gcc_checking_assert (processing_template_decl == 1);

>>> +	  current_template_parms

>>> +	    = build_tree_list (size_int (1), make_tree_vec (0));

>>> +	}

>>

>> How about handling this in normalization, rather than in the parser?

> 

> That works nicely too, how does the below look?  Tested

> x86_64-pc-linux-gnu.


OK, thanks.

>> If you set current_template_parms here, when is it cleared?

> 

> The 'ctp' temp_override would take care of that.


Ah, of course, somehow I overlooked that line.

> -- >8 --

> 

> Subject: [PATCH] c++: normalization of non-templated return-type-req

>   [PR100946]

> 

> Here the satisfaction cache is conflating the satisfaction value of the

> two return-type-requirements because the corresponding constrained

> 'auto's have level 2, but they capture an empty current_template_parms.

> This ultimately causes the satisfaction cache to think the type

> constraint doesn't depend on the deduced type of the expression.

> 

> When normalizing the constraints on an 'auto', the assumption made by

> normalize_placeholder_type_constraints is that the level of the 'auto'

> is one greater than the depth of the captured current_template_parms, an

> assumption which is not holding here.  This patch relaxes makes

> normalize_placeholder_type_constraints adjust the normalization context

> appropriately in this situation.

> 

> 	PR c++/100946

> 

> gcc/cp/ChangeLog:

> 

> 	* constraint.cc (normalize_placeholder_type_constraints): When

> 	normalizing a non-templated return-type-requirement, add a dummy

> 	level to initial_parms.

> 

> gcc/testsuite/ChangeLog:

> 

> 	* g++.dg/cpp2a/concepts-return-req3.C: New test.

> ---

>   gcc/cp/constraint.cc                              | 9 +++++++++

>   gcc/testsuite/g++.dg/cpp2a/concepts-return-req3.C | 6 ++++++

>   2 files changed, 15 insertions(+)

>   create mode 100644 gcc/testsuite/g++.dg/cpp2a/concepts-return-req3.C

> 

> diff --git a/gcc/cp/constraint.cc b/gcc/cp/constraint.cc

> index 03ce8eb9ff2..74b16d27101 100644

> --- a/gcc/cp/constraint.cc

> +++ b/gcc/cp/constraint.cc

> @@ -3065,6 +3065,15 @@ normalize_placeholder_type_constraints (tree t, bool diag)

>        scope for this placeholder type; use them as the initial template

>        parameters for normalization.  */

>     tree initial_parms = TREE_PURPOSE (ci);

> +

> +  if (!initial_parms && TEMPLATE_TYPE_LEVEL (t) == 2)

> +    /* This is a return-type-requirement of a non-templated requires-expression,

> +       which are parsed under processing_template_decl == 1 and empty

> +       current_template_parms; hence the 'auto' has level 2 and initial_parms

> +       is empty.  Fix up initial_parms to be consistent with the value of

> +       processing_template_decl whence the 'auto' was created.  */

> +    initial_parms = build_tree_list (size_int (1), make_tree_vec (0));

> +

>     /* The 'auto' itself is used as the first argument in its own constraints,

>        and its level is one greater than its template depth.  So in order to

>        capture all used template parameters, we need to add an extra level of

> diff --git a/gcc/testsuite/g++.dg/cpp2a/concepts-return-req3.C b/gcc/testsuite/g++.dg/cpp2a/concepts-return-req3.C

> new file mode 100644

> index 00000000000..a546c6457be

> --- /dev/null

> +++ b/gcc/testsuite/g++.dg/cpp2a/concepts-return-req3.C

> @@ -0,0 +1,6 @@

> +// PR c++/100946

> +// { dg-do compile { target c++20 } }

> +

> +template<class T> concept C = __is_same(T, int);

> +static_assert(requires { { 0 } -> C; });

> +static_assert(requires { { true } -> C; }); // { dg-error "failed" }

>

Patch

diff --git a/gcc/cp/parser.c b/gcc/cp/parser.c
index d59a829d0b9..8278a5608ae 100644
--- a/gcc/cp/parser.c
+++ b/gcc/cp/parser.c
@@ -29181,6 +29181,18 @@  cp_parser_compound_requirement (cp_parser *parser)
       cp_lexer_consume_token (parser->lexer);
       cp_token *tok = cp_lexer_peek_token (parser->lexer);
 
+      auto ctp = make_temp_override (current_template_parms);
+      if (!current_template_parms)
+	{
+	  /* We're parsing a return-type-requirement within a non-templated
+	     requires-expression.  Update current_template_parms to agree with
+	     processing_template_decl so that the normalization context that's
+	     captured by the corresponding constrained 'auto' is sensible.  */
+	  gcc_checking_assert (processing_template_decl == 1);
+	  current_template_parms
+	    = build_tree_list (size_int (1), make_tree_vec (0));
+	}
+
       bool saved_result_type_constraint_p = parser->in_result_type_constraint_p;
       parser->in_result_type_constraint_p = true;
       /* C++20 allows either a type-id or a type-constraint. Parsing
diff --git a/gcc/cp/pt.c b/gcc/cp/pt.c
index b0155a9c370..05679b12973 100644
--- a/gcc/cp/pt.c
+++ b/gcc/cp/pt.c
@@ -28131,6 +28131,14 @@  make_constrained_placeholder_type (tree type, tree con, tree args)
   expr = build_concept_check (expr, type, args, tf_warning_or_error);
   --processing_template_decl;
 
+  /* Verify the normalization context is consistent with the level of
+     this 'auto'.  */
+  if (TEMPLATE_TYPE_LEVEL (type) == 1)
+    gcc_checking_assert (!current_template_parms);
+  else
+    gcc_checking_assert (1 + TMPL_PARMS_DEPTH (current_template_parms)
+			 == TEMPLATE_TYPE_LEVEL (type));
+
   PLACEHOLDER_TYPE_CONSTRAINTS_INFO (type)
     = build_tree_list (current_template_parms, expr);
 
diff --git a/gcc/testsuite/g++.dg/cpp2a/concepts-return-req3.C b/gcc/testsuite/g++.dg/cpp2a/concepts-return-req3.C
new file mode 100644
index 00000000000..a546c6457be
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp2a/concepts-return-req3.C
@@ -0,0 +1,6 @@ 
+// PR c++/100946
+// { dg-do compile { target c++20 } }
+
+template<class T> concept C = __is_same(T, int);
+static_assert(requires { { 0 } -> C; });
+static_assert(requires { { true } -> C; }); // { dg-error "failed" }