On Tue, Mar 23, 2021 at 04:51:52PM -0400, Jason Merrill via Gcc-patches wrote:
> > + if (TREE_CODE (*tp) == DECL_EXPR
> > + && VAR_P (DECL_EXPR_DECL (*tp))
> > + && DECL_ARTIFICIAL (DECL_EXPR_DECL (*tp))
> > + && !TREE_STATIC (DECL_EXPR_DECL (*tp))
> > + && DECL_CONTEXT (DECL_EXPR_DECL (*tp)) == NULL_TREE
>
> I might drop the DECL_CONTEXT check; I'd think any embedded temporaries that
> happen to have it set would also need this treatment.
>
> > + && !splay_tree_lookup (target_remap,
> > + (splay_tree_key) DECL_EXPR_DECL (*tp)))
> > + {
> > + tree t = create_temporary_var (TREE_TYPE (DECL_EXPR_DECL (*tp)));
>
> You don't need to copy DECL_INITIAL here?
Ok, I had another look at both of these issues.
The reason for the nsdmi12.C failures with the removed DECL_CONTEXT ... ===
NULL_TREE
check was that apparently it is not just walk_tree_1 BIND_EXPR handling that
walks DECL_INITIAL:
case BIND_EXPR:
{
tree decl;
for (decl = BIND_EXPR_VARS (*tp); decl; decl = DECL_CHAIN (decl))
{
/* Walk the DECL_INITIAL and DECL_SIZE. We don't want to walk
into declarations that are just mentioned, rather than
declared; they don't really belong to this part of the tree.
And, we can see cycles: the initializer for a declaration
can refer to the declaration itself. */
WALK_SUBTREE (DECL_INITIAL (decl));
WALK_SUBTREE (DECL_SIZE (decl));
WALK_SUBTREE (DECL_SIZE_UNIT (decl));
}
WALK_SUBTREE_TAIL (BIND_EXPR_BODY (*tp));
}
but it is also cp_walk_subtrees DECL_EXPR handling:
case DECL_EXPR:
/* User variables should be mentioned in BIND_EXPR_VARS
and their initializers and sizes walked when walking
the containing BIND_EXPR. Compiler temporaries are
handled here. And also normal variables in templates,
since do_poplevel doesn't build a BIND_EXPR then. */
if (VAR_P (TREE_OPERAND (*tp, 0))
&& (processing_template_decl
|| (DECL_ARTIFICIAL (TREE_OPERAND (*tp, 0))
&& !TREE_STATIC (TREE_OPERAND (*tp, 0)))))
{
tree decl = TREE_OPERAND (*tp, 0);
WALK_SUBTREE (DECL_INITIAL (decl));
WALK_SUBTREE (DECL_SIZE (decl));
WALK_SUBTREE (DECL_SIZE_UNIT (decl));
}
break;
(though e.g. cp/optimize.c (clone_body) uses plain walk_tree and therefore
won't walk the DECL_INITIAL of the temporaries, so why it is better for them
to be in BIND_EXPRs).
Anyway, what happens in the nsdmi12.C case is that in the newly added
bot_manip code BIND_EXPR handling we decide to clone a temporary with
DECL_INITIAL, replace it in the new BIND_EXPR and by walking the subtrees
we duplicate its DECL_INITIAL. But then when walking with bot_replace, due
to the above mentioned cp_walk_subtrees code we first bot_replace the
DECL_INITIAL of the original temporary rather than its copy when we
encounter DECL_EXPR for it, and only afterwards when walking the subtrees
of the DECL_EXPR replace the temporary with its clone, which means that
both original temporary and copy's DECL_INITIAL will now contain other
temporary's copies and of course that is fatal.
I have fixed that by the bot_replace hunk, that the code changes DECL_EXPR
operand if needed already when walking the DECL_EXPR. That means that
cp_walk_subtrees called right after that will already see the new decl
and DTRT.
And, given the cp_walk_subtrees DECL_INITIAL handling, I'm copying
DECL_INITIAL also for temporaries that have just DECL_EXPR and aren't
mentioned in BIND_EXPR, because there is some chance it could work.
Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
2021-03-25 Jakub Jelinek <ja...@redhat.com>
PR c++/99705
* tree.c (bot_manip): Remap artificial automatic temporaries mentioned
in DECL_EXPR or in BIND_EXPR_VARS.
(bot_replace): For DECL_EXPR, replace DECL_EXPR_DECL if needed before
walking subtrees.
* g++.dg/cpp0x/new5.C: New test.
--- gcc/cp/tree.c.jj 2021-03-23 10:20:42.823717414 +0100
+++ gcc/cp/tree.c 2021-03-24 20:06:40.385176204 +0100
@@ -3128,6 +3128,35 @@ bot_manip (tree* tp, int* walk_subtrees,
}
return NULL_TREE;
}
+ if (TREE_CODE (*tp) == DECL_EXPR
+ && VAR_P (DECL_EXPR_DECL (*tp))
+ && DECL_ARTIFICIAL (DECL_EXPR_DECL (*tp))
+ && !TREE_STATIC (DECL_EXPR_DECL (*tp))
+ && !splay_tree_lookup (target_remap,
+ (splay_tree_key) DECL_EXPR_DECL (*tp)))
+ {
+ tree t = create_temporary_var (TREE_TYPE (DECL_EXPR_DECL (*tp)));
+ DECL_INITIAL (t) = DECL_INITIAL (DECL_EXPR_DECL (*tp));
+ splay_tree_insert (target_remap, (splay_tree_key) DECL_EXPR_DECL (*tp),
+ (splay_tree_value) t);
+ }
+ if (TREE_CODE (*tp) == BIND_EXPR && BIND_EXPR_VARS (*tp))
+ {
+ copy_tree_r (tp, walk_subtrees, NULL);
+ for (tree *p = &BIND_EXPR_VARS (*tp); *p; p = &DECL_CHAIN (*p))
+ {
+ gcc_assert (VAR_P (*p) && DECL_ARTIFICIAL (*p) && !TREE_STATIC (*p));
+ tree t = create_temporary_var (TREE_TYPE (*p));
+ DECL_INITIAL (t) = DECL_INITIAL (*p);
+ DECL_CHAIN (t) = DECL_CHAIN (*p);
+ splay_tree_insert (target_remap, (splay_tree_key) *p,
+ (splay_tree_value) t);
+ *p = t;
+ }
+ if (data.clear_location && EXPR_HAS_LOCATION (*tp))
+ SET_EXPR_LOCATION (*tp, input_location);
+ return NULL_TREE;
+ }
/* Make a copy of this node. */
t = copy_tree_r (tp, walk_subtrees, NULL);
@@ -3178,6 +3207,18 @@ bot_replace (tree* t, int* /*walk_subtre
*t = build_base_path (PLUS_EXPR, TREE_OPERAND (*t, 0), binfo, true,
tf_warning_or_error);
}
+ else if (TREE_CODE (*t) == DECL_EXPR && VAR_P (DECL_EXPR_DECL (*t)))
+ {
+ /* For DECL_EXPRs where the variable needs to be remapped,
+ remap it before recursing on subtrees, because cp_walk_subtrees
+ might walk DECL_INITIAL etc. and we must not clobber DECL_INITIAL
+ etc. of the original decl. */
+ splay_tree_node n
+ = splay_tree_lookup (target_remap,
+ (splay_tree_key) DECL_EXPR_DECL (*t));
+ if (n)
+ DECL_EXPR_DECL (*t) = (tree) n->value;
+ }
return NULL_TREE;
}
--- gcc/testsuite/g++.dg/cpp0x/new5.C.jj 2021-03-24 17:44:02.629963259
+0100
+++ gcc/testsuite/g++.dg/cpp0x/new5.C 2021-03-24 17:44:02.629963259 +0100
@@ -0,0 +1,21 @@
+// PR c++/99705
+// { dg-do compile { target c++11 } }
+
+template <typename T>
+struct C
+{
+ C () { f (); }
+ ~C () {}
+ static void f () {}
+};
+
+struct X
+{
+ X ();
+ int n = 10;
+ C<int> *p = new C<int>[n];
+};
+
+X::X ()
+{
+}
Jakub
