Re: [mainline] C++ PATCH: Tidy name hiding

[Gabriel Dos Reis <gdr at integrable-solutions dot net>]

Gabriel Dos Reis <gdr@integrable-solutions.net> writes:

| Mark Mitchell <mark@codesourcery.com> writes:
| 
| [...]
| 
| | > Also, since that function is actually hiding a declaration for a name,
| | > I renamed it to cxx_binding_hide_name.
| | 
| | I like everything in your patch, except the change of name.
| | 
| | The "hide_name" name makes me think that somehow the name is going to be
| | hidden, which it's not -- it's just that the type binding will require
| | an elaborated-type-specifier.
| | 
| | I actually think add_binding is OK, but supplement_binding might be
| | better?
| 
| Go for supplement_binding.  I'll resend an iteration I did sinice then,
| with your suggestion incorporated.

Here is the version I'm bootstrapping.

-- Gaby

Index: ChangeLog
===================================================================
RCS file: /cvs/gcc/gcc/gcc/cp/ChangeLog,v
retrieving revision 1.3652
diff -p -r1.3652 ChangeLog
*** ChangeLog	6 Sep 2003 16:14:29 -0000	1.3652
--- ChangeLog	6 Sep 2003 18:11:09 -0000
***************
*** 1,3 ****
--- 1,15 ----
+ 2003-09-07  Gabriel Dos Reis  <gdr@integrable-solutions.net>
+ 
+ 	* cp-tree.h (add_binding): Remove declaration.
+ 	* name-lookup.h (supplement_binding): Declare.
+ 	* decl.c (add_binding): Move to name-lookup.c.
+ 	(push_local_binding): Adjust.
+ 	(push_class_binding): Likewise.
+ 	(set_identifier_type_value_with_scope): Likewise.
+ 	* name-lookup.c (supplement_binding): Rename from add_binding.
+ 	Return a bool.  Improve documentation. 
+ 	(set_namespace_binding): Adjust.
+ 
  2003-09-06  Roger Sayle  <roger@eyesopen.com>
  
  	PR c++/11409
Index: cp-tree.h
===================================================================
RCS file: /cvs/gcc/gcc/gcc/cp/cp-tree.h,v
retrieving revision 1.913
diff -p -r1.913 cp-tree.h
*** cp-tree.h	5 Sep 2003 08:24:19 -0000	1.913
--- cp-tree.h	6 Sep 2003 18:11:12 -0000
*************** extern void register_dtor_fn            
*** 3754,3760 ****
  extern tmpl_spec_kind current_tmpl_spec_kind    (int);
  extern tree cp_fname_init			(const char *);
  extern tree check_elaborated_type_specifier     (enum tag_types, tree, bool);
- extern int add_binding                          (cxx_binding *, tree);
  extern bool have_extern_spec;
  
  /* in decl2.c */
--- 3754,3759 ----
Index: decl.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/cp/decl.c,v
retrieving revision 1.1122
diff -p -r1.1122 decl.c
*** decl.c	6 Sep 2003 15:44:33 -0000	1.1122
--- decl.c	6 Sep 2003 18:11:19 -0000
*************** push_binding (tree id, tree decl, cxx_sc
*** 911,992 ****
    IDENTIFIER_BINDING (id) = binding;
  }
  
