This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH] PR c++/26693


Hello,

Please find attached a reworked version patch that takes your feedback in account. Below are some in-line comments to your precedent email.

Jason Merrill a Ãcrit :
I think putting the code to create typedef variants for class scope typedefs belongs in grokfield rather than grokdeclarator.

Done.


This patch should also update the code in pushdecl_maybe_friend and save_template_attributes that creates typedef variants;

Done.


the code in save_template_attributes may not be necessary anymore, and if it still is I'd like to know why.

Yes, it's not needed anymore.


It also seems that we need clone_underlying_type to propagate TYPE_STUB_DECL.

Done.


These changes did uncover some additional issues though.
Unlike previously, every class scope typedef is now associated with a typedef variant type. This creates some issues when the typedef involves typename constructs like: 'typedef typename T::foo bar;'.


The issues are twofold:

1/ If t is a typedef variant type, then TYPE_NAME (t) contains the TYPE_DECL representing the declaration of the typedef. But at the same time, if t2 is the TYPENAME_TYPE type of 'typename T::foo', then TYPE_NAME (t2) will contain the TYPE_DECL representing the declaration of 'foo'. So there is a clash regarding the use of TYPE_NAME(t) by typedefs and typename constructs. This clash can become pathological when we have a construct like 'typedef typename T::foo bar;' that involves a typedef and a typename.
So the patch did modify resolve_typename_type() to make it be aware of the issue.


2/ When tsubst() encounters a typedef variant type, it tries to "reuse" its specialization, instead of tsubst-ing its main variant type. This works great. But when the main variant type is a TYPENAME_TYPE, I think we need to make sure the main variant type can be properly tsubst-ed before attempting to reuse the specialization of the typedef variant type. Otherwise, there are error cases that are not caught and as a result g++ would compile things that it should not compile. E.g, g++ would fail to report the error on line 14 in gcc/testsuite/g++.dg/template/sfinae3.C.
To address this, the patch makes sure the main variant type can be tsubst-ed before re-using the typedef variant.


This patch passes regtests and compiles libstdc++ on trunk.

Thanks,

Dodji.
gcc/ChangeLog:
2008-10-30  Dodji Seketeli  <dodji@redhat.com>

	PR c++/26693
	* c-decl.c: (clone_underlying_type): Move this  ...
	* tree.c: ... here so that it can be re-used by other front ends.
	Also, make sure the function  properly sets TYPE_STUB_DECL() on
	the newly created typedef variant type.

gcc/cp/ChangeLog/
2008-10-29  Dodji Seketeli  <dodji@redhat.com>

	PR c++/26693
	* decl2.c (grokfield): when a typedef appears in a
	class, make sure to create the typedef variant type node
	for it. The creation of the typedef variant is done by calling
	the clone_underlying_type function.
	* name-lookup.c (pushdecl_maybe_friend): Reuse the
	clone_underlying_type function to install typedef variant types.
	* pt.c (tsubst): When faced with the typedef variant type of a
	TYPENAME_TYPE, do not re-use the template specialization of
	the typedef variant type if substituting the corresponding main type variant
	fails.
	If we are accessing a type that is a typedef name defined in a class,
	make sure the typedef name is accessible for it.

gcc/testsuite/ChangeLog:
2008-10-29  Dodji Seketeli  <dodji@redhat.com>

	PR c++/26693
	* g++.dg/template/typedef11.C: New test.

libstdc++-v3/ChangeLog:
2008-10-29  Dodji Seketeli  <dodji@redhat.com>

	* include/ext/bitmap_allocator.h: the typedefs should be made public
	if we want them to be accessible. This has been revealed by the patch
	that fixes PR c++/26693 in g++.

diff --git a/gcc/c-decl.c b/gcc/c-decl.c
index eba6161..2226928 100644
--- a/gcc/c-decl.c
+++ b/gcc/c-decl.c
@@ -1971,67 +1971,6 @@ warn_if_shadowing (tree new_decl)
       }
 }
 
