public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
From: Jason Merrill <jason@redhat.com>
To: gcc-patches List <gcc-patches@gcc.gnu.org>
Subject: Re: C++ PATCH for c++/84489, dependent default template argument
Date: Fri, 23 Mar 2018 22:14:00 -0000	[thread overview]
Message-ID: <CADzB+2=MfP39sGGB4Hh9FBUrYP+gGPgyD7ri=J58FW9pwFyNoA@mail.gmail.com> (raw)
In-Reply-To: <CADzB+2m=LGcK+OOCKMXu3MSU8zbq0-15xiJ91TsNqhx=pUTcrQ@mail.gmail.com>

[-- Attachment #1: Type: text/plain, Size: 992 bytes --]

On Tue, Feb 27, 2018 at 12:26 PM, Jason Merrill <jason@redhat.com> wrote:
> The logic in type_unification_real for handling template parms that
> depend on earlier template parms is a bit complicated.  It already
> recognizes when the type of the parm depends on something not
> available yet, and it dealt with the case where substituting partial
> args left some template parm uses behind, but it didn't handle the
> case where substituting partial args just failed.

And that was still wrong, leading to the regression on the later
testcase in 78489.  When substitution fails, that still needs to cause
deduction to fail immediately.  The important part of the 84489 patch
was setting processing_template_decl so unresolved template parms
don't cause substitution failure.

And to handle the first testcase in 78489, we also need to substitute
into the type of non-type parameters even if we aren't going to use
their default arguments yet.

Tested x86_64-pc-linux-gnu, applying to trunk.

[-- Attachment #2: 78489.diff --]
[-- Type: text/x-patch, Size: 5126 bytes --]

commit 02864b90c69f3af56054a627e834dd66a0aa0c80
Author: Jason Merrill <jason@redhat.com>
Date:   Fri Mar 23 11:15:05 2018 -0400

            PR c++/78489 - wrong SFINAE behavior.
    
            PR c++/84489
            * pt.c (type_unification_real): Don't defer substitution failure.

diff --git a/gcc/cp/pt.c b/gcc/cp/pt.c
index 5293c2b5491..8af29cef8d3 100644
--- a/gcc/cp/pt.c
+++ b/gcc/cp/pt.c
@@ -19996,41 +19996,49 @@ type_unification_real (tree tparms,
 	  if (targ || tparm == error_mark_node)
 	    continue;
 	  tree parm = TREE_VALUE (tparm);
-
-	  tsubst_flags_t fcomplain = complain;
-	  if (saw_undeduced == 1)
-	    {
-	      /* When saw_undeduced == 1, substitution into parm and arg might
-		 fail or not replace all template parameters, and that's
-		 fine.  */
-	      fcomplain = tf_none;
-	      if (TREE_CODE (parm) == PARM_DECL
-		  && uses_template_parms (TREE_TYPE (parm)))
-		continue;
-	    }
-
 	  tree arg = TREE_PURPOSE (tparm);
 	  reopen_deferring_access_checks (*checks);
 	  location_t save_loc = input_location;
 	  if (DECL_P (parm))
 	    input_location = DECL_SOURCE_LOCATION (parm);
-
 	  if (saw_undeduced == 1)
 	    ++processing_template_decl;
-	  arg = tsubst_template_arg (arg, full_targs, fcomplain, NULL_TREE);
+
+	  if (saw_undeduced == 1
+	      && TREE_CODE (parm) == PARM_DECL
+	      && uses_template_parms (TREE_TYPE (parm)))
+	    {
+	      /* The type of this non-type parameter depends on undeduced
+		 parameters.  Don't try to use its default argument yet,
+		 but do check whether the arguments we already have cause
+		 substitution failure, so that that happens before we try
+		 later default arguments (78489).  */
+	      tree type = tsubst (TREE_TYPE (parm), full_targs, complain,
+				  NULL_TREE);
+	      if (type == error_mark_node)
+		arg = error_mark_node;
+	      else
+		arg = NULL_TREE;
+	    }
+	  else
+	    {
+	      arg = tsubst_template_arg (arg, full_targs, complain, NULL_TREE);
+
+	      if (!uses_template_parms (arg))
+		arg = convert_template_argument (parm, arg, full_targs,
+						 complain, i, NULL_TREE);
+	      else if (saw_undeduced == 1)
+		arg = NULL_TREE;
+	      else
+		arg = error_mark_node;
+	    }
+
 	  if (saw_undeduced == 1)
 	    --processing_template_decl;
-
-	  if (arg != error_mark_node && !uses_template_parms (arg))
-	    arg = convert_template_argument (parm, arg, full_targs, complain,
-					     i, NULL_TREE);
-	  else if (saw_undeduced == 1)
-	    arg = NULL_TREE;
-	  else
-	    arg = error_mark_node;
 	  input_location = save_loc;
 	  *checks = get_deferred_access_checks ();
 	  pop_deferring_access_checks ();
+
 	  if (arg == error_mark_node)
 	    return 1;
 	  else if (arg)
diff --git a/gcc/testsuite/g++.dg/cpp0x/fntmpdefarg4a.C b/gcc/testsuite/g++.dg/cpp0x/fntmpdefarg4a.C
new file mode 100644
index 00000000000..023d9afdf70
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp0x/fntmpdefarg4a.C
@@ -0,0 +1,6 @@
+// PR c++/55724
+// { dg-do compile { target c++11 } }
+
+template<int N> struct S {};
+template<typename T = int, T N = 42> void f(S<N>) {}
+int main() { S<1> s; f(s); }
diff --git a/gcc/testsuite/g++.dg/cpp0x/sfinae60.C b/gcc/testsuite/g++.dg/cpp0x/sfinae60.C
new file mode 100644
index 00000000000..cfb4dc0b9a7
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp0x/sfinae60.C
@@ -0,0 +1,25 @@
+// PR c++/78489
+// { dg-do compile { target c++11 } }
+
+template <bool P, class T = void> struct enable_if { using type = T; };
+template <class T> struct enable_if<false, T> {};
+
+template <class Dummy> struct use_type { using type = int; };
+
+template <bool Pred>
+struct get_type {
+    static_assert(Pred, "");
+    using type = int;
+};
+
+template <bool Val,
+              class      = typename enable_if<Val>::type, // Evaluation/Substitution should end here
+              class ValT = typename get_type<Val>::type,  // This should not be instantiated
+              typename use_type<ValT>::type = 0           // This NTTP causes ValT to be required
+            >
+constexpr bool test(int) { return false; }
+
+template <bool>
+constexpr bool test(long) { return true; }
+
+static_assert(test<false>(0), ""); // should call test(long)
diff --git a/gcc/testsuite/g++.dg/cpp0x/sfinae61.C b/gcc/testsuite/g++.dg/cpp0x/sfinae61.C
new file mode 100644
index 00000000000..9e7145305e0
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp0x/sfinae61.C
@@ -0,0 +1,21 @@
+// PR c++/78489
+// { dg-do compile { target c++11 } }
+
+template <bool Pred, class T> struct enable_if { typedef T type; };
+template <class T> struct enable_if<false, T> {};
+
+template <int Idx> struct blows_up { static_assert(Idx != Idx, ""); };
+
+template <int Idx,
+           // substitution should fail here
+          typename enable_if<Idx != Idx, int>::type = 0,
+          // GCC evaluates this statement
+          class = typename blows_up<Idx>::type 
+>
+void Foo() {}
+
+// Check the constructor in as SFINAE context
+template <int I> constexpr auto test(int) -> decltype((Foo<I>(), true)) { return true; }
+template <int>   constexpr bool test(long) { return false; }
+
+static_assert(!test<3>(0), ""); // Blows up

      reply	other threads:[~2018-03-23 22:04 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-02-27 17:26 Jason Merrill
2018-03-23 22:14 ` Jason Merrill [this message]

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='CADzB+2=MfP39sGGB4Hh9FBUrYP+gGPgyD7ri=J58FW9pwFyNoA@mail.gmail.com' \
    --to=jason@redhat.com \
    --cc=gcc-patches@gcc.gnu.org \
    /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).