public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [PATCH] Fix ICE with extend_ref_init_temps of structured binding (PR c++/81197)
@ 2017-12-05  0:01 Jakub Jelinek
  2017-12-15 21:03 ` Jason Merrill
  0 siblings, 1 reply; 2+ messages in thread
From: Jakub Jelinek @ 2017-12-05  0:01 UTC (permalink / raw)
  To: Jason Merrill, Nathan Sidwell; +Cc: gcc-patches

Hi!

We were calling cp_finish_decl before cp_finish_decomp, so that the
latter has the initializer finalized and can decompose it, and
cp_finish_decomp was mangling the decl if needed.
Unfortunately, as the following testcase shows, sometimes we need
it mangled already earlier, during that cp_finish_decl call.

Most of the patch below arranges the mangling (which needs just the list
of the identifiers and namespace context) to be done before the
cp_finish_decl.
Another thing is that with the change we wouldn't ICE, but would emit
weird mangled names, like _ZGR7_ZDC1tE0.  The problem is that we don't
keep the list of the corresponding identifiers around and only store
the full mangled name in DECL_ASSEMBLER_NAME, but the DC <source-name>+ E
case us <unqualified-name> and can appear elsewhere, including the lifetime
extended temporary manglings.  The patch just attempts to discover the DC
in the mangled name and use a substring of the mangled name, there should be
no substutions in there.

Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

Note, the _ZGR* mangling still looks weird, the ABI says:

  <special-name> ::= GR <object name> _             # First temporary
  <special-name> ::= GR <object name> <seq-id> _    # Subsequent temporaries

but we instead of the _ or <seq-id> always emit a number, starting with 0.
And, that number increases on every GR we produce, rather than just when
referring to portions of some initializer of the same object.
So, with the patch we e.g. emit _ZGRDC1tE0, while LLVM emits _ZGRDC1tE_
(and ICEs on the other structured binding, where we emit
_ZGRN1A1BDC1u1v2wwEE1 but should be emitting _ZGRN1A1BDC1u1v2wwEE_
probably).

2017-12-05  Jakub Jelinek  <jakub@redhat.com>

	PR c++/81197
	* cp-tree.h (cp_maybe_mangle_decomp): Declare.
	* decl.c (cp_maybe_mangle_decomp): New function.
	(cp_finish_decomp): Don't SET_DECL_ASSEMBLER_NAME here.
	* parser.c (cp_convert_range_for,
	cp_parser_decomposition_declaration): Call cp_maybe_mangle_decomp.
	* pt.c (tsubst_expr): Likewise.
	* mangle.c (write_unqualified_name): Handle DECL_DECOMPOSITION_P
	where DECL_ASSEMBLER_NAME is already set.

	* g++.dg/cpp1z/decomp34.C: New test.

--- gcc/cp/cp-tree.h.jj	2017-12-01 09:19:07.000000000 +0100
+++ gcc/cp/cp-tree.h	2017-12-04 13:14:11.673367545 +0100
@@ -6141,6 +6141,7 @@ extern void start_decl_1			(tree, bool);
 extern bool check_array_initializer		(tree, tree, tree);
 extern void cp_finish_decl			(tree, tree, bool, tree, int);
 extern tree lookup_decomp_type			(tree);
+extern void cp_maybe_mangle_decomp		(tree, tree, unsigned int);
 extern void cp_finish_decomp			(tree, tree, unsigned int);
 extern int cp_complete_array_type		(tree *, tree, bool);
 extern int cp_complete_array_type_or_error	(tree *, tree, bool, tsubst_flags_t);
--- gcc/cp/decl.c.jj	2017-11-30 23:45:45.000000000 +0100
+++ gcc/cp/decl.c	2017-12-04 12:52:44.679418902 +0100
@@ -7359,6 +7359,25 @@ lookup_decomp_type (tree v)
   return *decomp_type_table->get (v);
 }
 
+/* Mangle a decomposition declaration if needed.  Arguments like
+   in cp_finish_decomp.  */
+
+void
+cp_maybe_mangle_decomp (tree decl, tree first, unsigned int count)
+{
+  if (!processing_template_decl
+      && !error_operand_p (decl)
+      && DECL_NAMESPACE_SCOPE_P (decl))
+    {
+      auto_vec<tree, 16> v;
+      v.safe_grow (count);
+      tree d = first;
+      for (unsigned int i = 0; i < count; i++, d = DECL_CHAIN (d))
+	v[count - i - 1] = d;
+      SET_DECL_ASSEMBLER_NAME (decl, mangle_decomp (decl, v));
+    }
+}
+
 /* Finish a decomposition declaration.  DECL is the underlying declaration
    "e", FIRST is the head of a chain of decls for the individual identifiers
    chained through DECL_CHAIN in reverse order and COUNT is the number of
@@ -7630,8 +7649,6 @@ cp_finish_decomp (tree decl, tree first,
 	    DECL_HAS_VALUE_EXPR_P (v[i]) = 1;
 	  }
     }
-  else if (DECL_NAMESPACE_SCOPE_P (decl))
-    SET_DECL_ASSEMBLER_NAME (decl, mangle_decomp (decl, v));
 }
 
 /* Returns a declaration for a VAR_DECL as if:
--- gcc/cp/parser.c.jj	2017-12-01 22:13:19.000000000 +0100
+++ gcc/cp/parser.c	2017-12-04 12:56:04.883923605 +0100
@@ -11926,6 +11926,9 @@ cp_convert_range_for (tree statement, tr
 				     tf_warning_or_error);
   finish_for_expr (expression, statement);
 
+  if (VAR_P (range_decl) && DECL_DECOMPOSITION_P (range_decl))
+    cp_maybe_mangle_decomp (range_decl, decomp_first_name, decomp_cnt);
+
   /* The declaration is initialized with *__begin inside the loop body.  */
   cp_finish_decl (range_decl,
 		  build_x_indirect_ref (input_location, begin, RO_UNARY_STAR,
@@ -13269,6 +13272,7 @@ cp_parser_decomposition_declaration (cp_
 
       if (decl != error_mark_node)
 	{
+	  cp_maybe_mangle_decomp (decl, prev, v.length ());
 	  cp_finish_decl (decl, initializer, non_constant_p, NULL_TREE,
 			  is_direct_init ? LOOKUP_NORMAL : LOOKUP_IMPLICIT);
 	  cp_finish_decomp (decl, prev, v.length ());
--- gcc/cp/pt.c.jj	2017-11-30 23:45:48.000000000 +0100
+++ gcc/cp/pt.c	2017-12-04 12:59:10.309612507 +0100
@@ -16096,19 +16096,23 @@ tsubst_expr (tree t, tree args, tsubst_f
 		    if (VAR_P (decl))
 		      const_init = (DECL_INITIALIZED_BY_CONSTANT_EXPRESSION_P
 				    (pattern_decl));
-		    cp_finish_decl (decl, init, const_init, NULL_TREE, 0);
 		    if (VAR_P (decl)
 			&& DECL_DECOMPOSITION_P (decl)
 			&& TREE_TYPE (pattern_decl) != error_mark_node)
 		      {
 			unsigned int cnt;
 			tree first;
-			decl = tsubst_decomp_names (decl, pattern_decl, args,
-						    complain, in_decl, &first,
-						    &cnt);
-			if (decl != error_mark_node)
-			  cp_finish_decomp (decl, first, cnt);
+			tree ndecl
+			  = tsubst_decomp_names (decl, pattern_decl, args,
+						 complain, in_decl, &first, &cnt);
+			if (ndecl != error_mark_node)
+			  cp_maybe_mangle_decomp (ndecl, first, cnt);
+			cp_finish_decl (decl, init, const_init, NULL_TREE, 0);
+			if (ndecl != error_mark_node)
+			  cp_finish_decomp (ndecl, first, cnt);
 		      }
+		    else
+		      cp_finish_decl (decl, init, const_init, NULL_TREE, 0);
 		  }
 	      }
 	  }
--- gcc/cp/mangle.c.jj	2017-11-17 08:40:32.000000000 +0100
+++ gcc/cp/mangle.c	2017-12-04 14:28:49.132795687 +0100
@@ -1291,7 +1291,56 @@ write_unqualified_name (tree decl)
     {
       found = true;
       gcc_assert (DECL_ASSEMBLER_NAME_SET_P (decl));
-      write_source_name (DECL_ASSEMBLER_NAME (decl));
+      if (VAR_P (decl)
+	  && DECL_DECOMPOSITION_P (decl)
+	  && DECL_NAME (decl) == NULL_TREE
+	  && DECL_NAMESPACE_SCOPE_P (decl))
+	{
+	  const char *p = IDENTIFIER_POINTER (DECL_ASSEMBLER_NAME (decl));
+	  const char *end = p + IDENTIFIER_LENGTH (DECL_ASSEMBLER_NAME (decl));
+	  bool nested = false;
+	  /* The list of identifiers is likely gone at this point, so find the
+	     DC <source-name>+ E part in the mangled name.  */
+	  if (!strncmp (p, "_Z", 2))
+	    {
+	      p += 2;
+	      if (!strncmp (p, "St", 2))
+		p += 2;
+	      else if (*p == 'N')
+		{
+		  nested = true;
+		  ++p;
+		  while (ISDIGIT (p[0]))
+		    {
+		      char *e;
+		      long num = strtol (p, &e, 10);
+		      if (num >= 1 && num < end - e)
+			p = e + num;
+		      else
+			break;
+		    }
+		}
+	      if (strncmp (p, "DC", 2))
+		p = NULL;
+	      if (nested)
+		{
+		  if (end[-1] != 'E')
+		    p = NULL;
+		  else
+		    --end;
+		}
+	      if (end[-1] != 'E')
+		p = NULL;
+	    }
+	  else
+	    p = NULL;
+	  if (p == NULL)
+	    write_source_name (DECL_ASSEMBLER_NAME (decl));
+	  else
+	    write_chars (p, end - p);
+	}
+      else
+	write_source_name (DECL_ASSEMBLER_NAME (decl));
     }
   else if (DECL_DECLARES_FUNCTION_P (decl))
     {
--- gcc/testsuite/g++.dg/cpp1z/decomp34.C.jj	2017-12-04 14:32:40.967956222 +0100
+++ gcc/testsuite/g++.dg/cpp1z/decomp34.C	2017-12-04 14:30:57.000000000 +0100
@@ -0,0 +1,11 @@
+// PR c++/81197
+// { dg-do compile { target c++11 } }
+// { dg-options "" }
+
+struct X { int a; };
+struct Y { int b, c, d; };
+auto&& [t] = X{};	// { dg-warning "structured bindings only available with" "" { target c++14_down } }
+namespace A { namespace B { auto&& [u, v, ww] = Y{}; } }	// { dg-warning "structured bindings only available with" "" { target c++14_down } }
+
+// { dg-final { scan-assembler "_ZGRDC1tE0" } }
+// { dg-final { scan-assembler "_ZGRN1A1BDC1u1v2wwEE1" } }

	Jakub

^ permalink raw reply	[flat|nested] 2+ messages in thread

* Re: [PATCH] Fix ICE with extend_ref_init_temps of structured binding (PR c++/81197)
  2017-12-05  0:01 [PATCH] Fix ICE with extend_ref_init_temps of structured binding (PR c++/81197) Jakub Jelinek
@ 2017-12-15 21:03 ` Jason Merrill
  0 siblings, 0 replies; 2+ messages in thread
