[PATCH] Follow up of PR c++/26693

[H.J. Lu]

On Mon, Jan 19, 2009 at 9:29 AM, Dodji Seketeli <dodji@redhat.com> wrote:
> Hello,
>
> Please find an updated version of the patch that I hope should address your
> comments.
>
> I have answered your comments below.
>
> Jason Merrill a écrit :
>>>
>>> +/* Returns true if X is a typedef type.  */
>>> +bool
>>> +is_typedef_type (tree x)
>>> +{
>>> +  return (x
>>> +      && TREE_CODE (x) == TYPE_DECL
>>
>> The name is misleading;
>
> Right. I changed that to is_typedef_decl.
>
>>
>>> +/* The chained list of some types that are referenced in templates.
>>> +   These types are those which need to be access checked at
>>> +   template instantiation time.  For the time being, only typedef-ed
>>> types defined
>>> +   as class members are put here at parsing time.
>>> +   Other types for which access check could be required at template
>>> instantiation
>>> +   time could be added later.  */
>>> +#define MEMBER_TYPES_NEEDING_ACCESS_CHECK(NODE) DECL_ACCESS (NODE)
>>
>> So this doesn't handle nested classes?  Why not?
>>
>
> Yes, types that are nested classes seems to be properly access checked in
> the current code base in gcc 4.3 and in trunk. That's why I did not feel the
> need to add them to that list.
>
>>> +      if (current_function_decl)
>>> +    templ_type = current_function_decl;
>>
>> templ_type is misnamed.  Also, try current_scope().
>
> Fixed.
>
>>
>>> +      && ((current_class_type
>>> +           && !same_type_p (scope, current_class_type)
>>> +           && !is_class_nested (scope, current_class_type))
>>> +          || (current_function_decl
>>> +          && !DECL_FUNCTION_MEMBER_P (current_function_decl))))
>>
>> I think this could all be simplified to
>>
>> && !currently_open_class (scope)
>>
>> No need to add a new is_class_nested function.
>
> Right. I have simplified it accordingly.
>
>
>>> +      || TREE_CODE (t) == NAMESPACE_DECL
>>> +      || args == NULL)
>>
>> Steve Ellcey just added a check for args==NULL a bit lower, so this
>> shouldn't be needed anymore.
>>
>
> Ah right. I have rebased the patch on top of his commit and so I removed
>  the args == NULL I did add. Thanks for noticing this.
>
>
>>> +  node = build_tree_list (type_decl, scope);
>>> +  MEMBER_TYPES_NEEDING_ACCESS_CHECK (templ_decl) =
>>> +    chainon (node, MEMBER_TYPES_NEEDING_ACCESS_CHECK (templ_decl));
>>
>> tree_cons
>
> Fixed.
>
> The resulting patch applied to trunk compiles libstdc++ and passes regtests
> on x86_64.
>
> Thanks.
>
> gcc/ChangeLog:
> 2009-01-19  Dodji Seketeli  <dodji@redhat.com>
>
>        PR c++/26693
>        * c-decl.c: (clone_underlying_type): Move this  ...
>        * c-common.c (set_underlying_type): ... here.
>        Also, make sure the function  properly sets TYPE_STUB_DECL() on
>        the newly created typedef variant type.
>        (is_typedef_decl ): New entry point.
>        * tree.h: Added a new member member_types_needing_access_check to
>        struct tree_decl_non_common.
>        (set_underlying_type): New entry point.
>        (is_typedef_type): Likewise.
>
> gcc/cp/ChangeLog/
> 2009-01-19  Dodji Seketeli  <dodji@redhat.com>
>
>        PR c++/26693
>        * decl2.c (grokfield): when a typedef appears in a
>        class, create the typedef variant type node for it.
>        (save_template_attributes): Creating typedef variant type node
>         here is now useless.
>        * decl.c (grokdeclarator): If the typedef'ed struct/class was
>        anonymous, set the proper type name to all its type variants.
>        * name-lookup.c (pushdecl_maybe_friend): Reuse the
>        set_underlying_type function to install typedef variant types.
>        * cp-tree.h (MEMBER_TYPES_NEEDING_ACCESS_CHECK): New template
> accessor
>        macro.
>        (append_type_to_template_for_access_check): New entry points.
>        * semantics.c (check_accessibility_of_qualified_id):
>        When a typedef that is a member of a class appears in a template,
>        add it to the template. It will be ...
>        * pt.c (instantiate_class_template, instantiate_template ): ...
> access
>        checked at template instantiation time.
>        (tsubst): Handle the case of being called with NULL args.
>        (resolve_type_name_type): The type name should be the name of the
>        main type variant.
>        (append_type_to_template_for_access_check): New entry point.
>
> gcc/testsuite/ChangeLog:
> 2009-01-19  Dodji Seketeli  <dodji@redhat.com>
>
>        PR c++/26693
>        * g++.dg/template/typedef11.C: New test.
>        * g++.dg/template/typedef12.C: Likewise.
>        * g++.dg/template/typedef13.C: Likewise.
>        * g++.dg/template/typedef14.C: Likewise.
>        * g++.dg/template/sfinae3.C: Compile this pedantically.
>        The only errors expected should be the one saying the typedef is ill
>        formed.
>        * g++.old-deja/g++.pt/typename8.C: Likewise.
>        * g++.dg/template/access11.C: Update this.
>
> libstdc++-v3/ChangeLog:
> 2009-01-19  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++.
>

This patch breaks bootstrap:

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=38930

-- 
H.J.