Expunge namespace-scope IDENTIFIER_TYPE_VALUE & global_type_name [PR 99039]
authorNathan Sidwell <nathan@acm.org>
Thu, 11 Feb 2021 21:06:03 +0000 (13:06 -0800)
committerNathan Sidwell <nathan@acm.org>
Fri, 12 Feb 2021 21:50:03 +0000 (13:50 -0800)
IDENTIFIER_TYPE_VALUE and friends is a remnant of G++'s C origins.  It
holds elaborated types on identifier-nodes.  While this is fine for C
and for local and class-scopes in C++, it fails badly for namespaces.
In that case a marker 'global_type_node' was used, which essentially
signified 'this is a namespace-scope type *somewhere*', and you'd have
to do a regular name_lookup to find it.  As the parser and
substitution machinery has avanced over the last 25 years or so,
there's not much outside of actual name-lookup that uses that.
Amusingly the IDENTIFIER_HAS_TYPE_VALUE predicate will do an actual
name-lookup and then users would repeat that lookup to find the
now-known to be there type.

Rather late I realized that this interferes with the lazy loading of
module entities, because we were setting IDENTIFIER_TYPE_VALUE to
global_type_node.  But we could be inside some local scope where that
identifier is bound to some local type.  Not good!

Rather than add more cruft to look at an identifier's shadow stack and
alter that as necessary, this takes the approach of removing the
existing cruft.

We nuke the few places outside of name lookup that use
IDENTIFIER_TYPE_VALUE.  Replacing them with either proper name
lookups, alternative sequences, or in some cases asserting that they
(no longer) happen.  Class template instantiation was calling pushtag
after setting IDENTIFIER_TYPE_VALUE in order to stop pushtag creating
an implicit typedef and pushing it, but to get the bookkeeping it
needed.  Let's just do the bookkeeping directly.

Then we can stop having a 'bound at namespace-scope' marker at all,
which means lazy loading won't screw up local shadow stacks.  Also, it
simplifies set_identifier_type_value_with_scope, as it never needs to
inspect the scope stack.  When developing this patch, I discovered a
number of places we'd put an actual namespace-scope type on the
type_value slot, rather than global_type_node.  You might notice this
is killing at least two 'why are we doing this?' comments.

While this doesn't fix the two PRs mentioned, it is a necessary step.

PR c++/99039
PR c++/99040
gcc/cp/
* cp-tree.h (CPTI_GLOBAL_TYPE): Delete.
(global_type_node): Delete.
(IDENTIFIER_TYPE_VALUE): Delete.
(IDENTIFIER_HAS_TYPE_VALUE): Delete.
(get_type_value): Delete.
* name-lookup.h (identifier_type_value): Delete.
* name-lookup.c (check_module_override): Don't
SET_IDENTIFIER_TYPE_VALUE here.
(do_pushdecl): Nor here.
(identifier_type_value_1, identifier_type_value): Delete.
(set_identifier_type_value_with_scope): Only
SET_IDENTIFIER_TYPE_VALUE for local and class scopes.
(pushdecl_nanmespace_level): Remove shadow stack nadgering.
(do_pushtag): Use REAL_IDENTIFIER_TYPE_VALUE.
* call.c (check_dtor_name): Use lookup_name.
* decl.c (cxx_init_decl_processing): Drop global_type_node.
* decl2.c (cplus_decl_attributes): Don't SET_IDENTIFIER_TYPE_VALUE
here.
* init.c (get_type_value): Delete.
* pt.c (instantiate_class_template_1): Don't call pushtag or
SET_IDENTIFIER_TYPE_VALUE here.
(tsubst): Assert never an identifier.
(dependent_type_p): Drop global_type_node assert.
* typeck.c (error_args_num): Don't use IDENTIFIER_HAS_TYPE_VALUE
to determine ctorness.
gcc/testsuite/
* g++.dg/lookup/pr99039.C: New.