- /* ID is already bound in the current scope.  But, DECL is an
-    additional binding for ID in the same scope.  This is the `struct
-    stat' hack whereby a non-typedef class-name or enum-name can be
-    bound at the same level as some other kind of entity.  It's the
-    responsibility of the caller to check that inserting this name is
-    valid here.  Returns nonzero if the new binding was successful.  */
- 
- int
- add_binding (cxx_binding *binding, tree decl)
- {
-   tree bval = BINDING_VALUE (binding);
-   int ok = 1;
- 
-   timevar_push (TV_NAME_LOOKUP);
-   if (TREE_CODE (decl) == TYPE_DECL && DECL_ARTIFICIAL (decl))
-     /* The new name is the type name.  */
-     BINDING_TYPE (binding) = decl;
-   else if (!bval)
-     /* This situation arises when push_class_level_binding moves an
-        inherited type-binding out of the way to make room for a new
-        value binding.  */
-     BINDING_VALUE (binding) = decl;
-   else if (TREE_CODE (bval) == TYPE_DECL && DECL_ARTIFICIAL (bval))
-     {
-       /* The old binding was a type name.  It was placed in
- 	 BINDING_VALUE because it was thought, at the point it was
- 	 declared, to be the only entity with such a name.  Move the
- 	 type name into the type slot; it is now hidden by the new
- 	 binding.  */
-       BINDING_TYPE (binding) = bval;
-       BINDING_VALUE (binding) = decl;
-       INHERITED_VALUE_BINDING_P (binding) = 0;
-     }
-   else if (TREE_CODE (bval) == TYPE_DECL
- 	   && TREE_CODE (decl) == TYPE_DECL
- 	   && DECL_NAME (decl) == DECL_NAME (bval)
- 	   && (same_type_p (TREE_TYPE (decl), TREE_TYPE (bval))
- 	       /* If either type involves template parameters, we must
- 		  wait until instantiation.  */
- 	       || uses_template_parms (TREE_TYPE (decl))
- 	       || uses_template_parms (TREE_TYPE (bval))))
-     /* We have two typedef-names, both naming the same type to have
-        the same name.  This is OK because of:
- 
-          [dcl.typedef]
- 
- 	 In a given scope, a typedef specifier can be used to redefine
- 	 the name of any type declared in that scope to refer to the
- 	 type to which it already refers.  */
-     ok = 0;
-   /* There can be two block-scope declarations of the same variable,
-      so long as they are `extern' declarations.  However, there cannot
-      be two declarations of the same static data member:
- 
-        [class.mem]
- 
-        A member shall not be declared twice in the
-        member-specification.  */
-   else if (TREE_CODE (decl) == VAR_DECL && TREE_CODE (bval) == VAR_DECL
- 	   && DECL_EXTERNAL (decl) && DECL_EXTERNAL (bval)
- 	   && !DECL_CLASS_SCOPE_P (decl))
-     {
-       duplicate_decls (decl, BINDING_VALUE (binding));
-       ok = 0;
-     }
-   else
-     {
-       error ("declaration of `%#D'", decl);
-       cp_error_at ("conflicts with previous declaration `%#D'",
- 		   BINDING_VALUE (binding));
-       ok = 0;
-     }
- 
-   POP_TIMEVAR_AND_RETURN (TV_NAME_LOOKUP, ok);
- }
- 
  /* Add DECL to the list of things declared in B.  */
  
  static void
