diff mbox series

[C++] give builtin types consistent name

Message ID 012eb489-2db5-1fba-6041-7ed18b4d286a@acm.org
State New
Headers show
Series [C++] give builtin types consistent name | expand

Commit Message

Nathan Sidwell Oct. 4, 2017, 5:52 p.m. UTC 
The builtin type machinery is one of the places were we push decls into 
:: under not-their-name.

There's no real reason for this, and the fix is simple -- create a new 
TYPE_DECL if we use a name from the ridpointer table.  I took the 
opportunity of reordering record_builtin_type, which was a series of 
interleaved conditionals dealing with different things.

I add an assert to set_global_binding that the given name is DECL_NAME. 
Of course this raises the opportunity of just making set_global_binding 
have a single arg, like pushdecl and friends.  And then the two macros
IDENTIFIER_GLOBAL_VALUE and SET_IDENTIFIER_GLOBAL_VALUE could be 
dispensed with and we have simply
   tree get_global_value (tree identifier);
   tree set_global_value (tree decl);
perhaps 's/value/decl/'?

Jason, any preference?

set_global_value has to do the stat_hack dance, I can't recall the case 
where it happens, but I recall it biting me when I tried to dispense 
with it.

The remaining mismatched name case is the anonymous namespace.  Once 
that's fixed we can change namespace bindings from a map to a hash, and 
halve its size.

Applying to trunk.

nathan

Comments

Jason Merrill Oct. 4, 2017, 6:03 p.m. UTC | #1 
On Wed, Oct 4, 2017 at 1:52 PM, Nathan Sidwell <nathan@acm.org> wrote:
> The builtin type machinery is one of the places were we push decls into ::
> under not-their-name.
>
> There's no real reason for this, and the fix is simple -- create a new
> TYPE_DECL if we use a name from the ridpointer table.  I took the
> opportunity of reordering record_builtin_type, which was a series of
> interleaved conditionals dealing with different things.
>
> I add an assert to set_global_binding that the given name is DECL_NAME. Of
> course this raises the opportunity of just making set_global_binding have a
> single arg, like pushdecl and friends.  And then the two macros
> IDENTIFIER_GLOBAL_VALUE and SET_IDENTIFIER_GLOBAL_VALUE could be dispensed
> with and we have simply
>   tree get_global_value (tree identifier);
>   tree set_global_value (tree decl);
> perhaps 's/value/decl/'?
>
> Jason, any preference?

I lean toward "binding".

Jason
diff mbox series

Patch

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

	Give builtin types the correct name.
	* name-lookup.c (set_global_binding): Assert name is DECL_NAME.
	* decl.c (record_builtin_type): Reimplement, use new TYPE_DECL for
	rname.

Index: decl.c
===================================================================
--- decl.c	(revision 253413)
+++ decl.c	(working copy)
@@ -3895,47 +3895,47 @@  make_unbound_class_template (tree contex
 /* Push the declarations of builtin types into the global namespace.
    RID_INDEX is the index of the builtin type in the array
    RID_POINTERS.  NAME is the name used when looking up the builtin
-   type.  TYPE is the _TYPE node for the builtin type.  */
+   type.  TYPE is the _TYPE node for the builtin type.
+
+   The calls to SET_IDENTIFIER_GLOBAL_VALUE below should be
+   eliminated.  Built-in types should not be looked up name; their
+   names are keywords that the parser can recognize.  However, there
+   is code in c-common.c that uses identifier_global_value to look up
+   built-in types by name.  */
 
 void
 record_builtin_type (enum rid rid_index,
 		     const char* name,
 		     tree type)
 {
-  tree rname = NULL_TREE, tname = NULL_TREE;
-  tree tdecl = NULL_TREE;
+  tree decl = NULL_TREE;
 
-  if ((int) rid_index < (int) RID_MAX)
-    rname = ridpointers[(int) rid_index];
   if (name)
-    tname = get_identifier (name);
-
-  /* The calls to SET_IDENTIFIER_GLOBAL_VALUE below should be
-     eliminated.  Built-in types should not be looked up name; their
-     names are keywords that the parser can recognize.  However, there
-     is code in c-common.c that uses identifier_global_value to look
-     up built-in types by name.  */
-  if (tname)
     {
-      tdecl = build_decl (BUILTINS_LOCATION, TYPE_DECL, tname, type);
+      tree tname = get_identifier (name);
+      tree tdecl = build_decl (BUILTINS_LOCATION, TYPE_DECL, tname, type);
       DECL_ARTIFICIAL (tdecl) = 1;
       SET_IDENTIFIER_GLOBAL_VALUE (tname, tdecl);
+      decl = tdecl;
     }
-  if (rname)
-    {
-      if (!tdecl)
+
+  if ((int) rid_index < (int) RID_MAX)
+    if (tree rname = ridpointers[(int) rid_index])
+      if (!decl || DECL_NAME (decl) != rname)
 	{
-	  tdecl = build_decl (BUILTINS_LOCATION, TYPE_DECL, rname, type);
-	  DECL_ARTIFICIAL (tdecl) = 1;
+	  tree rdecl = build_decl (BUILTINS_LOCATION, TYPE_DECL, rname, type);
+	  DECL_ARTIFICIAL (rdecl) = 1;
+	  SET_IDENTIFIER_GLOBAL_VALUE (rname, rdecl);
+	  if (!decl)
+	    decl = rdecl;
 	}
-      SET_IDENTIFIER_GLOBAL_VALUE (rname, tdecl);
-    }
-
-  if (!TYPE_NAME (type))
-    TYPE_NAME (type) = tdecl;
 
-  if (tdecl)
-    debug_hooks->type_decl (tdecl, 0);
+  if (decl)
+    {
+      if (!TYPE_NAME (type))
+	TYPE_NAME (type) = decl;
+      debug_hooks->type_decl (decl, 0);
+    }
 }
 
 /* Push a type into the namespace so that the back ends ignore it.  */
Index: name-lookup.c
===================================================================
--- name-lookup.c	(revision 253421)
+++ name-lookup.c	(working copy)
@@ -4847,6 +4847,7 @@  set_global_binding (tree name, tree val)
 {
   bool subtime = timevar_cond_start (TV_NAME_LOOKUP);
 
+  gcc_checking_assert (name == DECL_NAME (val));
   tree *slot = find_namespace_slot (global_namespace, name, true);
   tree old = MAYBE_STAT_DECL (*slot);