gcc/cp/call.c
gcc/cp/cp-tree.h
gcc/cp/decl.c
gcc/cp/decl2.c
gcc/cp/init.c
gcc/cp/name-lookup.c
gcc/cp/name-lookup.h
gcc/cp/pt.c
gcc/cp/typeck.c
gcc/testsuite/g++.dg/lookup/pr99039.C [new file with mode: 0644]

index 4744c9768ecc9d4329c72ec077404d42dccd4f89..186feef6fe33e0128ad89502d7e7cf9c39927d7f 100644 (file)
@@ -236,8 +236,15 @@ check_dtor_name (tree basetype, tree name)
           || TREE_CODE (basetype) == ENUMERAL_TYPE)
          && name == constructor_name (basetype))
        return true;
-      else
-       name = get_type_value (name);
+
+      /* Otherwise lookup the name, it could be an unrelated typedef
+        of the correct type.  */
+      name = lookup_name (name, LOOK_want::TYPE);
+      if (!name)
+       return false;
+      name = TREE_TYPE (name);
+      if (name == error_mark_node)
+       return false;
     }
   else
     {
@@ -252,8 +259,6 @@ check_dtor_name (tree basetype, tree name)
       return false;
     }
 
-  if (!name || name == error_mark_node)
-    return false;
   return same_type_p (TYPE_MAIN_VARIANT (basetype), TYPE_MAIN_VARIANT (name));
 }
 
index 4ed3936ade27435f02a41958476b4524d8fbaeec..38b31e3908f0ab5d77d2147e73e5c193334404b7 100644 (file)
@@ -129,7 +129,6 @@ enum cp_tree_index
     CPTI_VTBL_TYPE,
     CPTI_VTBL_PTR_TYPE,
     CPTI_GLOBAL,
-    CPTI_GLOBAL_TYPE,
     CPTI_ABORT_FNDECL,
     CPTI_AGGR_TAG,
     CPTI_CONV_OP_MARKER,
@@ -250,7 +249,6 @@ extern GTY(()) tree cp_global_trees[CPTI_MAX];
 #define std_node                       cp_global_trees[CPTI_STD]
 #define abi_node                       cp_global_trees[CPTI_ABI]
 #define global_namespace               cp_global_trees[CPTI_GLOBAL]
-#define global_type_node               cp_global_trees[CPTI_GLOBAL_TYPE]
 #define const_type_info_type_node      cp_global_trees[CPTI_CONST_TYPE_INFO_TYPE]
 #define type_info_ptr_type             cp_global_trees[CPTI_TYPE_INFO_PTR_TYPE]
 #define conv_op_marker                 cp_global_trees[CPTI_CONV_OP_MARKER]
