diff mbox series

Commonize anon-name generation

Message ID 9398e11d-d8d4-ed5a-3e9f-fa65bb7b3b96@acm.org
State New
Headers show
Series Commonize anon-name generation | expand

Commit Message

Nathan Sidwell May 24, 2019, 2:34 p.m. UTC 
Currently gcc/tree.c exports an anonymous name format string, and a 
predicate to detect identifiers of that form.  The C++ and D FEs use 
those functions to create and check anonymous names.

That seems a little duplicative.  This patch changes tree.c to export an 
anonymous name generator, and uses a spare bit in IDENTIFIERs to mark 
them as anonymous, so no more strcmp to distinguish them.  Then replaces 
the bespoke handling in the two front ends to use the new interface.

While my original goal was to make there be only one anon-identifier in 
the C++ FE (because modules kind of would prefer not to have names 
depend on source ordering), I didn't get to that goal, I think we still 
sometimes place them in symbol tables.  But this still seems a good thing.

Are the common and D pieces ok?

Jason, lambda class names are similarly anonymous, and I have a patch 
for them too.  Would you prefer they just be from the same set of 
identifiers, with another C++-specific flag bit, or have a somewhat more 
distinct name, as they do now? (I.e. do you want to be able to 
distinguish lambdas when debugging just by looking at the string?)

nathan

Comments

Jason Merrill May 24, 2019, 2:41 p.m. UTC | #1 
On Fri, May 24, 2019 at 10:34 AM Nathan Sidwell <nathan@acm.org> wrote:
>
> Currently gcc/tree.c exports an anonymous name format string, and a
> predicate to detect identifiers of that form.  The C++ and D FEs use
> those functions to create and check anonymous names.
>
> That seems a little duplicative.  This patch changes tree.c to export an
> anonymous name generator, and uses a spare bit in IDENTIFIERs to mark
> them as anonymous, so no more strcmp to distinguish them.  Then replaces
> the bespoke handling in the two front ends to use the new interface.
>
> While my original goal was to make there be only one anon-identifier in
> the C++ FE (because modules kind of would prefer not to have names
> depend on source ordering), I didn't get to that goal, I think we still
> sometimes place them in symbol tables.  But this still seems a good thing.
>
> Are the common and D pieces ok?
>
> Jason, lambda class names are similarly anonymous, and I have a patch
> for them too.  Would you prefer they just be from the same set of
> identifiers, with another C++-specific flag bit, or have a somewhat more
> distinct name, as they do now? (I.e. do you want to be able to
> distinguish lambdas when debugging just by looking at the string?)

I don't have a strong opinion.  It can be useful to recognize them
that way, but I don't imagine I'd miss it too much.

Jason
Iain Buclaw May 24, 2019, 5:06 p.m. UTC | #2 
On Fri, 24 May 2019 at 16:34, Nathan Sidwell <nathan@acm.org> wrote:
>
> Currently gcc/tree.c exports an anonymous name format string, and a
> predicate to detect identifiers of that form.  The C++ and D FEs use
> those functions to create and check anonymous names.
>
> That seems a little duplicative.  This patch changes tree.c to export an
> anonymous name generator, and uses a spare bit in IDENTIFIERs to mark
> them as anonymous, so no more strcmp to distinguish them.  Then replaces
> the bespoke handling in the two front ends to use the new interface.
>
> While my original goal was to make there be only one anon-identifier in
> the C++ FE (because modules kind of would prefer not to have names
> depend on source ordering), I didn't get to that goal, I think we still
> sometimes place them in symbol tables.  But this still seems a good thing.
>
> Are the common and D pieces ok?
>

Seems reasonable to me.  Don't have a strong opinion either on how it's done.
diff mbox series

Patch

2019-05-24  Nathan Sidwell  <nathan@acm.org>

	gcc/
	* tree.h (IDENTIFIER_ANON_P): New.
	(anon_aggrname_format, anon_aggname_p): Don't declare.
	(make_anon_name): Declare.
	* lto-streamer-out.c (DFS::DFS_write_tree_body): Use IDENTIFIER_ANON_P.
	(hash_tree): Likewise.
	* tree-streamer-out.c (write_ts_decl_minimal_tree): Likewise.
	* tree.c (anon_aggrname_p, anon_aggrname_format): Delete.
	(anon_cnt, make_anon_name): New.

 	gcc/cp/
	* cp-tree.h (make_anon_name): Drop declaration.
	(TYPE_UNNAMED_P): Use IDENTIFIER_ANON_P.
	* cp-lang.c (cxx_dwarf_name): Likewise.
	* class.c (find_flexarrays): Likewise.
	* decl.c (name_unnamed_type, xref_tag_1): Likewise.
	* error.c (dump_aggr_type): Likewise.
	* pt.c (push_template_decl_real): Likewise.
	* name-lookup.c (consider_binding_level): Likewise.
	(anon_cnt, make_anon_name): Delete.

	gcc/d/
	* types.cc (fixup_anonymous_offset): Use IDENTIFIER_ANON_P.
	(layout_aggregate_members): Use make_anon_name.