From: Jason Merrill @ 2017-12-15 21:03 UTC (permalink / raw)
  To: Jakub Jelinek, Nathan Sidwell; +Cc: gcc-patches

On 12/04/2017 07:00 PM, Jakub Jelinek wrote:
> @@ -1291,7 +1291,56 @@ write_unqualified_name (tree decl)
>       {
>         found = true;
>         gcc_assert (DECL_ASSEMBLER_NAME_SET_P (decl));
> -      write_source_name (DECL_ASSEMBLER_NAME (decl));
> +      if (VAR_P (decl)
> +	  && DECL_DECOMPOSITION_P (decl)
> +	  && DECL_NAME (decl) == NULL_TREE
> +	  && DECL_NAMESPACE_SCOPE_P (decl))
> +	{
> +	  const char *p = IDENTIFIER_POINTER (DECL_ASSEMBLER_NAME (decl));
> +	  const char *end = p + IDENTIFIER_LENGTH (DECL_ASSEMBLER_NAME (decl));
> +	  bool nested = false;
> +	  /* The list of identifiers is likely gone at this point, so find the
> +	     DC <source-name>+ E part in the mangled name.  */
> +	  if (!strncmp (p, "_Z", 2))
> +	    {
> +	      p += 2;
> +	      if (!strncmp (p, "St", 2))
> +		p += 2;
> +	      else if (*p == 'N')
> +		{
> +		  nested = true;
> +		  ++p;
> +		  while (ISDIGIT (p[0]))
> +		    {
> +		      char *e;
> +		      long num = strtol (p, &e, 10);
> +		      if (num >= 1 && num < end - e)
> +			p = e + num;
> +		      else
> +			break;
> +		    }
> +		}
> +	      if (strncmp (p, "DC", 2))
> +		p = NULL;
> +	      if (nested)
> +		{
> +		  if (end[-1] != 'E')
> +		    p = NULL;
> +		  else
> +		    --end;
> +		}
> +	      if (end[-1] != 'E')
> +		p = NULL;
> +	    }
> +	  else
> +	    p = NULL;
> +	  if (p == NULL)
> +	    write_source_name (DECL_ASSEMBLER_NAME (decl));
> +	  else
> +	    write_chars (p, end - p);
> +	}
> +      else
> +	write_source_name (DECL_ASSEMBLER_NAME (decl));
>       }

Please break (most of) this out into a separate function.  OK with that 
change.

Jason

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2017-12-15 21:03 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-12-05  0:01 [PATCH] Fix ICE with extend_ref_init_temps of structured binding (PR c++/81197) Jakub Jelinek
2017-12-15 21:03 ` Jason Merrill

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).