Re: [C++ Patch] for c++/52465
2012/4/11 Jason Merrill : [...] > Your ChangeLog needs to be adjusted. :) I did write an updated ChangeLog, but I pasted here the wrong one... I committed it to trunk yesterday, hopefully with the correct ChangeLog . I'll be backporting it to 4.7 after a relaxing weekend :-) -- Fabien
Re: [C++ Patch] for c++/52465
On 04/10/2012 04:23 PM, Fabien Chêne wrote: Ah yes, that's slightly better. (I've kept the NULL check in strip_using_decl, it seems safer to me. Just tell me if you prefer not) Tested x86_64-unkown-linux-gnu. OK for trunk and 4.7 ? 2012-03-08 Fabien Chêne PR c++/52465 * decl.c (grokdeclarator): Call strip_using_decl. * parser.c (cp_parser_class_name): Call strip_using_decl and perform some checks on the target decl. * name-lookup.c (strip_using_decl): Returns NULL_TREE if the decl to be stripped is NULL_TREE. (qualify_lookup): Call strip_using_decl and perform some checks on the target decl. Your ChangeLog needs to be adjusted. :) The patch is OK, thanks! Jason
Re: [C++ Patch] for c++/52465
2012/4/7 Jason Merrill : > On 04/07/2012 11:37 AM, Fabien Chêne wrote: >> >> Perhaps it is more correct like that, in cp_parser_set_decl_spec_type ? > > Even that seems late. Why not just return the target decl from > cp_parser_class_name? Ah yes, that's slightly better. (I've kept the NULL check in strip_using_decl, it seems safer to me. Just tell me if you prefer not) Tested x86_64-unkown-linux-gnu. OK for trunk and 4.7 ? gcc/testsuite/ChangeLog 2012-03-08 Fabien Chêne PR c++/52465 * g++.dg/lookup/using52.C: New. gcc/cp/ChangeLog 2012-03-08 Fabien Chêne PR c++/52465 * decl.c (grokdeclarator): Call strip_using_decl. * parser.c (cp_parser_class_name): Call strip_using_decl and perform some checks on the target decl. * name-lookup.c (strip_using_decl): Returns NULL_TREE if the decl to be stripped is NULL_TREE. (qualify_lookup): Call strip_using_decl and perform some checks on the target decl. -- Fabien 52465_3.patch Description: Binary data
Re: [C++ Patch] for c++/52465
On 04/07/2012 11:37 AM, Fabien Chêne wrote: Perhaps it is more correct like that, in cp_parser_set_decl_spec_type ? Even that seems late. Why not just return the target decl from cp_parser_class_name? Jason
Re: [C++ Patch] for c++/52465
2012/3/12 Fabien Chêne :
> Salut Dodji,
>
> 2012/3/12 Dodji Seketeli :
> [...]
>>> Index: gcc/cp/decl.c
>>> ===
>>> --- gcc/cp/decl.c (revision 184891)
>>> +++ gcc/cp/decl.c (working copy)
>>> @@ -8686,6 +8686,9 @@ grokdeclarator (const cp_declarator *dec
>>> type = NULL_TREE;
>>> type_was_error_mark_node = true;
>>> }
>>> +
>>> + type = strip_using_decl (type);
>>> +
>>
>> I am a little bit curious as to why this change is necessary. It seems
>> to me that the test case of your patch would pass even without this
>> change, wouldn't it?
>
> Yes, this testcase would pass, but an existing testcase wouldn't. I
> don't remeber which one, but I think it was related to using
> declarations that refer to a typedef.
>
> struct A { typedef int T; };
> stuct B : A { using B::T; };
More precisely, the testcase which was failing is g++.other/using5.C.
--
Fabien
Re: [C++ Patch] for c++/52465
2012/3/29 Jason Merrill : > On 03/08/2012 04:34 PM, Fabien Chêne wrote: >> >> * decl.c (grokdeclarator): Call strip_using_decl. > > > I would think we ought to be stripping USING_DECLs at a lower level, when we > first look up the name in the parser. They shouldn't make it as far as > grokdeclarator. Perhaps it is more correct like that, in cp_parser_set_decl_spec_type ? Bootstrapped/tested x86_64-unknown-linux-gnu. gcc/testsuite/ChangeLog 2012-04-07 Fabien Chêne PR c++/52465 * g++.dg/lookup/using52.C: New. gcc/cp/ChangeLog 2012-04-07 Fabien Chêne PR c++/52465 * parser.c (cp_parser_class_name): Call strip_using_decl and perform some checks on the target decl. (cp_parser_set_decl_spec_type): Change its prototype so that the second argument be modifiable, and strip using declarations of that argument. (cp_parser_type_specifier): Adjust the calls to cp_parser_set_decl_spec_type. (cp_parser_simple_type_specifier): Likewise. * name-lookup.c (strip_using_decl): Returns NULL_TREE if the decl to be stripped is NULL_TREE. (qualify_lookup): Call strip_using_decl and perform some checks on the target decl. -- Fabien 52465_2.patch Description: Binary data
Re: [C++ Patch] for c++/52465
On 03/08/2012 04:34 PM, Fabien Chêne wrote: * decl.c (grokdeclarator): Call strip_using_decl. I would think we ought to be stripping USING_DECLs at a lower level, when we first look up the name in the parser. They shouldn't make it as far as grokdeclarator. Jason
Re: [C++ Patch] for c++/52465
Salut Dodji,
2012/3/12 Dodji Seketeli :
[...]
>> Index: gcc/cp/decl.c
>> ===
>> --- gcc/cp/decl.c (revision 184891)
>> +++ gcc/cp/decl.c (working copy)
>> @@ -8686,6 +8686,9 @@ grokdeclarator (const cp_declarator *dec
>> type = NULL_TREE;
>> type_was_error_mark_node = true;
>> }
>> +
>> + type = strip_using_decl (type);
>> +
>
> I am a little bit curious as to why this change is necessary. It seems
> to me that the test case of your patch would pass even without this
> change, wouldn't it?
Yes, this testcase would pass, but an existing testcase wouldn't. I
don't remeber which one, but I think it was related to using
declarations that refer to a typedef.
struct A { typedef int T; };
stuct B : A { using B::T; };
--
Fabien
Re: [C++ Patch] for c++/52465
Hello Fabien, Fabien Chêne a écrit: [...] > Index: gcc/cp/decl.c > === > --- gcc/cp/decl.c (revision 184891) > +++ gcc/cp/decl.c (working copy) > @@ -8686,6 +8686,9 @@ grokdeclarator (const cp_declarator *dec >type = NULL_TREE; >type_was_error_mark_node = true; > } > + > + type = strip_using_decl (type); > + I am a little bit curious as to why this change is necessary. It seems to me that the test case of your patch would pass even without this change, wouldn't it? Thanks. -- Dodji
