Re: [PATCH] c++: requires-expr with dependent extra args [PR101181]

2021-07-09 Thread Patrick Palka via Gcc-patches
On Thu, 8 Jul 2021, Jason Merrill wrote:

> On 7/8/21 11:28 AM, Patrick Palka wrote:
> > Here we're crashing ultimately because the mechanism for delaying
> > substitution into a requires-expression (or constexpr if) doesn't
> > expect to see dependent args.  But we end up capturing dependent
> > args here when substituting into the default template argument during
> > coerce_template_parms for the dependent specialization p.
> > 
> > This patch enables the commented out code in add_extra_args for
> > handling this situation.  It turns out we also need to make a copy of
> > the captured arguments so that coerce_template_parms doesn't later
> > add to the argument, which would form an unexpected cycle.  And we
> > need to make tsubst_template_args more forgiving about missing template
> > arguments, since the arguments we capture from coerce_template_parms are
> > incomplete.
> > 
> > Bootstrapped and regtested on x86_64-pc-linux-gnu, does this look OK for
> > trunk/11?
> > 
> > PR c++/101181
> > 
> > gcc/cp/ChangeLog:
> > 
> > * constraint.cc (tsubst_requires_expr): Pass complain/in_decl to
> > add_extra_args.
> > * cp-tree.h (add_extra_args): Add complain/in_decl parameters.
> > * pt.c (build_extra_args): Make a copy of args.
> > (add_extra_args): Add complain/in_decl parameters.  Handle the
> > case where the extra arguments are dependent.
> > (tsubst_pack_expansion): Pass complain/in_decl to
> > add_extra_args.
> > (tsubst_template_args): Handle missing template arguments.
> > (tsubst_expr) : Pass complain/in_decl to
> > add_extra_args.
> > 
> > gcc/testsuite/ChangeLog:
> > 
> > * g++.dg/cpp2a/concepts-requires26.C: New test.
> > * g++.dg/cpp2a/lambda-uneval16.C: New test.
> > ---
> >   gcc/cp/constraint.cc  |  3 +-
> >   gcc/cp/cp-tree.h  |  2 +-
> >   gcc/cp/pt.c   | 31 +--
> >   .../g++.dg/cpp2a/concepts-requires26.C| 18 +++
> >   gcc/testsuite/g++.dg/cpp2a/lambda-uneval16.C  | 22 +
> >   5 files changed, 58 insertions(+), 18 deletions(-)
> >   create mode 100644 gcc/testsuite/g++.dg/cpp2a/concepts-requires26.C
> >   create mode 100644 gcc/testsuite/g++.dg/cpp2a/lambda-uneval16.C
> > 
> > diff --git a/gcc/cp/constraint.cc b/gcc/cp/constraint.cc
> > index 99d3ccc6998..4ee5215df50 100644
> > --- a/gcc/cp/constraint.cc
> > +++ b/gcc/cp/constraint.cc
> > @@ -2266,7 +2266,8 @@ tsubst_requires_expr (tree t, tree args, sat_info
> > info)
> > /* A requires-expression is an unevaluated context.  */
> > cp_unevaluated u;
> >   -  args = add_extra_args (REQUIRES_EXPR_EXTRA_ARGS (t), args);
> > +  args = add_extra_args (REQUIRES_EXPR_EXTRA_ARGS (t), args,
> > +info.complain, info.in_decl);
> > if (processing_template_decl)
> >   {
> > /* We're partially instantiating a generic lambda.  Substituting
> > into
> > diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h
> > index 58da7460001..0a5f13489cc 100644
> > --- a/gcc/cp/cp-tree.h
> > +++ b/gcc/cp/cp-tree.h
> > @@ -7289,7 +7289,7 @@ extern void add_mergeable_specialization(bool
> > is_decl, bool is_alias,
> >  tree outer, unsigned);
> >   extern tree add_to_template_args  (tree, tree);
> >   extern tree add_outermost_template_args   (tree, tree);
> > -extern tree add_extra_args (tree, tree);
> > +extern tree add_extra_args (tree, tree, tsubst_flags_t,
> > tree);
> >   extern tree build_extra_args  (tree, tree,
> > tsubst_flags_t);
> > /* in rtti.c */
> > diff --git a/gcc/cp/pt.c b/gcc/cp/pt.c
> > index 06116d16887..e4bdac087ad 100644
> > --- a/gcc/cp/pt.c
> > +++ b/gcc/cp/pt.c
> > @@ -12928,7 +12928,9 @@ extract_local_specs (tree pattern, tsubst_flags_t
> > complain)
> >   tree
> >   build_extra_args (tree pattern, tree args, tsubst_flags_t complain)
> >   {
> > -  tree extra = args;
> > +  /* Make a copy of the extra arguments so that they won't get changed
> > + from under us.  */
> > +  tree extra = copy_template_args (args);
> > if (local_specializations)
> >   if (tree locals = extract_local_specs (pattern, complain))
> > extra = tree_cons (NULL_TREE, extra, locals);
> > @@ -12939,7 +12941,7 @@ build_extra_args (tree pattern, tree args,
> > tsubst_flags_t complain)
> >  normal template args to ARGS.  */
> > tree
> > -add_extra_args (tree extra, tree args)
> > +add_extra_args (tree extra, tree args, tsubst_flags_t complain, tree
> > in_decl)
> >   {
> > if (extra && TREE_CODE (extra) == TREE_LIST)
> >   {
> > @@ -12959,20 +12961,14 @@ add_extra_args (tree extra, tree args)
> > gcc_assert (!TREE_PURPOSE (extra));
> > extra = TREE_VALUE (extra);
> >   }
> > -#if 1
> > -  /* I think we should always be able to substitute dependent args into the
> > - pattern.  

Re: [PATCH] c++: requires-expr with dependent extra args [PR101181]

2021-07-08 Thread Jason Merrill via Gcc-patches

On 7/8/21 11:28 AM, Patrick Palka wrote:

Here we're crashing ultimately because the mechanism for delaying
substitution into a requires-expression (or constexpr if) doesn't
expect to see dependent args.  But we end up capturing dependent
args here when substituting into the default template argument during
coerce_template_parms for the dependent specialization p.

This patch enables the commented out code in add_extra_args for
handling this situation.  It turns out we also need to make a copy of
the captured arguments so that coerce_template_parms doesn't later
add to the argument, which would form an unexpected cycle.  And we
need to make tsubst_template_args more forgiving about missing template
arguments, since the arguments we capture from coerce_template_parms are
incomplete.

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

PR c++/101181

gcc/cp/ChangeLog:

* constraint.cc (tsubst_requires_expr): Pass complain/in_decl to
add_extra_args.
* cp-tree.h (add_extra_args): Add complain/in_decl parameters.
* pt.c (build_extra_args): Make a copy of args.
(add_extra_args): Add complain/in_decl parameters.  Handle the
case where the extra arguments are dependent.
(tsubst_pack_expansion): Pass complain/in_decl to
add_extra_args.
(tsubst_template_args): Handle missing template arguments.
(tsubst_expr) : Pass complain/in_decl to
add_extra_args.

gcc/testsuite/ChangeLog:

* g++.dg/cpp2a/concepts-requires26.C: New test.
* g++.dg/cpp2a/lambda-uneval16.C: New test.
---
  gcc/cp/constraint.cc  |  3 +-
  gcc/cp/cp-tree.h  |  2 +-
  gcc/cp/pt.c   | 31 +--
  .../g++.dg/cpp2a/concepts-requires26.C| 18 +++
  gcc/testsuite/g++.dg/cpp2a/lambda-uneval16.C  | 22 +
  5 files changed, 58 insertions(+), 18 deletions(-)
  create mode 100644 gcc/testsuite/g++.dg/cpp2a/concepts-requires26.C
  create mode 100644 gcc/testsuite/g++.dg/cpp2a/lambda-uneval16.C

diff --git a/gcc/cp/constraint.cc b/gcc/cp/constraint.cc
index 99d3ccc6998..4ee5215df50 100644
--- a/gcc/cp/constraint.cc
+++ b/gcc/cp/constraint.cc
@@ -2266,7 +2266,8 @@ tsubst_requires_expr (tree t, tree args, sat_info info)
/* A requires-expression is an unevaluated context.  */
cp_unevaluated u;
  
-  args = add_extra_args (REQUIRES_EXPR_EXTRA_ARGS (t), args);

+  args = add_extra_args (REQUIRES_EXPR_EXTRA_ARGS (t), args,
+info.complain, info.in_decl);
if (processing_template_decl)
  {
/* We're partially instantiating a generic lambda.  Substituting into
diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h
index 58da7460001..0a5f13489cc 100644
--- a/gcc/cp/cp-tree.h
+++ b/gcc/cp/cp-tree.h
@@ -7289,7 +7289,7 @@ extern void add_mergeable_specialization(bool 
is_decl, bool is_alias,
 tree outer, unsigned);
  extern tree add_to_template_args  (tree, tree);
  extern tree add_outermost_template_args   (tree, tree);
-extern tree add_extra_args (tree, tree);
+extern tree add_extra_args (tree, tree, tsubst_flags_t, 
tree);
  extern tree build_extra_args  (tree, tree, tsubst_flags_t);
  
  /* in rtti.c */

diff --git a/gcc/cp/pt.c b/gcc/cp/pt.c
index 06116d16887..e4bdac087ad 100644
--- a/gcc/cp/pt.c
+++ b/gcc/cp/pt.c
@@ -12928,7 +12928,9 @@ extract_local_specs (tree pattern, tsubst_flags_t 
complain)
  tree
  build_extra_args (tree pattern, tree args, tsubst_flags_t complain)
  {
-  tree extra = args;
+  /* Make a copy of the extra arguments so that they won't get changed
+ from under us.  */
+  tree extra = copy_template_args (args);
if (local_specializations)
  if (tree locals = extract_local_specs (pattern, complain))
extra = tree_cons (NULL_TREE, extra, locals);
@@ -12939,7 +12941,7 @@ build_extra_args (tree pattern, tree args, 
tsubst_flags_t complain)
 normal template args to ARGS.  */
  
  tree

-add_extra_args (tree extra, tree args)
+add_extra_args (tree extra, tree args, tsubst_flags_t complain, tree in_decl)
  {
if (extra && TREE_CODE (extra) == TREE_LIST)
  {
@@ -12959,20 +12961,14 @@ add_extra_args (tree extra, tree args)
gcc_assert (!TREE_PURPOSE (extra));
extra = TREE_VALUE (extra);
  }
-#if 1
-  /* I think we should always be able to substitute dependent args into the
- pattern.  If that turns out to be incorrect in some cases, enable the
- alternate code (and add complain/in_decl parms to this function).  */


Ah, because these cases aren't pack expansions, so we aren't trying to 
do the substitution; I wonder if it would be feasible to do so.  But 
this approach is probably simpler.  OK.



-  gcc_checking_assert (!uses_template_parms (extra));
-#else
-  if (!use

[PATCH] c++: requires-expr with dependent extra args [PR101181]

2021-07-08 Thread Patrick Palka via Gcc-patches
Here we're crashing ultimately because the mechanism for delaying
substitution into a requires-expression (or constexpr if) doesn't
expect to see dependent args.  But we end up capturing dependent
args here when substituting into the default template argument during
coerce_template_parms for the dependent specialization p.

This patch enables the commented out code in add_extra_args for
handling this situation.  It turns out we also need to make a copy of
the captured arguments so that coerce_template_parms doesn't later
add to the argument, which would form an unexpected cycle.  And we
need to make tsubst_template_args more forgiving about missing template
arguments, since the arguments we capture from coerce_template_parms are
incomplete.

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

PR c++/101181

gcc/cp/ChangeLog:

* constraint.cc (tsubst_requires_expr): Pass complain/in_decl to
add_extra_args.
* cp-tree.h (add_extra_args): Add complain/in_decl parameters.
* pt.c (build_extra_args): Make a copy of args.
(add_extra_args): Add complain/in_decl parameters.  Handle the
case where the extra arguments are dependent.
(tsubst_pack_expansion): Pass complain/in_decl to
add_extra_args.
(tsubst_template_args): Handle missing template arguments.
(tsubst_expr) : Pass complain/in_decl to
add_extra_args.

gcc/testsuite/ChangeLog:

* g++.dg/cpp2a/concepts-requires26.C: New test.
* g++.dg/cpp2a/lambda-uneval16.C: New test.
---
 gcc/cp/constraint.cc  |  3 +-
 gcc/cp/cp-tree.h  |  2 +-
 gcc/cp/pt.c   | 31 +--
 .../g++.dg/cpp2a/concepts-requires26.C| 18 +++
 gcc/testsuite/g++.dg/cpp2a/lambda-uneval16.C  | 22 +
 5 files changed, 58 insertions(+), 18 deletions(-)
 create mode 100644 gcc/testsuite/g++.dg/cpp2a/concepts-requires26.C
 create mode 100644 gcc/testsuite/g++.dg/cpp2a/lambda-uneval16.C

diff --git a/gcc/cp/constraint.cc b/gcc/cp/constraint.cc
index 99d3ccc6998..4ee5215df50 100644
--- a/gcc/cp/constraint.cc
+++ b/gcc/cp/constraint.cc
@@ -2266,7 +2266,8 @@ tsubst_requires_expr (tree t, tree args, sat_info info)
   /* A requires-expression is an unevaluated context.  */
   cp_unevaluated u;
 
-  args = add_extra_args (REQUIRES_EXPR_EXTRA_ARGS (t), args);
+  args = add_extra_args (REQUIRES_EXPR_EXTRA_ARGS (t), args,
+info.complain, info.in_decl);
   if (processing_template_decl)
 {
   /* We're partially instantiating a generic lambda.  Substituting into
diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h
index 58da7460001..0a5f13489cc 100644
--- a/gcc/cp/cp-tree.h
+++ b/gcc/cp/cp-tree.h
@@ -7289,7 +7289,7 @@ extern void add_mergeable_specialization(bool 
is_decl, bool is_alias,
 tree outer, unsigned);
 extern tree add_to_template_args   (tree, tree);
 extern tree add_outermost_template_args(tree, tree);
-extern tree add_extra_args (tree, tree);
+extern tree add_extra_args (tree, tree, tsubst_flags_t, 
tree);
 extern tree build_extra_args   (tree, tree, tsubst_flags_t);
 
 /* in rtti.c */
diff --git a/gcc/cp/pt.c b/gcc/cp/pt.c
index 06116d16887..e4bdac087ad 100644
--- a/gcc/cp/pt.c
+++ b/gcc/cp/pt.c
@@ -12928,7 +12928,9 @@ extract_local_specs (tree pattern, tsubst_flags_t 
complain)
 tree
 build_extra_args (tree pattern, tree args, tsubst_flags_t complain)
 {
-  tree extra = args;
+  /* Make a copy of the extra arguments so that they won't get changed
+ from under us.  */
+  tree extra = copy_template_args (args);
   if (local_specializations)
 if (tree locals = extract_local_specs (pattern, complain))
   extra = tree_cons (NULL_TREE, extra, locals);
@@ -12939,7 +12941,7 @@ build_extra_args (tree pattern, tree args, 
tsubst_flags_t complain)
normal template args to ARGS.  */
 
 tree
-add_extra_args (tree extra, tree args)
+add_extra_args (tree extra, tree args, tsubst_flags_t complain, tree in_decl)
 {
   if (extra && TREE_CODE (extra) == TREE_LIST)
 {
@@ -12959,20 +12961,14 @@ add_extra_args (tree extra, tree args)
   gcc_assert (!TREE_PURPOSE (extra));
   extra = TREE_VALUE (extra);
 }
-#if 1
-  /* I think we should always be able to substitute dependent args into the
- pattern.  If that turns out to be incorrect in some cases, enable the
- alternate code (and add complain/in_decl parms to this function).  */
-  gcc_checking_assert (!uses_template_parms (extra));
-#else
-  if (!uses_template_parms (extra))
+  if (uses_template_parms (extra))
 {
-  gcc_unreachable ();
+  /* This can happen during dependent substitution into a requires-expr
+or a lambda that uses constexpr if.  */
   extra = tsubst_template_args (extra