2016-04-26 Nathan Sidwell <nathan@acm.org>
cp/
* constexpr.c (get_fundef_copy): Use the original function for
non-recursive evaluations.
(save_fundef_copy): Always expect a slot to be available.
testsuite/
* g++.dg/cpp0x/constexpr-recursion3.C: New.
* g++.dg/ubsan/pr63956.C: Adjust error location.
===================================================================
@@ -989,7 +989,8 @@ maybe_initialize_fundef_copies_table ()
}
/* Reuse a copy or create a new unshared copy of the function FUN.
- Return this copy. */
+ Return this copy. We use a TREE_LIST whose PURPOSE is body, VALUE
+ is parms, TYPE is result. */
static tree
get_fundef_copy (tree fun)
@@ -997,15 +998,26 @@ get_fundef_copy (tree fun)
maybe_initialize_fundef_copies_table ();
tree copy;
- tree *slot = fundef_copies_table->get (fun);
- if (slot == NULL || *slot == NULL_TREE)
+ bool existed;
+ tree *slot = &fundef_copies_table->get_or_insert (fun, &existed);
+
+ if (!existed)
+ {
+ /* There is no cached function available, or in use. We can use
+ the function directly. That the slot is now created records
+ that this function is now in use. */
+ copy = build_tree_list (DECL_SAVED_TREE (fun), DECL_ARGUMENTS (fun));
+ TREE_TYPE (copy) = DECL_RESULT (fun);
+ }
+ else if (*slot == NULL_TREE)
{
+ /* We've already used the function itself, so make a copy. */
copy = build_tree_list (NULL, NULL);
- /* PURPOSE is body, VALUE is parms, TYPE is result. */
TREE_PURPOSE (copy) = copy_fn (fun, TREE_VALUE (copy), TREE_TYPE (copy));
}
else
{
+ /* We have a cached function available. */
copy = *slot;
*slot = TREE_CHAIN (copy);
}
@@ -1013,12 +1025,14 @@ get_fundef_copy (tree fun)
return copy;
}
-/* Save the copy COPY of function FUN for later reuse by get_fundef_copy(). */
+/* Save the copy COPY of function FUN for later reuse by
+ get_fundef_copy(). By construction, there will always be an entry
+ to find. */
static void
save_fundef_copy (tree fun, tree copy)
{
- tree *slot = &fundef_copies_table->get_or_insert (fun, NULL);
+ tree *slot = fundef_copies_table->get (fun);
TREE_CHAIN (copy) = *slot;
*slot = copy;
}
===================================================================
@@ -0,0 +1,14 @@
+// { dg-do compile { target c++11 } }
+
+constexpr int Foo (int i)
+{
+ return (i ? Foo (i - 1): 0) + i;
+}
+
+static int a = Foo (0);
+static int b = Foo (1);
+static int d = Foo (3);
+static int c = Foo (2);
+static int e = Foo (4);
+static int g = Foo (6);
+static int f = Foo (5);
===================================================================
@@ -92,7 +92,7 @@ constexpr int
fn6 (const int &a, int b)
{
if (b != 2)
- b = a;
+ b = a; // { dg-error "is not a constant expression" }
return b;
}
@@ -106,7 +106,7 @@ fn7 (const int *a, int b)
constexpr int n1 = 7;
constexpr int n2 = fn7 (&n1, 5);
-constexpr int n3 = fn7 ((const int *) 0, 8); // { dg-error "is not a constant expression" }
+constexpr int n3 = fn7 ((const int *) 0, 8);
constexpr int
fn8 (int i)
Jason. we currently clone constexpr function bodies when evaluating them (via the reuse cache that caused such fun). The cloning is necessary so decls in different (recursive) evaluations are distinct. We only need to clone for recursive evaluations though -- the outermost evaluation could use the original function body. That leaves us with needing some mechanism to know whether we're doing a recursive call or not. This patch implements such an optimization. get_fundef_copy is changed to use get-or-insert, and uses the 'existed' arg of get_or_insert to know whether a new slot was created or not. If a new slot was created, we know we're not currently evaluating the function, so we can use it directly without copy. Otherwise, we look in the slot to see if there's a cached body available. If there isn't we create a clone. When saving the function for later reuse, the original fn is not distinguished from copies. That's ok -- we'll just see it as a cached body available for reuse on the next call. and if we run out of cached bodies, we'll create them because the slot will not have just been created. Manually instrumenting the new testcase shows things behaving as I expected. The change in error behaviour of ubsan/pr63956.C was a surprise, but I think a good one. That testcase boiled down to: constexpr int fn6 (const int &a, int b) { b = a; <-- problem here return b; } constexpr int fn7 (const int *a, int b) { return fn6 (*a, b); } constexpr int n4 = fn7 ((const int *) 0, 8); <-- diag here and we emitted the following diagnostic: pr63956.ii:15:24: in constexpr expansion of 'fn7(0u, 8)' pr63956.ii:12:14: in constexpr expansion of 'fn6((* a), b)' pr63956.ii:15:43: error: 'a' is not a constant expression constexpr int n4 = fn7 ((const int *) 0, 8); notice that final line is the assignment of 'n4' and not the location of the problem. With this patch we now produce: pr63956.ii: At global scope: pr63956.ii:14:24: in constexpr expansion of 'fn7(0u, 8)' pr63956.ii:11:14: in constexpr expansion of 'fn6((* a), b)' pr63956.ii:5:7: error: 'a' is not a constant expression b = a; which is better. There's something about the function cloning that throws off the error location information. Changing fn6 to be conditionally recursive will lead to the older error location behaviour when the problem is the cloned instance. IMHO this is a collateral latent bug, and now it only affects recursive constexpr fns. ok for trunk? nathan