@@ -1149,24 +1147,16 @@ enum GTY(()) abstract_class_use {
 
 /* Macros for access to language-specific slots in an identifier.  */
 
-/* The IDENTIFIER_BINDING is the innermost cxx_binding for the
-    identifier.  Its PREVIOUS is the next outermost binding.  Each
-    VALUE field is a DECL for the associated declaration.  Thus,
-    name lookup consists simply of pulling off the node at the front
-    of the list (modulo oddities for looking up the names of types,
-    and such.)  You can use SCOPE field to determine the scope
-    that bound the name.  */
+/* Identifiers map directly to block or class-scope bindings.
+   Namespace-scope bindings are held in hash tables on the respective
+   namespaces.  The identifier bindings are the innermost active
+   binding, from whence you can get the decl and/or implicit-typedef
+   of an elaborated type.   When not bound to a local entity the
+   values are NULL.  */
 #define IDENTIFIER_BINDING(NODE) \
   (LANG_IDENTIFIER_CAST (NODE)->bindings)
-
-/* TREE_TYPE only indicates on local and class scope the current
-   type. For namespace scope, the presence of a type in any namespace
-   is indicated with global_type_node, and the real type behind must
-   be found through lookup.  */
-#define IDENTIFIER_TYPE_VALUE(NODE) identifier_type_value (NODE)
 #define REAL_IDENTIFIER_TYPE_VALUE(NODE) TREE_TYPE (NODE)
 #define SET_IDENTIFIER_TYPE_VALUE(NODE,TYPE) (TREE_TYPE (NODE) = (TYPE))
-#define IDENTIFIER_HAS_TYPE_VALUE(NODE) (IDENTIFIER_TYPE_VALUE (NODE) ? 1 : 0)
 
 /* Kinds of identifiers.  Values are carefully chosen.  */
 enum cp_identifier_kind {
@@ -6862,7 +6852,6 @@ extern void emit_mem_initializers         (tree);
 extern tree build_aggr_init                    (tree, tree, int,
                                                  tsubst_flags_t);
 extern int is_class_type                       (tree, int);
-extern tree get_type_value                     (tree);
 extern tree build_zero_init                    (tree, tree, bool);
 extern tree build_value_init                   (tree, tsubst_flags_t);
 extern tree build_value_init_noctor            (tree, tsubst_flags_t);
index 0aa49f91d5a118498c80920d080f686f9bac29dd..6f3414f058eb8b6a28fdb8a0134332de975bc2b0 100644 (file)
@@ -4510,9 +4510,6 @@ cxx_init_decl_processing (void)
   abi_node = current_namespace;
   pop_namespace ();
 
-  global_type_node = make_node (LANG_TYPE);
-  record_unknown_type (global_type_node, "global type");
-
   any_targ_node = make_node (LANG_TYPE);
   record_unknown_type (any_targ_node, "any type");
 
index b087753cfba51ae89fa3a292545b005eafd054b6..c46100de89a69959fd05b1b0f9e3fc4c7e959874 100644 (file)
@@ -1596,9 +1596,6 @@ cplus_decl_attributes (tree *decl, tree attributes, int flags)
       decl_attributes (decl, attributes, flags, last_decl);
     }
 
-  if (TREE_CODE (*decl) == TYPE_DECL)
-    SET_IDENTIFIER_TYPE_VALUE (DECL_NAME (*decl), TREE_TYPE (*decl));
-
   /* Propagate deprecation out to the template.  */
   if (TREE_DEPRECATED (*decl))
     if (tree ti = get_template_info (*decl))
index 1381a23449e8f4ed71da7668a0f5d18901c76086..49950d405216ab2ae986d6443e108eea90275a6f 100644 (file)
@@ -2118,18 +2118,6 @@ is_class_type (tree type, int or_else)
   return 1;
 }
 
-tree
-get_type_value (tree name)
-{
-  if (name == error_mark_node)
-    return NULL_TREE;
-
-  if (IDENTIFIER_HAS_TYPE_VALUE (name))
-    return IDENTIFIER_TYPE_VALUE (name);
-  else
-    return NULL_TREE;
-}
-
 /* Build a reference to a member of an aggregate.  This is not a C++
    `&', but really something which can have its address taken, and
    then act as a pointer to member, for example TYPE :: FIELD can have
index 1cd4f67b8b217c688d31788377e4c02b4fe16eb1..8aa490db6340aab05ae679fff0bad0f1c9c8543c 100644 (file)
@@ -3620,14 +3620,7 @@ check_module_override (tree decl, tree mvec, bool hiding,
          if (iter.using_p ())
            ;
          else if (tree match = duplicate_decls (decl, *iter, hiding))
-           {
-             if (TREE_CODE (match) == TYPE_DECL)
-               /* The IDENTIFIER will have the type referring to the
-                  now-smashed TYPE_DECL, because ...?  Reset it.  */
-               SET_IDENTIFIER_TYPE_VALUE (name, TREE_TYPE (match));
-
-             return match;
-           }
+           return match;
       }
 
   if (TREE_PUBLIC (scope) && TREE_PUBLIC (decl) && !not_module_p ()
@@ -3649,11 +3642,7 @@ check_module_override (tree decl, tree mvec, bool hiding,
          tree match = *iter;
          
          if (duplicate_decls (decl, match, hiding))
-           {
-             if (TREE_CODE (match) == TYPE_DECL)
-               SET_IDENTIFIER_TYPE_VALUE (name, TREE_TYPE (match));
-             return match;
-           }
+           return match;
        }
     }
 