Index: gcc/cp/class.c
===================================================================
--- gcc/cp/class.c	(revision 271599)
+++ gcc/cp/class.c	(working copy)
@@ -6586,5 +6586,5 @@  find_flexarrays (tree t, flexmems_t *fme
 	  && DECL_IMPLICIT_TYPEDEF_P (fld)
 	  && CLASS_TYPE_P (TREE_TYPE (fld))
-	  && anon_aggrname_p (DECL_NAME (fld)))
+	  && IDENTIFIER_ANON_P (DECL_NAME (fld)))
 	{
 	  /* Check the nested unnamed type referenced via a typedef
Index: gcc/cp/cp-lang.c
===================================================================
--- gcc/cp/cp-lang.c	(revision 271599)
+++ gcc/cp/cp-lang.c	(working copy)
@@ -111,5 +111,5 @@  cxx_dwarf_name (tree t, int verbosity)
 
   if (DECL_NAME (t)
-      && (anon_aggrname_p (DECL_NAME (t)) || LAMBDA_TYPE_P (t)))
+      && (IDENTIFIER_ANON_P (DECL_NAME (t)) || LAMBDA_TYPE_P (t)))
     return NULL;
   if (verbosity >= 2)
Index: gcc/cp/cp-tree.h
===================================================================
--- gcc/cp/cp-tree.h	(revision 271599)
+++ gcc/cp/cp-tree.h	(working copy)
@@ -1938,5 +1938,5 @@  enum languages { lang_c, lang_cplusplus
 /* Nonzero if NODE has no name for linkage purposes.  */
 #define TYPE_UNNAMED_P(NODE) \
-  (OVERLOAD_TYPE_P (NODE) && anon_aggrname_p (TYPE_LINKAGE_IDENTIFIER (NODE)))
+  (OVERLOAD_TYPE_P (NODE) && IDENTIFIER_ANON_P (TYPE_LINKAGE_IDENTIFIER (NODE)))
 
 /* The _DECL for this _TYPE.  */
@@ -6351,5 +6351,4 @@  extern tree strip_fnptr_conv			(tree);
 /* in name-lookup.c */
 extern void maybe_push_cleanup_level		(tree);
-extern tree make_anon_name			(void);
 extern tree maybe_push_decl			(tree);
 extern tree current_decl_namespace		(void);
Index: gcc/cp/decl.c
===================================================================
--- gcc/cp/decl.c	(revision 271599)
+++ gcc/cp/decl.c	(working copy)
@@ -10234,13 +10234,10 @@  name_unnamed_type (tree type, tree decl)
   /* Replace the anonymous name with the real name everywhere.  */
   for (tree t = TYPE_MAIN_VARIANT (type); t; t = TYPE_NEXT_VARIANT (t))
-    {
-      if (anon_aggrname_p (TYPE_IDENTIFIER (t)))
-	/* We do not rename the debug info representing the
-	   unnamed tagged type because the standard says in
-	   [dcl.typedef] that the naming applies only for
-	   linkage purposes.  */
-	/*debug_hooks->set_name (t, decl);*/
-	TYPE_NAME (t) = decl;
-    }
+    if (IDENTIFIER_ANON_P (TYPE_IDENTIFIER (t)))
+      /* We do not rename the debug info representing the unnamed
+	 tagged type because the standard says in [dcl.typedef] that
+	 the naming applies only for linkage purposes.  */
+      /*debug_hooks->set_name (t, decl);*/
+      TYPE_NAME (t) = decl;
 
   if (TYPE_LANG_SPECIFIC (type))
@@ -14062,5 +14059,5 @@  xref_tag_1 (enum tag_types tag_code, tre
      make type node and push name.  Name lookup is not required.  */
   tree t = NULL_TREE;
-  if (scope != ts_lambda && !anon_aggrname_p (name))
+  if (scope != ts_lambda && !IDENTIFIER_ANON_P (name))
     t = lookup_and_check_tag  (tag_code, name, scope, template_header_p);
   
Index: gcc/cp/error.c
===================================================================
--- gcc/cp/error.c	(revision 271599)
+++ gcc/cp/error.c	(working copy)
@@ -739,5 +739,5 @@  dump_aggr_type (cxx_pretty_printer *pp,
     }
 
-  if (name == 0 || anon_aggrname_p (name))
+  if (!name || IDENTIFIER_ANON_P (name))
     {
       if (flags & TFF_CLASS_KEY_OR_ENUM)
Index: gcc/cp/name-lookup.c
===================================================================
--- gcc/cp/name-lookup.c	(revision 271599)
+++ gcc/cp/name-lookup.c	(working copy)
@@ -3798,23 +3798,7 @@  constructor_name_p (tree name, tree type
 }
 
-/* Counter used to create anonymous type names.  */
-
-static GTY(()) int anon_cnt;
-
-/* Return an IDENTIFIER which can be used as a name for
-   unnamed structs and unions.  */
-
-tree
-make_anon_name (void)
-{
-  char buf[32];
-
-  sprintf (buf, anon_aggrname_format (), anon_cnt++);
-  return get_identifier (buf);
-}
-
-/* This code is practically identical to that for creating
-   anonymous names, but is just used for lambdas instead.  This isn't really
-   necessary, but it's convenient to avoid treating lambdas like other
+/* This code is practically identical to that for creating anonymous
+   names, but is just used for lambdas instead.  This isn't really
+   necessary, but it's convenient to avoid mistaking lambdas for other
    unnamed types.  */
 
@@ -5983,5 +5967,5 @@  consider_binding_level (tree name, best_
       /* Don't suggest names that are for anonymous aggregate types, as
 	 they are an implementation detail generated by the compiler.  */
-      if (anon_aggrname_p (suggestion))
+      if (IDENTIFIER_ANON_P (suggestion))
 	continue;
 
Index: gcc/cp/pt.c
===================================================================
--- gcc/cp/pt.c	(revision 271599)
+++ gcc/cp/pt.c	(working copy)
@@ -5502,5 +5502,5 @@  push_template_decl_real (tree decl, bool
 	member_template_p = true;
       if (TREE_CODE (decl) == TYPE_DECL
-	  && anon_aggrname_p (DECL_NAME (decl)))
+	  && IDENTIFIER_ANON_P (DECL_NAME (decl)))
 	{
 	  error ("template class without a name");
Index: gcc/d/types.cc
===================================================================
--- gcc/d/types.cc	(revision 271599)
+++ gcc/d/types.cc	(working copy)
@@ -240,5 +240,5 @@  fixup_anonymous_offset (tree fields, tre
 	 Set the anonymous decl offset to its first member.  */
       tree ftype = TREE_TYPE (fields);
-      if (TYPE_NAME (ftype) && anon_aggrname_p (TYPE_IDENTIFIER (ftype)))
+      if (TYPE_NAME (ftype) && IDENTIFIER_ANON_P (TYPE_IDENTIFIER (ftype)))
 	{
 	  tree vfields = TYPE_FIELDS (ftype);
@@ -325,10 +325,5 @@  layout_aggregate_members (Dsymbols *memb
       if (ad != NULL)
 	{
-	  /* Use a counter to create anonymous type names.  */
-	  static int anon_cnt = 0;
-	  char buf[32];
-	  sprintf (buf, anon_aggrname_format (), anon_cnt++);
-
-	  tree ident = get_identifier (buf);
+	  tree ident = make_anon_name ();
 	  tree type = make_node (ad->isunion ? UNION_TYPE : RECORD_TYPE);
 	  ANON_AGGR_TYPE_P (type) = 1;
Index: gcc/lto-streamer-out.c
===================================================================
--- gcc/lto-streamer-out.c	(revision 271599)
+++ gcc/lto-streamer-out.c	(working copy)
@@ -783,5 +783,5 @@  DFS::DFS_write_tree_body (struct output_
       if (DECL_NAME (expr)
 	  && TREE_CODE (DECL_NAME (expr)) == IDENTIFIER_NODE
-	  && anon_aggrname_p (DECL_NAME (expr)))
+	  && IDENTIFIER_ANON_P (DECL_NAME (expr)))
 	;
       else
@@ -1212,5 +1212,5 @@  hash_tree (struct streamer_tree_cache_d
       if (DECL_NAME (t)
 	  && TREE_CODE (DECL_NAME (t)) == IDENTIFIER_NODE
-	  && anon_aggrname_p (DECL_NAME (t)))
+	  && IDENTIFIER_ANON_P (DECL_NAME (t)))
 	;
       else
Index: gcc/tree-streamer-out.c
===================================================================
--- gcc/tree-streamer-out.c	(revision 271599)
+++ gcc/tree-streamer-out.c	(working copy)
@@ -580,5 +580,5 @@  write_ts_decl_minimal_tree_pointers (str
   if (DECL_NAME (expr)
       && TREE_CODE (DECL_NAME (expr)) == IDENTIFIER_NODE
-      && anon_aggrname_p (DECL_NAME (expr)))
+      && IDENTIFIER_ANON_P (DECL_NAME (expr)))
     stream_write_tree (ob, NULL_TREE, ref_p);
   else
Index: gcc/tree.c
===================================================================
--- gcc/tree.c	(revision 271599)
+++ gcc/tree.c	(working copy)
@@ -9746,38 +9746,30 @@  clean_symbol_name (char *p)
 }
 
-/* For anonymous aggregate types, we need some sort of name to
-   hold on to.  In practice, this should not appear, but it should
-   not be harmful if it does.  */
-bool 
-anon_aggrname_p(const_tree id_node)
-{
-#ifndef NO_DOT_IN_LABEL
- return (IDENTIFIER_POINTER (id_node)[0] == '.'
-	 && IDENTIFIER_POINTER (id_node)[1] == '_');
-#else /* NO_DOT_IN_LABEL */
-#ifndef NO_DOLLAR_IN_LABEL
-  return (IDENTIFIER_POINTER (id_node)[0] == '$' \
-	  && IDENTIFIER_POINTER (id_node)[1] == '_');
-#else /* NO_DOLLAR_IN_LABEL */
-#define ANON_AGGRNAME_PREFIX "__anon_"
-  return (!strncmp (IDENTIFIER_POINTER (id_node), ANON_AGGRNAME_PREFIX, 
-		    sizeof (ANON_AGGRNAME_PREFIX) - 1));
-#endif	/* NO_DOLLAR_IN_LABEL */
-#endif	/* NO_DOT_IN_LABEL */
-}
+static GTY(()) unsigned anon_cnt = 0; /* Saved for PCH.  */
+
+/* Create a unique anonymous identifier.  The identifier is still a
+   valid assembly label.  */
 
-/* Return a format for an anonymous aggregate name.  */
-const char *
-anon_aggrname_format()
+tree
+make_anon_name ()
 {
-#ifndef NO_DOT_IN_LABEL
- return "._%d";
-#else /* NO_DOT_IN_LABEL */
-#ifndef NO_DOLLAR_IN_LABEL
-  return "$_%d";
-#else /* NO_DOLLAR_IN_LABEL */
-  return "__anon_%d";
-#endif	/* NO_DOLLAR_IN_LABEL */
-#endif	/* NO_DOT_IN_LABEL */
+  const char *fmt = 
+#if !defined (NO_DOT_IN_LABEL)
+    "."
+#elif !defined (NO_DOLLAR_IN_LABEL)
+    "$"
+#else
+    "_"
+#endif
+    "_anon_%d";
+
+  char buf[24];
+  int len = snprintf (buf, sizeof (buf), fmt, anon_cnt++);
+  gcc_checking_assert (len < int (sizeof (buf)));
+
+  tree id = get_identifier_with_length (buf, len);
+  IDENTIFIER_ANON_P (id) = true;
+
+  return id;
 }
 
Index: gcc/tree.h
===================================================================
--- gcc/tree.h	(revision 271599)
+++ gcc/tree.h	(working copy)
@@ -933,4 +933,9 @@  extern void omp_clause_range_check_faile
   ((NODE)->base.deprecated_flag)
 
+/* Nonzero indicates an IDENTIFIER_NODE that names an anonymous
+   aggregate, (as created by anon_aggr_name_format).  */
+#define IDENTIFIER_ANON_P(NODE) \
+  (IDENTIFIER_NODE_CHECK (NODE)->base.private_flag)
+
 /* Nonzero in an IDENTIFIER_NODE if the name is a local alias, whose
    uses are to be substituted for uses of the TREE_CHAINed identifier.  */
@@ -5442,7 +5447,7 @@  target_opts_for_fn (const_tree fndecl)
 /* For anonymous aggregate types, we need some sort of name to
    hold on to.  In practice, this should not appear, but it should
-   not be harmful if it does.  */
-extern const char *anon_aggrname_format();
-extern bool anon_aggrname_p (const_tree);
+   not be harmful if it does.  Identifiers returned will be
+   IDENTIFIER_ANON_P.  */
+extern tree make_anon_name ();
 
 /* The tree and const_tree overload templates.   */