diff mbox series

[C++] Move mangling alias out of ::

Message ID e24b450b-6e4f-1f08-f4f8-5179867f048a@acm.org
State New
Headers show
Series [C++] Move mangling alias out of :: | expand

Commit Message

Nathan Sidwell Oct. 4, 2017, 4:51 p.m. UTC
To deal with changes in the ABI that affected manglings, we like pushing 
decls into :: under their mangled names.  This is one of the 3 places we 
push decls under something not their DECL_NAME, and causes the namespace 
binding map to not be a simple hash table of DECLs.

It also can give rise to confusing duplicate decl errors that are 
followed by a somewhat obscure note that an ABI change will fix it.

This patch reimplements that machinery using a bespoke hash_map.  This 
allows a better diagnostic that talks about mangled names up front.

Applying to trunk.

nathan

Comments

Bernhard Reutner-Fischer Oct. 5, 2017, 9:28 a.m. UTC | #1
On Wed, Oct 04, 2017 at 12:51:18PM -0400, Nathan Sidwell wrote:

> Applying to trunk.

+void
+record_mangling (tree decl, bool need_warning)
+{
+  if (!mangled_decls)
+    mangled_decls = hash_map<lang_identifier *, tree>::create_ggc (499);
+
+  gcc_checking_assert (DECL_ASSEMBLER_NAME_SET_P (decl));
+  tree id = DECL_ASSEMBLER_NAME (decl);
+  bool existed;
+  tree *slot = &mangled_decls->get_or_insert (id, &existed);
+
+  /* If this is already an alias, remove the alias, because the real
+     decl takes presidence.  */

s/presidence/precedence/ ?

+  if (!existed)
+    ;
+  else if (DECL_ARTIFICIAL (*slot) && DECL_IGNORED_P (*slot))
+    if (symtab_node *n = symtab_node::get (*slot))
+      if (n->cpp_implicit_alias)
+       {
+         n->remove ();
+         existed = false;
+       }

Wouldn't simply
  symtab_node *n;
  if (existed
      && DECL_ARTIFICIAL (*slot)
      && DECL_IGNORED_P (*slot)
      && (n = symtab_node::get (*slot))
      && n->cpp_implicit_alias)
    {
      n->remove ();
      existed = false;
    }
be more in line with the coding style?

thanks,
+
+  if (!existed)
+    *slot = decl;
+  else if (need_warning)
+    {
+      error_at (DECL_SOURCE_LOCATION (decl),
+               "mangling of %q#D as %qE conflicts with a previous mangle",
+               decl, id);
+      inform (DECL_SOURCE_LOCATION (*slot),
+             "previous mangling %q#D", *slot);
+      inform (DECL_SOURCE_LOCATION (decl),
+             "a later -fabi-version= (or =0)"
+             " avoids this error with a change in mangling");
+      *slot = decl;
+    }
+}
Nathan Sidwell Oct. 5, 2017, 10:14 a.m. UTC | #2
On 10/05/2017 05:28 AM, Bernhard Reutner-Fischer wrote:
> On Wed, Oct 04, 2017 at 12:51:18PM -0400, Nathan Sidwell wrote:

> +  /* If this is already an alias, remove the alias, because the real
> +     decl takes presidence.  */
> 
> s/presidence/precedence/ ?

thanks,
> 
> +  if (!existed)
> +    ;
> +  else if (DECL_ARTIFICIAL (*slot) && DECL_IGNORED_P (*slot))

> Wouldn't simply
...
> be more in line with the coding style?

Moving the 'existed' test, but I dislike assignments in the middle of 
conditionals.  I think an earlier version had a non-empty first block, 
but I failed to clean it up.

Fixing thusly.

nathan
diff mbox series

Patch

2017-10-04  Nathan Sidwell  <nathan@acm.org>

	gcc/cp/
	Move mangling aliases out of global namespace.
	* cp-tree.h (record_mangling): New.
	(maybe_remove_implicit_alias): Delete.
	* decl2.c (mangled_decls): New hash map.
	(generate_mangling_alias): Reimplement using mangled_decls.
	(record_mangling): New.
	* mangle.c (decl_implicit_alias_p,
	maybe_remove_implicit_alias): Delete.
	(mangle_decl): Use record_mangling.
	* name-lookup.c (supplement_binding_1): Remove
	maybe_remove_implicit_alias check.

	* call.c (convert_arg_to_ellipsis): Correct comment about passing
	by reference.

	gcc/testsuite/
	* g++.dg/abi/mangle41.C: Adjust diagnostics.

	libcc1/
	* libcp1plugin.cc (supplement_binding): Don't use
	maybe_remove_implicit_alias.

