On 3/25/21 6:50 AM, Jakub Jelinek wrote:
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)))))
         {

What if we WALK_SUBTREE (DECL_EXPR_DECL (*tp)) here, instead of the bot_replace hunk? OK either way.

           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


Reply via email to