@@ -3736,9 +3725,14 @@ do_pushdecl (tree decl, bool hiding)
            if (match == error_mark_node)
              ;
            else if (TREE_CODE (match) == TYPE_DECL)
-             /* The IDENTIFIER will have the type referring to the
-                now-smashed TYPE_DECL, because ...?  Reset it.  */
-             SET_IDENTIFIER_TYPE_VALUE (name, TREE_TYPE (match));
+             {
+               auto *l = level;
+               while (l->kind == sk_template_parms)
+                 l = l->level_chain;
+               gcc_checking_assert (REAL_IDENTIFIER_TYPE_VALUE (name)
+                                    == (l->kind == sk_namespace
+                                        ? NULL_TREE : TREE_TYPE (match)));
+             }
            else if (iter.hidden_p () && !hiding)
              {
                /* Unhiding a previously hidden decl.  */
@@ -4748,37 +4742,6 @@ print_binding_stack (void)
   print_binding_level (NAMESPACE_LEVEL (global_namespace));
 }
 \f
-/* Return the type associated with ID.  */
-
-static tree
-identifier_type_value_1 (tree id)
-{
-  /* There is no type with that name, anywhere.  */
-  if (REAL_IDENTIFIER_TYPE_VALUE (id) == NULL_TREE)
-    return NULL_TREE;
-  /* This is not the type marker, but the real thing.  */
-  if (REAL_IDENTIFIER_TYPE_VALUE (id) != global_type_node)
-    return REAL_IDENTIFIER_TYPE_VALUE (id);
-  /* Have to search for it. It must be on the global level, now.
-     Ask lookup_name not to return non-types.  */
-  id = lookup_name (id, LOOK_where::BLOCK_NAMESPACE, LOOK_want::TYPE);
-  if (id)
-    return TREE_TYPE (id);
-  return NULL_TREE;
-}
-
-/* Wrapper for identifier_type_value_1.  */
-
-tree
-identifier_type_value (tree id)
-{
-  tree ret;
-  timevar_start (TV_NAME_LOOKUP);
-  ret = identifier_type_value_1 (id);
-  timevar_stop (TV_NAME_LOOKUP);
-  return ret;
-}
-
 /* Push a definition of struct, union or enum tag named ID.  into
    binding_level B.  DECL is a TYPE_DECL for the type.  DECL has
    already been pushed into its binding level.  This is bookkeeping to
@@ -4787,26 +4750,21 @@ identifier_type_value (tree id)
 static void
 set_identifier_type_value_with_scope (tree id, tree decl, cp_binding_level *b)
 {
-  tree type;
+  while (b->kind == sk_template_parms)
+    b = b->level_chain;
 
-  if (b->kind != sk_namespace)
-    {
-      /* Shadow the marker, not the real thing, so that the marker
-        gets restored later.  */
-      tree old_type_value = REAL_IDENTIFIER_TYPE_VALUE (id);
-      b->type_shadowed = tree_cons (id, old_type_value, b->type_shadowed);
-      type = decl ? TREE_TYPE (decl) : NULL_TREE;
-      TREE_TYPE (b->type_shadowed) = type;
-    }
+  if (b->kind == sk_namespace)
+    /* At namespace scope we should not see an identifier type value.  */
+    gcc_checking_assert (!REAL_IDENTIFIER_TYPE_VALUE (id));
   else
     {
-      gcc_assert (decl);
-
-      /* Store marker instead of real type.  */
-      type = global_type_node;
+      /* Push the current type value, so we can restore it later  */
+      tree old = REAL_IDENTIFIER_TYPE_VALUE (id);
+      b->type_shadowed = tree_cons (id, old, b->type_shadowed);
+      tree type = decl ? TREE_TYPE (decl) : NULL_TREE;
+      TREE_TYPE (b->type_shadowed) = type;
+      SET_IDENTIFIER_TYPE_VALUE (id, type);
     }
