This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PATCH] PR c++/26693
- From: Dodji Seketeli <dodji at redhat dot com>
- To: Jason Merrill <jason at redhat dot com>
- Cc: Manuel LÃpez-IbÃÃez <lopezibanez at gmail dot com>, Gcc Patch List <gcc-patches at gcc dot gnu dot org>
- Date: Wed, 12 Nov 2008 15:19:10 +0100
- Subject: Re: [PATCH] PR c++/26693
- References: <4908DEAF.5040607@redhat.com> <6c33472e0810291622y13afde4tcbef839ad16fc4df@mail.gmail.com> <4909C2F6.4090901@redhat.com> <490F4917.1010702@redhat.com>
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