-
-/* Subroutine of pushdecl.
-
-   X is a TYPE_DECL for a typedef statement.  Create a brand new
-   ..._TYPE node (which will be just a variant of the existing
-   ..._TYPE node with identical properties) and then install X
-   as the TYPE_NAME of this brand new (duplicate) ..._TYPE node.
-
-   The whole point here is to end up with a situation where each
-   and every ..._TYPE node the compiler creates will be uniquely
-   associated with AT MOST one node representing a typedef name.
-   This way, even though the compiler substitutes corresponding
-   ..._TYPE nodes for TYPE_DECL (i.e. "typedef name") nodes very
-   early on, later parts of the compiler can always do the reverse
-   translation and get back the corresponding typedef name.  For
-   example, given:
-
-	typedef struct S MY_TYPE;
-	MY_TYPE object;
-
-   Later parts of the compiler might only know that `object' was of
-   type `struct S' if it were not for code just below.  With this
-   code however, later parts of the compiler see something like:
-
-	struct S' == struct S
-	typedef struct S' MY_TYPE;
-	struct S' object;
-
-    And they can then deduce (from the node for type struct S') that
-    the original object declaration was:
-
-		MY_TYPE object;
-
-    Being able to do this is important for proper support of protoize,
-    and also for generating precise symbolic debugging information
-    which takes full account of the programmer's (typedef) vocabulary.
-
-    Obviously, we don't want to generate a duplicate ..._TYPE node if
-    the TYPE_DECL node that we are now processing really represents a
-    standard built-in type.  */
-
-static void
-clone_underlying_type (tree x)
-{
-  if (DECL_IS_BUILTIN (x))
-    {
-      if (TYPE_NAME (TREE_TYPE (x)) == 0)
-	TYPE_NAME (TREE_TYPE (x)) = x;
-    }
-  else if (TREE_TYPE (x) != error_mark_node
-	   && DECL_ORIGINAL_TYPE (x) == NULL_TREE)
-    {
-      tree tt = TREE_TYPE (x);
-      DECL_ORIGINAL_TYPE (x) = tt;
-      tt = build_variant_type_copy (tt);
-      TYPE_NAME (tt) = x;
-      TREE_USED (tt) = TREE_USED (x);
-      TREE_TYPE (x) = tt;
-    }
-}
-
 /* Record a decl-node X as belonging to the current lexical scope.
    Check for errors (such as an incompatible declaration for the same
    name already seen in the same scope).
diff --git a/gcc/cp/decl.c b/gcc/cp/decl.c
index b0b03a5..8ddae96 100644
--- a/gcc/cp/decl.c
+++ b/gcc/cp/decl.c
@@ -8680,6 +8680,7 @@ grokdeclarator (const cp_declarator *declarator,
 	decl = build_lang_decl (TYPE_DECL, unqualified_id, type);
       else
 	decl = build_decl (TYPE_DECL, unqualified_id, type);
+
       if (id_declarator && declarator->u.id.qualifying_scope) {
 	error ("%Jtypedef name may not be a nested-name-specifier", decl);
 	TREE_TYPE (decl) = error_mark_node;
@@ -8714,12 +8715,11 @@ grokdeclarator (const cp_declarator *declarator,
 	  && TYPE_ANONYMOUS_P (type)
 	  && cp_type_quals (type) == TYPE_UNQUALIFIED)
 	{
-	  tree oldname = TYPE_NAME (type);
 	  tree t;
 
 	  /* Replace the anonymous name with the real name everywhere.  */
 	  for (t = TYPE_MAIN_VARIANT (type); t; t = TYPE_NEXT_VARIANT (t))
-	    if (TYPE_NAME (t) == oldname)
+	    if (ANON_AGGRNAME_P (TYPE_IDENTIFIER (t)))
 	      TYPE_NAME (t) = decl;
 
 	  if (TYPE_LANG_SPECIFIC (type))