-
-  SET_IDENTIFIER_TYPE_VALUE (id, type);
 }
 
 /* As set_identifier_type_value_with_scope, but using
@@ -6238,51 +6196,17 @@ do_namespace_alias (tree alias, tree name_space)
     (*debug_hooks->early_global_decl) (alias);
 }
 
-/* Like pushdecl, only it places X in the current namespace,
+/* Like pushdecl, only it places DECL in the current namespace,
    if appropriate.  */
 
 tree
-pushdecl_namespace_level (tree x, bool hiding)
+pushdecl_namespace_level (tree decl, bool hiding)
 {
-  cp_binding_level *b = current_binding_level;
-  tree t;
-
   bool subtime = timevar_cond_start (TV_NAME_LOOKUP);
-  t = do_pushdecl_with_scope (x, NAMESPACE_LEVEL (current_namespace), hiding);
-
-  /* Now, the type_shadowed stack may screw us.  Munge it so it does
-     what we want.  */
-  if (TREE_CODE (t) == TYPE_DECL)
-    {
-      tree name = DECL_NAME (t);
-      tree newval;
-      tree *ptr = (tree *)0;
-      for (; !global_scope_p (b); b = b->level_chain)
-       {
-         tree shadowed = b->type_shadowed;
-         for (; shadowed; shadowed = TREE_CHAIN (shadowed))
-           if (TREE_PURPOSE (shadowed) == name)
-             {
-               ptr = &TREE_VALUE (shadowed);
-               /* Can't break out of the loop here because sometimes
-                  a binding level will have duplicate bindings for
-                  PT names.  It's gross, but I haven't time to fix it.  */
-             }
-       }
-      newval = TREE_TYPE (t);
-      if (ptr == (tree *)0)
-       {
-         /* @@ This shouldn't be needed.  My test case "zstring.cc" trips
-            up here if this is changed to an assertion.  --KR  */
-         SET_IDENTIFIER_TYPE_VALUE (name, t);
-       }
-      else
-       {
-         *ptr = newval;
-       }
-    }
+  tree res = do_pushdecl_with_scope (decl, NAMESPACE_LEVEL (current_namespace),
+                                    hiding);
   timevar_cond_stop (TV_NAME_LOOKUP, subtime);
-  return t;
+  return res;
 }
 
 /* Wrapper around push_local_binding to push the bindings for
@@ -8257,6 +8181,8 @@ do_pushtag (tree name, tree type, TAG_how how)
 {
   tree decl;
 
+  gcc_assert (identifier_p (name));
+
   cp_binding_level *b = current_binding_level;
   while (true)
     {
@@ -8282,10 +8208,8 @@ do_pushtag (tree name, tree type, TAG_how how)
        break;
     }
 
-  gcc_assert (identifier_p (name));
-
   /* Do C++ gratuitous typedefing.  */
-  if (identifier_type_value_1 (name) != type)
+  if (REAL_IDENTIFIER_TYPE_VALUE (name) != type)
     {
       tree tdef;
       tree context = TYPE_CONTEXT (type);
index 75db5b38061d94d0e5b6853fa57ceb595279291c..e159942eda44bbd29c9c674d62ca1f15a294e8e7 100644 (file)
@@ -188,7 +188,6 @@ struct GTY(()) tree_binding_vec {
 #define BINDING_VECTOR_PENDING_IS_PARTITION_P(NODE) \
   (BINDING_VECTOR_CHECK (NODE)->base.private_flag)
 
-extern tree identifier_type_value (tree);
 extern void set_identifier_type_value (tree, tree);
 extern void push_binding (tree, tree, cp_binding_level*);
 extern void pop_local_binding (tree, tree);
index 50cdb00310f67b618cbdc27bac3f03f229282e12..60de8e93ff73a6338d39664c2359673b569b5e72 100644 (file)
@@ -11971,14 +11971,13 @@ instantiate_class_template_1 (tree type)
                       instantiation, but the TYPE is not.) */
                    CLASSTYPE_USE_TEMPLATE (newtag) = 0;
 
-                 /* Now, we call pushtag to put this NEWTAG into the scope of
-                    TYPE.  We first set up the IDENTIFIER_TYPE_VALUE to avoid
-                    pushtag calling push_template_decl.  We don't have to do
-                    this for enums because it will already have been done in
-                    tsubst_enum.  */
-                 if (name)
-                   SET_IDENTIFIER_TYPE_VALUE (name, newtag);
-                 pushtag (name, newtag);
+                 /* Now, install the tag.  We don't use pushtag
+                    because that does too much work -- creating an
+                    implicit typedef, which we've already done.  */
+                 set_identifier_type_value (name, TYPE_NAME (newtag));
+                 maybe_add_class_template_decl_list (type, newtag, false);
+                 TREE_PUBLIC (TYPE_NAME (newtag)) = true;
+                 determine_visibility (TYPE_NAME (newtag));
                }
            }
          else if (DECL_DECLARES_FUNCTION_P (t))
