public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
From: Patrick Palka <ppalka@redhat.com>
To: Jason Merrill <jason@redhat.com>
Cc: Patrick Palka <ppalka@redhat.com>, gcc-patches@gcc.gnu.org
Subject: Re: [PATCH] c++: ICE with constrained placeholder return type [PR98346]
Date: Fri, 15 Jan 2021 11:37:19 -0500 (EST)	[thread overview]
Message-ID: <df1f16e-a594-b0e0-9b28-87ce28a8186c@idea> (raw)
In-Reply-To: <43875458-90ee-327d-b7fe-97310bce1c93@redhat.com>

On Mon, 11 Jan 2021, Jason Merrill wrote:

> On 1/7/21 4:06 PM, Patrick Palka wrote:
> > This is essentially a followup to r11-3714 -- we ICEing from another
> > "unguarded" call to build_concept_check, this time in do_auto_deduction,
> > due to the presence of templated trees when !processing_template_decl.
> > 
> > Bootstrapped and regtested on x86_64-pc-linux-gnu, does this look OK for
> > trunk and perhaps the 10 branch?
> > 
> > gcc/cp/ChangeLog:
> > 
> > 	PR c++/98346
> > 	* pt.c (do_auto_deduction): Temporarily increment
> > 	processing_template_decl before calling build_concept_check.
> > 
> > gcc/testsuite/ChangeLog:
> > 
> > 	PR c++/98346
> > 	* g++.dg/cpp2a/concepts-placeholder3.C: New test.
> > ---
> >   gcc/cp/pt.c                                       |  2 ++
> >   .../g++.dg/cpp2a/concepts-placeholder3.C          | 15 +++++++++++++++
> >   2 files changed, 17 insertions(+)
> >   create mode 100644 gcc/testsuite/g++.dg/cpp2a/concepts-placeholder3.C
> > 
> > diff --git a/gcc/cp/pt.c b/gcc/cp/pt.c
> > index beabcc4b027..111a694e0c5 100644
> > --- a/gcc/cp/pt.c
> > +++ b/gcc/cp/pt.c
> > @@ -29464,7 +29464,9 @@ do_auto_deduction (tree type, tree init, tree
> > auto_node,
> >             cargs = targs;
> >     	/* Rebuild the check using the deduced arguments.  */
> > +	++processing_template_decl;
> >   	check = build_concept_check (cdecl, cargs, tf_none);
> > +	--processing_template_decl;
> 
> This shouldn't be necessary; if processing_template_decl is 0, we should have
> non-dependent args.
> 
> I think your patch only works for this testcase because the concept is trivial
> and doesn't actually try to to do anything with the arguments.
> 
> Handling of PLACEHOLDER_TYPE_CONSTRAINTS is overly complex, partly because the
> 'auto' is represented as an argument in its own constraints.
> 
> A constrained auto variable declaration has the same problem.

D'oh, good point..  We need to also substitute the template arguments of
the current instantiation into the constraint at some point.   This is
actually PR96443 / PR96444, which I reported and posted a patch for back
in August: https://gcc.gnu.org/pipermail/gcc-patches/2020-August/551375.html

The approach the August patch used was to substitute into the
PLACEHOLDER_TYPE_CONSTRAINTS during tsubst, which was ruled out.  We can
instead do the same substitution during do_auto_deduction, as in the
patch below.  Does this approach look better?  It seems consistent with
how type_deducible_p substitutes into the return-type-requirement of a
compound-requirement.

Alternatively we could not substitute into PLACEHOLDER_TYPE_CONSTRAINTS
at all and instead pass the targs of the enclosing function directly
into satisfaction, but that seems inconsistent with type_deducible_p.

-- >8 --

Subject: [PATCH] c++: dependent constraint on placeholder return type
 [PR96443]

We're never substituting the template arguments of the enclosing
function into the constraint of a placeholder variable or return type,
which leads to errors during satisfaction when the constraint is
dependent.  This patch fixes this issue by doing the appropriate
substitution in do_auto_deduction before checking satisfaction.

Bootstrapped and regtested on x86_64-pc-linux-gnu, does this look OK
for trunk?  Also tested on cmcstl2 and range-v3.

gcc/cp/ChangeLog:

	PR c++/96443
	* pt.c (do_auto_deduction): Try checking the placeholder
	constraint template parse time.  Substitute the template
	arguments of the containing function into the placeholder
	constraint.  If the constraint is still dependent, defer
	deduction until instantiation time.

gcc/testsuite/ChangeLog:

	PR c++/96443
	* g++.dg/concepts/concepts-ts1.C: Add dg-bogus directive to the
	call to f15 that we expect to accept.
	* g++.dg/cpp2a/concepts-placeholder3.C: New test.
---
 gcc/cp/pt.c                                   | 19 ++++++++++++++++++-
 .../g++.dg/cpp2a/concepts-placeholder3.C      | 16 ++++++++++++++++
 gcc/testsuite/g++.dg/cpp2a/concepts-ts1.C     |  2 +-
 3 files changed, 35 insertions(+), 2 deletions(-)
 create mode 100644 gcc/testsuite/g++.dg/cpp2a/concepts-placeholder3.C

diff --git a/gcc/cp/pt.c b/gcc/cp/pt.c
index c6b7318b378..b70a9a451e1 100644
--- a/gcc/cp/pt.c
+++ b/gcc/cp/pt.c
@@ -29455,7 +29455,7 @@ do_auto_deduction (tree type, tree init, tree auto_node,
     }
 
   /* Check any placeholder constraints against the deduced type. */
-  if (flag_concepts && !processing_template_decl)
+  if (flag_concepts)
     if (tree check = NON_ERROR (PLACEHOLDER_TYPE_CONSTRAINTS (auto_node)))
       {
         /* Use the deduced type to check the associated constraints. If we
@@ -29475,6 +29475,23 @@ do_auto_deduction (tree type, tree init, tree auto_node,
         else
           cargs = targs;
 
+	if ((context == adc_return_type || context == adc_variable_type)
+	    && current_function_decl
+	    && DECL_TEMPLATE_INFO (current_function_decl))
+	  {
+	    /* Substitute the template arguments of the enclosing function.  */
+	    cargs = tsubst_template_args (cargs,
+					  DECL_TI_ARGS (current_function_decl),
+					  complain, current_function_decl);
+	    if (cargs == error_mark_node)
+	      return error_mark_node;
+	  }
+
+	if (any_dependent_template_arguments_p (cargs))
+	  /* The constraint is dependent, so we can't complete the type
+	     deduction ahead of time.  */
+	  return type;
+
 	/* Rebuild the check using the deduced arguments.  */
 	check = build_concept_check (cdecl, cargs, tf_none);
 
diff --git a/gcc/testsuite/g++.dg/cpp2a/concepts-placeholder3.C b/gcc/testsuite/g++.dg/cpp2a/concepts-placeholder3.C
new file mode 100644
index 00000000000..abf5930e902
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp2a/concepts-placeholder3.C
@@ -0,0 +1,16 @@
+// PR c++/96443
+// { dg-do compile { target c++20 } }
+
+template <class T, class U> concept same_as = __is_same(T, U);
+
+auto f(auto x) -> same_as<decltype(x)> auto { return 0; }; // { dg-error "constraints" }
+void g(auto x) { same_as<decltype(x)> auto y = 0; } // { dg-error "constraints" }
+auto h(auto x) -> same_as<decltype(x.missing)> auto { return 0; } // { dg-error "missing" }
+
+int main() {
+  f(0); // { dg-bogus "" }
+  f(true); // { dg-message "required from here" }
+  g(0); // { dg-bogus "" }
+  g(true); // { dg-message "required from here" }
+  h(0); // { dg-message "required from here" }
+}
diff --git a/gcc/testsuite/g++.dg/cpp2a/concepts-ts1.C b/gcc/testsuite/g++.dg/cpp2a/concepts-ts1.C
index 1cefe3b243f..a116cac4ea4 100644
--- a/gcc/testsuite/g++.dg/cpp2a/concepts-ts1.C
+++ b/gcc/testsuite/g++.dg/cpp2a/concepts-ts1.C
@@ -40,7 +40,7 @@ void driver()
   f3('a'); // { dg-error "" }
   f4(0, 0);
   f4(0, 'a'); // { dg-error "" }
-  f15(0);
+  f15(0); // { dg-bogus "" }
   f15('a'); // { dg-message "" }
 }
 
-- 
2.30.0


  parent reply	other threads:[~2021-01-15 16:37 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-01-07 21:06 Patrick Palka
2021-01-11 21:40 ` Jason Merrill
2021-01-11 22:08   ` Jason Merrill
2021-01-15 16:37   ` Patrick Palka [this message]
2021-01-19 21:34     ` Jason Merrill

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=df1f16e-a594-b0e0-9b28-87ce28a8186c@idea \
    --to=ppalka@redhat.com \
    --cc=gcc-patches@gcc.gnu.org \
    --cc=jason@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).