--- 911,916 ----
*************** push_local_binding (tree id, tree decl, 
*** 1040,1046 ****
    if (lookup_name_current_level (id))
      {
        /* Supplement the existing binding.  */
!       if (!add_binding (IDENTIFIER_BINDING (id), decl))
  	/* It didn't work.  Something else must be bound at this
  	   level.  Do not add DECL to the list of things to pop
  	   later.  */
--- 964,970 ----
    if (lookup_name_current_level (id))
      {
        /* Supplement the existing binding.  */
!       if (!supplement_binding (IDENTIFIER_BINDING (id), decl))
  	/* It didn't work.  Something else must be bound at this
  	   level.  Do not add DECL to the list of things to pop
  	   later.  */
*************** push_class_binding (tree id, tree decl)
*** 1079,1085 ****
  
    if (binding && BINDING_SCOPE (binding) == class_binding_level)
      /* Supplement the existing binding.  */
!     result = add_binding (IDENTIFIER_BINDING (id), decl);
    else
      /* Create a new binding.  */
      push_binding (id, decl, class_binding_level);
--- 1003,1009 ----
  
    if (binding && BINDING_SCOPE (binding) == class_binding_level)
      /* Supplement the existing binding.  */
!     result = supplement_binding (IDENTIFIER_BINDING (id), decl);
    else
      /* Create a new binding.  */
      push_binding (id, decl, class_binding_level);
*************** set_identifier_type_value_with_scope (tr
*** 2336,2342 ****
        if (decl)
  	{
  	  if (BINDING_VALUE (binding))
! 	    add_binding (binding, decl);
  	  else
  	    BINDING_VALUE (binding) = decl;
  	}
--- 2260,2266 ----
        if (decl)
  	{
  	  if (BINDING_VALUE (binding))
! 	    supplement_binding (binding, decl);
  	  else
  	    BINDING_VALUE (binding) = decl;
  	}
Index: name-lookup.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/cp/name-lookup.c,v
retrieving revision 1.9
diff -p -r1.9 name-lookup.c
*** name-lookup.c	20 Aug 2003 07:06:42 -0000	1.9
--- name-lookup.c	6 Sep 2003 18:11:19 -0000
*************** cxx_binding_free (cxx_binding *binding)
*** 298,303 ****
--- 298,390 ----
    binding->previous = free_bindings;
    free_bindings = binding;
  }
+ 
+ /* BINDING records an existing declaration for a namein the current scope.
+    But, DECL is another declaration for that same identifier in the
+    same scope.  This is the `struct stat' hack whereby a non-typedef
+    class name or enum-name can be bound at the same level as some other
+    kind of entity.
+    3.3.7/1
+ 
+      A class name (9.1) or enumeration name (7.2) can be hidden by the
+      name of an object, function, or enumerator declared in the same scope.
+      If a class or enumeration name and an object, function, or enumerator
+      are declared in the same scope (in any order) with the same name, the
+      class or enumeration name is hidden wherever the object, function, or
+      enumerator name is visible.
+ 
+    It's the responsibility of the caller to check that
+    inserting this name is valid here.  Returns nonzero if the new binding
+    was successful.  */
+ 
+ bool
+ supplement_binding (cxx_binding *binding, tree decl)
+ {
+   tree bval = binding->value;
+   bool ok = true;
+ 
+   timevar_push (TV_NAME_LOOKUP);
+   if (TREE_CODE (decl) == TYPE_DECL && DECL_ARTIFICIAL (decl))
+     /* The new name is the type name.  */
+     binding->type = decl;
+   else if (!bval)
+     /* This situation arises when push_class_level_binding moves an
+        inherited type-binding out of the way to make room for a new
+        value binding.  */
+     binding->value = decl;
+   else if (TREE_CODE (bval) == TYPE_DECL && DECL_ARTIFICIAL (bval))
+     {
+       /* The old binding was a type name.  It was placed in
+ 	 BINDING_VALUE because it was thought, at the point it was
+ 	 declared, to be the only entity with such a name.  Move the
+ 	 type name into the type slot; it is now hidden by the new
+ 	 binding.  */
+       binding->type = bval;
+       binding->value = decl;
+       binding->value_is_inherited = false;
+     }
+   else if (TREE_CODE (bval) == TYPE_DECL
+ 	   && TREE_CODE (decl) == TYPE_DECL
+ 	   && DECL_NAME (decl) == DECL_NAME (bval)
+ 	   && (same_type_p (TREE_TYPE (decl), TREE_TYPE (bval))
+ 	       /* If either type involves template parameters, we must
+ 		  wait until instantiation.  */
+ 	       || uses_template_parms (TREE_TYPE (decl))
+ 	       || uses_template_parms (TREE_TYPE (bval))))
+     /* We have two typedef-names, both naming the same type to have
+        the same name.  This is OK because of:
+ 
+          [dcl.typedef]
+ 
+ 	 In a given scope, a typedef specifier can be used to redefine
+ 	 the name of any type declared in that scope to refer to the
+ 	 type to which it already refers.  */
+     ok = false;
+   /* There can be two block-scope declarations of the same variable,
+      so long as they are `extern' declarations.  However, there cannot
+      be two declarations of the same static data member:
+ 
+        [class.mem]
+ 
+        A member shall not be declared twice in the
+        member-specification.  */
+   else if (TREE_CODE (decl) == VAR_DECL && TREE_CODE (bval) == VAR_DECL
+ 	   && DECL_EXTERNAL (decl) && DECL_EXTERNAL (bval)
+ 	   && !DECL_CLASS_SCOPE_P (decl))
+     {
+       duplicate_decls (decl, binding->value);
+       ok = false;
+     }
+   else
+     {
+       error ("declaration of `%#D'", decl);
+       cp_error_at ("conflicts with previous declaration `%#D'",
+ 		   binding->value);
+       ok = false;
+     }
+ 
+   POP_TIMEVAR_AND_RETURN (TV_NAME_LOOKUP, ok);
+ }
  
  /* Return (from the stack of) the BINDING, if any, establihsed at SCOPE.  */ 
  
*************** set_namespace_binding (tree name, tree s
*** 382,388 ****
        || val == error_mark_node)
      BINDING_VALUE (b) = val;
    else
!     add_binding (b, val);
    timevar_pop (TV_NAME_LOOKUP);
  }
  
--- 469,475 ----
        || val == error_mark_node)
      BINDING_VALUE (b) = val;
    else
!     supplement_binding (b, val);
    timevar_pop (TV_NAME_LOOKUP);
  }
  
Index: name-lookup.h
===================================================================
RCS file: /cvs/gcc/gcc/gcc/cp/name-lookup.h,v
retrieving revision 1.4
diff -p -r1.4 name-lookup.h
*** name-lookup.h	29 May 2003 11:04:10 -0000	1.4
--- name-lookup.h	6 Sep 2003 18:11:19 -0000
*************** struct cxx_binding GTY(())
*** 107,112 ****
--- 107,113 ----
  
  extern cxx_binding *cxx_binding_make (tree, tree);
  extern void cxx_binding_free (cxx_binding *);
+ extern bool supplement_binding (cxx_binding *, tree);
  
  /* True if SCOPE designates the global scope binding contour.  */
  #define global_scope_p(SCOPE) \