Index: gcc/cp/cp-tree.h
===================================================================
--- gcc/cp/cp-tree.h	(revision 253413)
+++ gcc/cp/cp-tree.h	(working copy)
@@ -6142,6 +6142,7 @@  extern tree finish_case_label			(locatio
 extern tree cxx_maybe_build_cleanup		(tree, tsubst_flags_t);
 
 /* in decl2.c */
+extern void record_mangling			(tree, bool);
 extern void note_mangling_alias			(tree, tree);
 extern void generate_mangling_aliases		(void);
 extern tree build_memfn_type			(tree, tree, cp_cv_quals, cp_ref_qualifier);
@@ -7154,7 +7155,6 @@  extern tree add_exception_specifier		(tr
 extern tree merge_exception_specifiers		(tree, tree);
 
 /* in mangle.c */
-extern bool maybe_remove_implicit_alias		(tree);
 extern void init_mangle				(void);
 extern void mangle_decl				(tree);
 extern const char *mangle_type_string		(tree);
Index: gcc/cp/decl2.c
===================================================================
--- gcc/cp/decl2.c	(revision 253413)
+++ gcc/cp/decl2.c	(working copy)
@@ -102,6 +102,10 @@  static GTY(()) vec<tree, va_gc> *no_link
    is to be an alias for the former if the former is defined.  */
 static GTY(()) vec<tree, va_gc> *mangling_aliases;
 
+/* A hash table of mangled names to decls.  Used to figure out if we
+   need compatibility aliases.  */
+static GTY(()) hash_map<lang_identifier *, tree> *mangled_decls;
+
 /* Nonzero if we're done parsing and into end-of-file activities.  */
 
 int at_eof;
@@ -4290,25 +4294,34 @@  handle_tls_init (void)
 static void
 generate_mangling_alias (tree decl, tree id2)
 {
+  struct cgraph_node *n = NULL;
+
+  if (TREE_CODE (decl) == FUNCTION_DECL)
+    {
+      n = cgraph_node::get (decl);
+      if (!n)
+	/* Don't create an alias to an unreferenced function.  */
+	return;
+    }
+
+  bool existed;
+  tree *slot = &mangled_decls->get_or_insert (id2, &existed);
+
   /* If there's a declaration already using this mangled name,
      don't create a compatibility alias that conflicts.  */
-  if (IDENTIFIER_GLOBAL_VALUE (id2))
-    return;
-
-  struct cgraph_node *n = NULL;
-  if (TREE_CODE (decl) == FUNCTION_DECL
-      && !(n = cgraph_node::get (decl)))
-    /* Don't create an alias to an unreferenced function.  */
+  if (existed)
     return;
 
   tree alias = make_alias_for (decl, id2);
-  SET_IDENTIFIER_GLOBAL_VALUE (id2, alias);
+  *slot = alias;
+
   DECL_IGNORED_P (alias) = 1;
   TREE_PUBLIC (alias) = TREE_PUBLIC (decl);
   DECL_VISIBILITY (alias) = DECL_VISIBILITY (decl);
   if (vague_linkage_p (decl))
     DECL_WEAK (alias) = 1;
-  if (TREE_CODE (decl) == FUNCTION_DECL)
+
+  if (n)
     n->create_same_body_alias (alias, decl);
   else
     varpool_node::create_extra_name_alias (alias, decl);
@@ -4347,6 +4360,50 @@  generate_mangling_aliases ()
   defer_mangling_aliases = false;
 }
 
+/* Record a mangling of DECL, whose DECL_ASSEMBLER_NAME has just been
+   set.  NEED_WARNING is true if we must warn about collisions.  We do
+   this to spot changes in mangling that may require compatibility
+   aliases.  */
+
+void
+record_mangling (tree decl, bool need_warning)
+{
+  if (!mangled_decls)
+    mangled_decls = hash_map<lang_identifier *, tree>::create_ggc (499);
+
+  gcc_checking_assert (DECL_ASSEMBLER_NAME_SET_P (decl));
+  tree id = DECL_ASSEMBLER_NAME (decl);
+  bool existed;
+  tree *slot = &mangled_decls->get_or_insert (id, &existed);
+
+  /* If this is already an alias, remove the alias, because the real
+     decl takes presidence.  */
+  if (!existed)
+    ;
+  else if (DECL_ARTIFICIAL (*slot) && DECL_IGNORED_P (*slot))
+    if (symtab_node *n = symtab_node::get (*slot))
+      if (n->cpp_implicit_alias)
+	{
+	  n->remove ();
+	  existed = false;
+	}
+
+  if (!existed)
+    *slot = decl;
+  else if (need_warning)
+    {
+      error_at (DECL_SOURCE_LOCATION (decl),
+		"mangling of %q#D as %qE conflicts with a previous mangle",
+		decl, id);
+      inform (DECL_SOURCE_LOCATION (*slot),
+	      "previous mangling %q#D", *slot);
+      inform (DECL_SOURCE_LOCATION (decl),
+	      "a later -fabi-version= (or =0)"
+	      " avoids this error with a change in mangling");
+      *slot = decl;
+    }
+}
+
 /* The entire file is now complete.  If requested, dump everything
    to a file.  */
 
Index: gcc/cp/mangle.c
===================================================================
--- gcc/cp/mangle.c	(revision 253413)
+++ gcc/cp/mangle.c	(working copy)
@@ -3783,38 +3783,6 @@  get_mangled_id (tree decl)
   return targetm.mangle_decl_assembler_name (decl, id);
 }
 
-/* If DECL is an implicit mangling alias, return its symtab node; otherwise
-   return NULL.  */
-
-static symtab_node *
-decl_implicit_alias_p (tree decl)
-{
-  if (DECL_P (decl) && DECL_ARTIFICIAL (decl)
-      && DECL_IGNORED_P (decl)
-      && (TREE_CODE (decl) == FUNCTION_DECL
-	  || (VAR_P (decl) && TREE_STATIC (decl))))
-    {
-      symtab_node *n = symtab_node::get (decl);
-      if (n && n->cpp_implicit_alias)
-	return n;
-    }
-  return NULL;
-}
-
-/* If DECL is a mangling alias, remove it from the symbol table and return
-   true; otherwise return false.  */
-
-bool
-maybe_remove_implicit_alias (tree decl)
-{
-  if (symtab_node *n = decl_implicit_alias_p (decl))
-    {
-      n->remove();
-      return true;
-    }
-  return false;
-}
-
 /* Create an identifier for the external mangled name of DECL.  */
 
 void
@@ -3871,29 +3839,11 @@  mangle_decl (const tree decl)
 
       if (!DECL_REALLY_EXTERN (decl))
 	{
-	  bool set = false;
-
-	  /* Check IDENTIFIER_GLOBAL_VALUE before setting to avoid redundant
-	     errors from multiple definitions.  */
-	  tree d = IDENTIFIER_GLOBAL_VALUE (id);
-	  if (!d || decl_implicit_alias_p (d))
-	    {
-	      set = true;
-	      SET_IDENTIFIER_GLOBAL_VALUE (id, decl);
-	    }
+	  record_mangling (decl, G.need_abi_warning);
 
 	  if (!G.need_abi_warning)
 	    return;
 
-	  /* If the mangling will change in the future, emit an alias with the
-	     future mangled name for forward-compatibility.  */
-	  if (!set)
-	    {
-	      SET_IDENTIFIER_GLOBAL_VALUE (id, decl);
-	      inform (DECL_SOURCE_LOCATION (decl), "a later -fabi-version= (or "
-		      "=0) avoids this error with a change in mangling");
-	    }
-
 	  flag_abi_version = flag_abi_compat_version;
 	  id2 = mangle_decl_string (decl);
 	  id2 = targetm.mangle_decl_assembler_name (decl, id2);
Index: gcc/cp/name-lookup.c
===================================================================
--- gcc/cp/name-lookup.c	(revision 253413)
+++ gcc/cp/name-lookup.c	(working copy)
@@ -2255,12 +2255,6 @@  supplement_binding_1 (cxx_binding *bindi
       region to refer only to the namespace to which it already
       refers.  */
     ok = false;
-  else if (maybe_remove_implicit_alias (bval))
-    {
-      /* There was a mangling compatibility alias using this mangled name,
-	 but now we have a real decl that wants to use it instead.  */
-      binding->value = decl;
-    }
   else
     {
       if (!error_operand_p (bval))
Index: gcc/testsuite/g++.dg/abi/mangle41.C
===================================================================
--- gcc/testsuite/g++.dg/abi/mangle41.C	(revision 253413)
+++ gcc/testsuite/g++.dg/abi/mangle41.C	(working copy)
@@ -3,6 +3,6 @@ 
 // { dg-options "-mavx -fabi-version=2" }
 
 #include <x86intrin.h>
-void f(__m128) { }		// { dg-message "previous declaration" }
-void f(__m256) { }		// { dg-error "conflicts" }
+void f(__m128) { }	// { dg-message "previous mangling" }
+void f(__m256) { }	// { dg-error "conflicts with a previous mangle" }
 // { dg-message "mangling" "" { target *-*-* } .-1 }
Index: libcc1/libcp1plugin.cc
===================================================================
--- libcc1/libcp1plugin.cc	(revision 253413)
+++ libcc1/libcp1plugin.cc	(working copy)
@@ -422,12 +422,6 @@  supplement_binding (cxx_binding *binding
       region to refer only to the namespace to which it already
       refers.  */
     ok = false;
-  else if (maybe_remove_implicit_alias (bval))
-    {
-      /* There was a mangling compatibility alias using this mangled name,
-	 but now we have a real decl that wants to use it instead.  */
-      binding->value = decl;
-    }
   else
     {
       // _1: diagnose_name_conflict (decl, bval);
Index: libiberty/cp-demangle.c
===================================================================
--- libiberty/cp-demangle.c	(revision 253413)
+++ libiberty/cp-demangle.c	(working copy)
@@ -1499,6 +1499,52 @@  d_nested_name (struct d_info *di)
   if (*pret == NULL)
     return NULL;
 
+  /* Heuristic to spot Clang's '$_N::operator ()' mangling of generic
+     lambda.  */
+  if ((*pret)->type == DEMANGLE_COMPONENT_TEMPLATE
+      && d_left (*pret)->type == DEMANGLE_COMPONENT_QUAL_NAME
+      && d_left (d_left (*pret))->type == DEMANGLE_COMPONENT_NAME
+      && d_right (d_left (*pret))->type == DEMANGLE_COMPONENT_OPERATOR
+      && d_left (d_left (*pret))->u.s_name.len > 2
+      && d_left (d_left (*pret))->u.s_name.s[0] == '$'
+      && d_left (d_left (*pret))->u.s_name.s[1] == '_'
+      && !strcmp (d_right (d_left (*pret))->u.s_operator.op->code, "cl"))
+    {
+      const char *name = d_left (d_left (*pret))->u.s_name.s;
+      int ix, len = d_left (d_left (*pret))->u.s_name.len;
+      int disc = 0;
+
+      /* Use the N part as the discriminator.  */
+      for (ix = 2; ix != len && IS_DIGIT (name[ix]); ix++)
+	disc = disc * 10 + (name[ix] - '0');
+
+      if (ix == len)
+	{
+	  /* Directly turn the node into a lambda.  Any future
+	     substitutions will then get this replacement.  */
+	  struct demangle_component *lam = d_left (d_left (*pret));
+	  struct demangle_component *inst = d_right (*pret);
+	  struct demangle_component **pptr = &lam->u.s_unary_num.sub;
+	  int arg_no = 0;
+
+	  lam->type = DEMANGLE_COMPONENT_LAMBDA;
+	  lam->u.s_unary_num.num = disc;
+
+	  /* Insert as many auto parameters as there are template
+	     arguments.  This is not complete in the general case
+	     (there could be non-auto parameters). */
+	  for (; inst; inst = inst->u.s_binary.right, arg_no++)
+	    {
+	      struct demangle_component *parm
+		= d_make_template_param (di, arg_no);
+	      parm = d_make_comp (di, DEMANGLE_COMPONENT_ARGLIST, parm, NULL);
+	      *pptr = parm;
+	      pptr = &d_right (parm);
+	    }
+	  *pptr = NULL;
+	}
+    }
+
   if (rqual)
     {
       d_left (rqual) = ret;
Index: libiberty/testsuite/demangle-expected
===================================================================
--- libiberty/testsuite/demangle-expected	(revision 253413)
+++ libiberty/testsuite/demangle-expected	(working copy)
@@ -4764,3 +4764,16 @@  Foo<int>()::X::fn
 _ZZZ3FooIiEfvENKUlT_E_clIcEEDaS0_EN1X2fnEv
 Foo<int>()::{lambda(auto:1)#1}::operator()<char>(char) const::X::fn()
 Foo<int>()::{lambda(auto:1)#1}::operator()<char>(char) const::X::fn
+# clang mangle for something inside a generic lambda
+--no-params
+_ZZ3FoovENK3$_0clIiEEfT_
+float Foo()::{lambda(auto:1)#1}::operator()<int>(int) const
+Foo()::{lambda(auto:1)#1}::operator()<int>
+--no-params
+_ZZ3FooP1XENK3$_0clIiEEfT_P1Y
+float Foo(X*)::{lambda(auto:1)#1}::operator()<int>(int, Y*) const
+Foo(X*)::{lambda(auto:1)#1}::operator()<int>
+--no-params
+_ZZZ3FooP1XENK3$_0clIiEEfT_P1YEN5Local2fnES5_
+float Foo(X*)::{lambda(auto:1)#1}::operator()<int>(int, Y*) const::Local::fn(Y*)
+float Foo(X*)::{lambda(auto:1)#1}::operator()<int>(int, Y*) const::Local::fn