[C++ PATCH] Reject self-recursive constexpr calls even in templates (PR c++/70449)

[C++ PATCH] Reject self-recursive constexpr calls even in templates (PR c++/70449)

[Jakub Jelinek]

Hi!

As the testcase shows, when not in a template, cxx_eval_call_expression
already complains about self-recursive calls in constexpr contexts,
but if we are in a function template, we ICE on the testcase,
because we try to instantiate the function template we are in the middle of
parsing, e.g. function_end_locus is UNKNOWN_LOCATION, and only the
statements that have been already parsed are in there.

The patch attempts to reject that.
Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

2016-04-01  Jakub Jelinek  <jakub@redhat.com>

	PR c++/70449
	* constexpr.c (cxx_eval_call_expression): Before calling
	instantiate_decl check if not trying to instantiate current
	function template and handle that the same as if
	fun == current_function_decl.

	* g++.dg/cpp1y/pr70449.C: New test.

--- gcc/cp/constexpr.c.jj	2016-03-29 19:31:21.000000000 +0200
+++ gcc/cp/constexpr.c	2016-04-01 16:26:53.591088640 +0200
@@ -1293,6 +1293,27 @@ cxx_eval_call_expression (const constexp
   if (!DECL_INITIAL (fun)
       && DECL_TEMPLOID_INSTANTIATION (fun))
     {
+      tree d = fun;
+      if (DECL_CLONED_FUNCTION_P (d))
+	d = DECL_CLONED_FUNCTION (d);
+      d = template_for_substitution (d);
+      if (DECL_TEMPLATE_RESULT (d) == current_function_decl)
+	{
+	  /* A call to the current function template, i.e.
+	     template <typename T>
+	     constexpr int f (int i) {
+	       constexpr int j = f<T>(i-1);
+	       return j;
+	     }
+	     This would be OK without the constexpr on the declaration
+	     of j.  */
+	  if (!ctx->quiet)
+	    error_at (loc, "%qD called in a constant expression before its "
+			   "definition is complete", fun);
+	  *non_constant_p = true;
+	  return t;
+	}
+
       ++function_depth;
       instantiate_decl (fun, /*defer_ok*/false, /*expl_inst*/false);
       --function_depth;
--- gcc/testsuite/g++.dg/cpp1y/pr70449.C.jj	2016-04-01 16:42:55.055190752 +0200
+++ gcc/testsuite/g++.dg/cpp1y/pr70449.C	2016-04-01 16:43:43.207545314 +0200
@@ -0,0 +1,26 @@
+// PR c++/70449
+// { dg-do compile { target c++14 } }
+// { dg-options "-Wall" }
+
+template <int N>
+constexpr int f1 ()
+{
+  enum E { a = f1<0> () }; // { dg-error "called in a constant expression before its definition is complete|is not an integer constant" }
+  return 0;
+}
+
+template <int N>
+constexpr int f2 ()
+{
+  enum E { a = f2<0> () };
+  return 0;
+}
+
+constexpr int f3 ()
+{
+  enum E { a = f3 () };	// { dg-error "called in a constant expression before its definition is complete|is not an integer constant" }
+  return 0; 
+}
+
+constexpr int c = f1<0> ();
+constexpr int d = f3 ();

	Jakub

Re: [C++ PATCH] Reject self-recursive constexpr calls even in templates (PR c++/70449)

[Jason Merrill]

On 04/01/2016 03:19 PM, Jakub Jelinek wrote:
> As the testcase shows, when not in a template, cxx_eval_call_expression
> already complains about self-recursive calls in constexpr contexts,
> but if we are in a function template, we ICE on the testcase,
> because we try to instantiate the function template we are in the middle of
> parsing, e.g. function_end_locus is UNKNOWN_LOCATION, and only the
> statements that have been already parsed are in there.

That's odd, we should have failed to instantiate the template. 
Investigating further, it seems that we can check for DECL_INITIAL == 
error_mark_node to tell that a function is still being defined.  So this 
patch does that, and also replaces my earlier fix for 70344.

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

Re: [C++ PATCH] Reject self-recursive constexpr calls even in templates (PR c++/70449)

[Jason Merrill]

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

On 04/01/2016 09:34 PM, Jason Merrill wrote:
> On 04/01/2016 03:19 PM, Jakub Jelinek wrote:
>> As the testcase shows, when not in a template, cxx_eval_call_expression
>> already complains about self-recursive calls in constexpr contexts,
>> but if we are in a function template, we ICE on the testcase,
>> because we try to instantiate the function template we are in the
>> middle of
>> parsing, e.g. function_end_locus is UNKNOWN_LOCATION, and only the
>> statements that have been already parsed are in there.
>
> That's odd, we should have failed to instantiate the template.
> Investigating further, it seems that we can check for DECL_INITIAL ==
> error_mark_node to tell that a function is still being defined.  So this
> patch does that, and also replaces my earlier fix for 70344.
>
> Tested x86_64-pc-linux-gnu, applying to trunk.
>
...with the patch.

[-- Attachment #2: 70449.patch --]
[-- Type: text/x-patch, Size: 2847 bytes --]

commit 522aeea737412d552c78b58466280cf9e6c38924
Author: Jason Merrill <jason@redhat.com>
Date:   Fri Apr 1 20:30:09 2016 -0400

    	PR c++/70449
    	PR c++/70344
    	* pt.c (instantiate_decl): A function isn't fully defined if
    	DECL_INITIAL is error_mark_node.
    	* constexpr.c (cxx_eval_call_expression): Likewise.

diff --git a/gcc/cp/constexpr.c b/gcc/cp/constexpr.c
index ea605dc..979f01f 100644
--- a/gcc/cp/constexpr.c
+++ b/gcc/cp/constexpr.c
@@ -1239,21 +1239,6 @@ cxx_eval_call_expression (const constexpr_ctx *ctx, tree t,
       return t;
     }
 
-  if (fun == current_function_decl)
-    {
-      /* A call to the current function, i.e.
-	 constexpr int f (int i) {
-	   constexpr int j = f(i-1);
-	   return j;
-	 }
-	 This would be OK without the constexpr on the declaration of j.  */
-      if (!ctx->quiet)
-	error_at (loc, "%qD called in a constant expression before its "
-		  "definition is complete", fun);
-      *non_constant_p = true;
-      return t;
-    }
-
   constexpr_ctx new_ctx = *ctx;
   if (DECL_CONSTRUCTOR_P (fun) && !ctx->object
       && TREE_CODE (t) == AGGR_INIT_EXPR)
@@ -1308,7 +1293,10 @@ cxx_eval_call_expression (const constexpr_ctx *ctx, tree t,
         {
 	  if (!ctx->quiet)
 	    {
-	      if (DECL_INITIAL (fun))
+	      if (DECL_INITIAL (fun) == error_mark_node)
+		error_at (loc, "%qD called in a constant expression before its "
+			  "definition is complete", fun);
+	      else if (DECL_INITIAL (fun))
 		{
 		  /* The definition of fun was somehow unsuitable.  */
 		  error_at (loc, "%qD called in a constant expression", fun);
diff --git a/gcc/cp/pt.c b/gcc/cp/pt.c
index a6398c0..2d93a5c 100644
--- a/gcc/cp/pt.c
+++ b/gcc/cp/pt.c
@@ -21741,7 +21741,8 @@ instantiate_decl (tree d, int defer_ok,
   if (TREE_CODE (d) == FUNCTION_DECL)
     {
       deleted_p = DECL_DELETED_FN (code_pattern);
-      pattern_defined = (DECL_SAVED_TREE (code_pattern) != NULL_TREE
+      pattern_defined = ((DECL_SAVED_TREE (code_pattern) != NULL_TREE
+			  && DECL_INITIAL (code_pattern) != error_mark_node)
 			 || DECL_DEFAULTED_OUTSIDE_CLASS_P (code_pattern)
 			 || deleted_p);
     }
diff --git a/gcc/testsuite/g++.dg/cpp1y/constexpr-recursion1.C b/gcc/testsuite/g++.dg/cpp1y/constexpr-recursion1.C
new file mode 100644
index 0000000..79e0b5a
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp1y/constexpr-recursion1.C
@@ -0,0 +1,16 @@
+// PR c++/70449
+// { dg-do compile { target c++14 } }
+// { dg-options "-Wall" }
+
+template <int N>
+constexpr int f1 ()
+{
+  enum E { a = f1<0> () }; // { dg-error "called in a constant expression before its definition is complete|is not an integer constant" }
+  return 0;
+}
+
+constexpr int f3 ()
+{
+  enum E { a = f3 () };	// { dg-error "called in a constant expression before its definition is complete|is not an integer constant" }
+  return 0;
+}

Re: [C++ PATCH] Reject self-recursive constexpr calls even in templates (PR c++/70449)

[Jason Merrill]

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

On 04/01/2016 09:35 PM, Jason Merrill wrote:
> On 04/01/2016 09:34 PM, Jason Merrill wrote:
>> On 04/01/2016 03:19 PM, Jakub Jelinek wrote:
>>> As the testcase shows, when not in a template, cxx_eval_call_expression
>>> already complains about self-recursive calls in constexpr contexts,
>>> but if we are in a function template, we ICE on the testcase,
>>> because we try to instantiate the function template we are in the
>>> middle of
>>> parsing, e.g. function_end_locus is UNKNOWN_LOCATION, and only the
>>> statements that have been already parsed are in there.
>>
>> That's odd, we should have failed to instantiate the template.
>> Investigating further, it seems that we can check for DECL_INITIAL ==
>> error_mark_node to tell that a function is still being defined.  So this
>> patch does that, and also replaces my earlier fix for 70344.

This doesn't quite work for the 70344 testcase when optimizing, because 
we call cp_fold_function between when we set DECL_INITIAL to a BLOCK and 
when we lower invisiref parameters, which leads to confusion when we try 
to evaluate a recursive call.  So let's put back the check against 
current_function_decl.

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


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

commit a65d5bc7e718824989e02233e6edf990afb15358
Author: Jason Merrill <jason@redhat.com>
Date:   Fri May 20 12:35:30 2016 -0400

    	PR c++/70344 - ICE with recursive constexpr
    
    	* constexpr.c (cxx_eval_call_expression): Check for
    	fun == current_function_decl again.

diff --git a/gcc/cp/constexpr.c b/gcc/cp/constexpr.c
index 7b56260..bb723f4 100644
--- a/gcc/cp/constexpr.c
+++ b/gcc/cp/constexpr.c
@@ -1371,11 +1371,17 @@ cxx_eval_call_expression (const constexpr_ctx *ctx, tree t,
   else
     {
       new_call.fundef = retrieve_constexpr_fundef (fun);
-      if (new_call.fundef == NULL || new_call.fundef->body == NULL)
+      if (new_call.fundef == NULL || new_call.fundef->body == NULL
+	  || fun == current_function_decl)
         {
 	  if (!ctx->quiet)
 	    {
-	      if (DECL_INITIAL (fun) == error_mark_node)
+	      /* We need to check for current_function_decl here in case we're
+		 being called during cp_fold_function, because at that point
+		 DECL_INITIAL is set properly and we have a fundef but we
+		 haven't lowered invisirefs yet (c++/70344).  */
+	      if (DECL_INITIAL (fun) == error_mark_node
+		  || fun == current_function_decl)
 		error_at (loc, "%qD called in a constant expression before its "
 			  "definition is complete", fun);
 	      else if (DECL_INITIAL (fun))
diff --git a/gcc/testsuite/g++.dg/cpp0x/constexpr-recursion2.C b/gcc/testsuite/g++.dg/cpp0x/constexpr-recursion2.C
index 978b998..ce2280c 100644
--- a/gcc/testsuite/g++.dg/cpp0x/constexpr-recursion2.C
+++ b/gcc/testsuite/g++.dg/cpp0x/constexpr-recursion2.C
@@ -1,5 +1,6 @@
 // PR c++/70344
 // { dg-do compile { target c++11 } }
+// { dg-options -O }
 
 struct Z
 {