diff --git a/gcc/cp/decl2.c b/gcc/cp/decl2.c
index b326752..17b3460 100644
--- a/gcc/cp/decl2.c
+++ b/gcc/cp/decl2.c
@@ -799,6 +799,9 @@ grokfield (const cp_declarator *declarator,
       DECL_NONLOCAL (value) = 1;
       DECL_CONTEXT (value) = current_class_type;
 
+      if (declspecs->specs[(int)ds_typedef])
+	clone_underlying_type (value);
+
       if (processing_template_decl)
 	value = push_template_decl (value);
 
@@ -1110,19 +1113,6 @@ save_template_attributes (tree *attr_p, tree *decl_p)
   if (!late_attrs)
     return;
 
-  /* Give this type a name so we know to look it up again at instantiation
-     time.  */
-  if (TREE_CODE (*decl_p) == TYPE_DECL
-      && DECL_ORIGINAL_TYPE (*decl_p) == NULL_TREE)
-    {
-      tree oldt = TREE_TYPE (*decl_p);
-      tree newt = build_variant_type_copy (oldt);
-      DECL_ORIGINAL_TYPE (*decl_p) = oldt;
-      TREE_TYPE (*decl_p) = newt;
-      TYPE_NAME (newt) = *decl_p;
-      TREE_USED (newt) = TREE_USED (*decl_p);
-    }
-
   if (DECL_P (*decl_p))
     q = &DECL_ATTRIBUTES (*decl_p);
   else
diff --git a/gcc/cp/name-lookup.c b/gcc/cp/name-lookup.c
index 0da373c..65b9dcd 100644
--- a/gcc/cp/name-lookup.c
+++ b/gcc/cp/name-lookup.c
@@ -847,28 +847,20 @@ pushdecl_maybe_friend (tree x, bool is_friend)
 
       /* If declaring a type as a typedef, copy the type (unless we're
 	 at line 0), and install this TYPE_DECL as the new type's typedef
-	 name.  See the extensive comment in ../c-decl.c (pushdecl).  */
+	 name.  See the extensive comment of clone_underlying_type ().  */
       if (TREE_CODE (x) == TYPE_DECL)
 	{
 	  tree type = TREE_TYPE (x);
-	  if (DECL_IS_BUILTIN (x))
-	    {
-	      if (TYPE_NAME (type) == 0)
-		TYPE_NAME (type) = x;
-	    }
-	  else if (type != error_mark_node && TYPE_NAME (type) != x
-		   /* We don't want to copy the type when all we're
-		      doing is making a TYPE_DECL for the purposes of
-		      inlining.  */
-		   && (!TYPE_NAME (type)
-		       || TYPE_NAME (type) != DECL_ABSTRACT_ORIGIN (x)))
-	    {
-	      DECL_ORIGINAL_TYPE (x) = type;
-	      type = build_variant_type_copy (type);
-	      TYPE_STUB_DECL (type) = TYPE_STUB_DECL (DECL_ORIGINAL_TYPE (x));
-	      TYPE_NAME (type) = x;
-	      TREE_TYPE (x) = type;
-	    }
+
+	  if (DECL_IS_BUILTIN (x)
+	      || (TREE_TYPE (x) != error_mark_node
+		  && TYPE_NAME (type) != x
+		  /* We don't want to copy the type when all we're
+		     doing is making a TYPE_DECL for the purposes of
+		     inlining.  */
+		  && (!TYPE_NAME (type)
+		      || TYPE_NAME (type) != DECL_ABSTRACT_ORIGIN (x))))
+	    clone_underlying_type (x);
 
 	  if (type != error_mark_node
 	      && TYPE_NAME (type)
diff --git a/gcc/cp/pt.c b/gcc/cp/pt.c
index 13a2361..a36e951 100644
--- a/gcc/cp/pt.c
+++ b/gcc/cp/pt.c
@@ -8921,22 +8921,56 @@ tsubst (tree t, tree args, tsubst_flags_t complain, tree in_decl)
       && TYPE_NAME (t) != TYPE_MAIN_DECL (t))
     {
       tree decl = TYPE_NAME (t);
-      
+
+      if (TREE_CODE (t) == TYPENAME_TYPE)
+	/* Here we are in the case of a declaration that reads:
+	     typedef typename Foo::bar baz;
+	   t is the typedef type variant of the TYPENAME_TYPE type
+	   representing typename Foo::bar.
+	   TYPE_NAME (t) is set to a TYPE_DECL representing the baz typedef.
+	   So before going forward with re-using the baz typedef,
+	   let's see first if tsubst-ing typename Foo::bar leads to an error or not.
+	   If it leads to an error then re-using the baz typedef is wrong. In that
+	   case we shall fall through and let the Foo::bar TYPENAME_TYPE be handled
+	   by the appropriate code that comes later in this function.  */
+	if (tsubst (TREE_TYPE (TYPE_MAIN_DECL (t)), args, tf_none, in_decl)
+	    == error_mark_node)
+	  goto after_typedef_reuse;
+
       if (DECL_CLASS_SCOPE_P (decl)
 	  && CLASSTYPE_TEMPLATE_INFO (DECL_CONTEXT (decl))
-	  && uses_template_parms (DECL_CONTEXT (decl)))
+	  && uses_template_parms (DECL_CONTEXT (decl))
+	  && uses_template_parms (decl))
 	{
 	  tree tmpl = most_general_template (DECL_TI_TEMPLATE (decl));
 	  tree gen_args = tsubst (DECL_TI_ARGS (decl), args, complain, in_decl);
 	  r = retrieve_specialization (tmpl, gen_args, false);
+	  if (r && !enforce_access (TYPE_BINFO (DECL_CONTEXT (r)), r, r))
+		return error_mark_node;
 	}
       else if (DECL_FUNCTION_SCOPE_P (decl)
 	       && DECL_TEMPLATE_INFO (DECL_CONTEXT (decl))
 	       && uses_template_parms (DECL_TI_ARGS (DECL_CONTEXT (decl))))
 	r = retrieve_local_specialization (decl);
       else
-	/* The typedef is from a non-template context.  */
-	return t;
+	{
+	  /* The typedef is from a non-template context.  */
+
+	  if (DECL_CONTEXT (decl)
+	      && AGGREGATE_TYPE_P (DECL_CONTEXT (decl))
+	      && !uses_template_parms (DECL_CONTEXT (decl)))
+	    {
+	      /* t has been lowered from a typedef'ed type (namely decl) that is
+		 a member of a class/struct. E.g:
+		 class Foo {
+		  typedef t X;
+		 };
+		 In that example, we need to make sure X is accessible from here.  */
+	      if (!enforce_access (TYPE_BINFO (DECL_CONTEXT (decl)), decl, decl))
+		return error_mark_node;
+	    }
+	  return t;
+	}
 
       if (r)
 	{
@@ -8948,6 +8982,7 @@ tsubst (tree t, tree args, tsubst_flags_t complain, tree in_decl)
 	}
       /* Else we must be instantiating the typedef, so fall through.  */
     }
+after_typedef_reuse:
 
   if (type
       && TREE_CODE (t) != TYPENAME_TYPE
@@ -16525,7 +16560,15 @@ resolve_typename_type (tree type, bool only_current_p)
   gcc_assert (TREE_CODE (type) == TYPENAME_TYPE);
 
   scope = TYPE_CONTEXT (type);
-  name = TYPE_IDENTIFIER (type);
+  /* Usually the non-qualified identifier of a TYPENAME_TYPE is
+     TYPE_IDENTIFIER (type). But when 'type' is a typedef variant of
+     a TYPENAME_TYPE node, then TYPE_NAME (type) is set to the TYPE_DECL representing
+     the typedef. In that case TYPE_IDENTIFIER (type) is not the non-qualified
+     identifier  of the TYPENAME_TYPE anymore.
+     So by getting the TYPE_IDENTIFIER of the _main declaration_ of the
+     TYPENAME_TYPE instead, we avoid messing up with a possible
+     typedef variant case.  */
+  name = TYPE_IDENTIFIER (TREE_TYPE (TYPE_MAIN_DECL (type)));
 
   /* If the SCOPE is itself a TYPENAME_TYPE, then we need to resolve
      it first before we can figure out what NAME refers to.  */
diff --git a/gcc/testsuite/g++.dg/template/typedef11.C b/gcc/testsuite/g++.dg/template/typedef11.C
new file mode 100644
index 0000000..6e71729
--- /dev/null
+++ b/gcc/testsuite/g++.dg/template/typedef11.C
@@ -0,0 +1,25 @@
+// Author: Dodji Seketeli <dodji@redhat.com>
+// Origin: PR c++/26693
+// { dg-do compile }
+
+
+class Alpha
+{
+  typedef int X; // { dg-error "'typedef int Alpha::X' is private" }
+};
+
+template<int>
+class Beta
+{
+    typedef int Y; // { dg-error "'typedef int Beta<0>::Y' is private" }
+};
+
+template <int>
+int
+bar ()
+{
+  Beta<0>::Y i = 0; // { dg-error "within this context" }
+  return Alpha::X (); // { dg-error "within this context" }
+}
+
+int i = bar<0> ();
diff --git a/gcc/tree.c b/gcc/tree.c
index 10b50d1..4b9e793 100644
--- a/gcc/tree.c
+++ b/gcc/tree.c
@@ -9167,4 +9167,65 @@ build_target_option_node (void)
   return t;
 }
 
+/* Setup a TYPE_DECL node as a typedef representation.
+
+   X is a TYPE_DECL for a typedef statement.  Create a brand new
+   ..._TYPE node (which will be just a variant of the existing
+   ..._TYPE node with identical properties) and then install X
+   as the TYPE_NAME of this brand new (duplicate) ..._TYPE node.
+
+   The whole point here is to end up with a situation where each
+   and every ..._TYPE node the compiler creates will be uniquely
+   associated with AT MOST one node representing a typedef name.
+   This way, even though the compiler substitutes corresponding
+   ..._TYPE nodes for TYPE_DECL (i.e. "typedef name") nodes very
+   early on, later parts of the compiler can always do the reverse
+   translation and get back the corresponding typedef name.  For
+   example, given:
+
+	typedef struct S MY_TYPE;
+	MY_TYPE object;
+
+   Later parts of the compiler might only know that `object' was of
+   type `struct S' if it were not for code just below.  With this
+   code however, later parts of the compiler see something like:
+
+	struct S' == struct S
+	typedef struct S' MY_TYPE;
+	struct S' object;
+
+    And they can then deduce (from the node for type struct S') that
+    the original object declaration was:
+
+		MY_TYPE object;
+
+    Being able to do this is important for proper support of protoize,
+    and also for generating precise symbolic debugging information
+    which takes full account of the programmer's (typedef) vocabulary.
+
+    Obviously, we don't want to generate a duplicate ..._TYPE node if
+    the TYPE_DECL node that we are now processing really represents a
+    standard built-in type.  */
+
+void
+clone_underlying_type (tree x)
+{
+  if (DECL_IS_BUILTIN (x))
+    {
+      if (TYPE_NAME (TREE_TYPE (x)) == 0)
+	TYPE_NAME (TREE_TYPE (x)) = x;
+    }
+  else if (TREE_TYPE (x) != error_mark_node
+	   && DECL_ORIGINAL_TYPE (x) == NULL_TREE)
+    {
+      tree tt = TREE_TYPE (x);
+      DECL_ORIGINAL_TYPE (x) = tt;
+      tt = build_variant_type_copy (tt);
+      TYPE_STUB_DECL (tt) = TYPE_STUB_DECL (DECL_ORIGINAL_TYPE (x));
+      TYPE_NAME (tt) = x;
+      TREE_USED (tt) = TREE_USED (x);
+      TREE_TYPE (x) = tt;
+    }
+}
+
 #include "gt-tree.h"
diff --git a/gcc/tree.h b/gcc/tree.h
index a85b4c6..2b3b128 100644
--- a/gcc/tree.h
+++ b/gcc/tree.h
@@ -3405,6 +3405,8 @@ struct tree_target_option GTY(())
 /* Return a tree node that encapsulates the current target options.  */
 extern tree build_target_option_node (void);
 
+extern void clone_underlying_type (tree x);
+
 
 /* Define the overall contents of a tree node.
    It may be any of the structures declared above
diff --git a/libstdc++-v3/include/ext/bitmap_allocator.h b/libstdc++-v3/include/ext/bitmap_allocator.h
index 7f5466a..7768bd2 100644
--- a/libstdc++-v3/include/ext/bitmap_allocator.h
+++ b/libstdc++-v3/include/ext/bitmap_allocator.h
@@ -549,11 +549,13 @@ _GLIBCXX_BEGIN_NAMESPACE(__gnu_cxx)
    */
   class free_list
   {
+  public:
     typedef size_t* 				value_type;
     typedef __detail::__mini_vector<value_type> vector_type;
     typedef vector_type::iterator 		iterator;
     typedef __mutex				__mutex_type;
 
+  private:
     struct _LT_pointer_compare
     {
       bool

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]