@@ -15424,10 +15423,8 @@ tsubst (tree t, tree args, tsubst_flags_t complain, tree in_decl)
 
   code = TREE_CODE (t);
 
-  if (code == IDENTIFIER_NODE)
-    type = IDENTIFIER_TYPE_VALUE (t);
-  else
-    type = TREE_TYPE (t);
+  gcc_assert (code != IDENTIFIER_NODE);
+  type = TREE_TYPE (t);
 
   gcc_assert (type != unknown_type_node);
 
@@ -26774,10 +26771,6 @@ dependent_type_p (tree type)
   if (type == error_mark_node)
     return false;
 
-  /* Getting here with global_type_node means we improperly called this
-     function on the TREE_TYPE of an IDENTIFIER_NODE.  */
-  gcc_checking_assert (type != global_type_node);
-
   /* If we have not already computed the appropriate value for TYPE,
      do so now.  */
   if (!TYPE_DEPENDENT_P_VALID (type))
index a87d5e5f2ac9bea9ee4dfcbc70c9e3840bf40a5d..d06d7440d5cd1fd66040abf49a9b37345c96bc3c 100644 (file)
@@ -4060,7 +4060,8 @@ error_args_num (location_t loc, tree fndecl, bool too_many_p)
       if (TREE_CODE (TREE_TYPE (fndecl)) == METHOD_TYPE)
        {
          if (DECL_NAME (fndecl) == NULL_TREE
-             || IDENTIFIER_HAS_TYPE_VALUE (DECL_NAME (fndecl)))
+             || (DECL_NAME (fndecl)
+                 == DECL_NAME (TYPE_NAME (DECL_CONTEXT (fndecl)))))
            error_at (loc,
                      too_many_p
                      ? G_("too many arguments to constructor %q#D")
diff --git a/gcc/testsuite/g++.dg/lookup/pr99039.C b/gcc/testsuite/g++.dg/lookup/pr99039.C
new file mode 100644 (file)
index 0000000..982a991
--- /dev/null
@@ -0,0 +1,51 @@
+// PR 99039, we need to remove the namespace-scope meaning of
+// IDENTIFIER_TYPE_VALUE.
+
+namespace std
+{
+typedef long unsigned int size_t;
+
+template<typename _CharT>
+struct char_traits
+{
+  typedef _CharT char_type;
+
+  template<typename U>
+  static int
+    compare(const char_type* __s1, const char_type* __s2, std::size_t __n);
+};
+
+template<typename _CharT>
+template<typename U>
+int
+char_traits<_CharT>::
+compare(const char_type* __s1, const char_type* __s2, std::size_t __n)
+{
+  return 0;
+}
+
+}
+
+struct CHAR_TRAITS;
+namespace std
+{
+typedef long unsigned int size_t;
+
+struct CHAR_TRAITS
+{
+  typedef char char_type;
+
+  static int
+    compare(const char_type* __s1, const char_type* __s2, std::size_t __n);
+};
+
+int
+CHAR_TRAITS::
+compare(const char_type* __s1, const char_type* __s2, std::size_t __n)
+{
+  return 0;
+}
+
+}
+
+struct CHAR_TRAITS;