| Message ID | 5034D3FE.9020109@oracle.com |
|---|---|
| State | New |
| Headers | show |
On 08/22/2012 08:43 AM, Paolo Carlini wrote: > yesterday I had a look to this old PR and noticed that we are almost > doing already the right thing for the original testcase: we are for > classes, but we ICE (something recent) for enums. The latter issue seems > easy to fix: handle specially enums at the beginning of > supplement_binding_1 only when the comment says, that is in templates > (otherwise we hit an assert in dependent_type_p). Which assert? The underlying type ought to be set by the time we see a using, and it shouldn't be a TEMPLATE_TYPE_PARM. > Then, Comment #4 in the audit trail added a case of duplicate using > declaration which we should also reject, involving VAR_DECLs at function > scope (C++11 7.3.3/10). Seems also easy to fix, by checking the return > value of validate_nonmember_using_decl at the beginning of > do_local_using_decl and erroring out if it's null. Let's put the error in validate_nonmember_using_decl with the other errors. Jason
Hi, On 08/22/2012 04:09 PM, Jason Merrill wrote: > On 08/22/2012 08:43 AM, Paolo Carlini wrote: >> yesterday I had a look to this old PR and noticed that we are almost >> doing already the right thing for the original testcase: we are for >> classes, but we ICE (something recent) for enums. The latter issue seems >> easy to fix: handle specially enums at the beginning of >> supplement_binding_1 only when the comment says, that is in templates >> (otherwise we hit an assert in dependent_type_p). > > Which assert? The underlying type ought to be set by the time we see > a using, and it shouldn't be a TEMPLATE_TYPE_PARM. Here: if (!processing_template_decl) { /* If we are not processing a template, then nobody should be providing us with a dependent type. */ gcc_assert (type); gcc_assert (TREE_CODE (type) != TEMPLATE_TYPE_PARM || is_auto (type)); return false; } type is NULL_TREE. Makes sense? > >> Then, Comment #4 in the audit trail added a case of duplicate using >> declaration which we should also reject, involving VAR_DECLs at function >> scope (C++11 7.3.3/10). Seems also easy to fix, by checking the return >> value of validate_nonmember_using_decl at the beginning of >> do_local_using_decl and erroring out if it's null. > > Let's put the error in validate_nonmember_using_decl with the other > errors. Ok. Indeed, I wanted to do that, but was annoyed that we cannot assume we are in a function, because validate_nonmember_using_decl is also called by do_toplevel_using_decl. I suppose I should use at_function_scope_p () then. Paolo.
Hi. On 08/22/2012 04:14 PM, Paolo Carlini wrote: > Hi, > > On 08/22/2012 04:09 PM, Jason Merrill wrote: >> On 08/22/2012 08:43 AM, Paolo Carlini wrote: >>> yesterday I had a look to this old PR and noticed that we are almost >>> doing already the right thing for the original testcase: we are for >>> classes, but we ICE (something recent) for enums. The latter issue >>> seems >>> easy to fix: handle specially enums at the beginning of >>> supplement_binding_1 only when the comment says, that is in templates >>> (otherwise we hit an assert in dependent_type_p). >> >> Which assert? The underlying type ought to be set by the time we see >> a using, and it shouldn't be a TEMPLATE_TYPE_PARM. > Here: > > if (!processing_template_decl) > { > /* If we are not processing a template, then nobody should be > providing us with a dependent type. */ > gcc_assert (type); > gcc_assert (TREE_CODE (type) != TEMPLATE_TYPE_PARM || is_auto > (type)); > return false; > } > > type is NULL_TREE. Like this: #0 dependent_type_p (type=0x0) at /scratch/Gcc/svn-dirs/trunk/gcc/cp/pt.c:19020 #1 0x000000000073301f in supplement_binding_1 (binding=0x7ffff651c7f8, decl=0x7ffff6521cf0) at /scratch/Gcc/svn-dirs/trunk/gcc/cp/name-lookup.c:448 #2 0x0000000000733a65 in supplement_binding (binding=0x7ffff651c7f8, decl=0x7ffff6521cf0) at /scratch/Gcc/svn-dirs/trunk/gcc/cp/name-lookup.c:568 #3 0x000000000073e4e0 in push_class_level_binding_1 (name=0x7ffff6534948, x=0x7ffff6521cf0) at /scratch/Gcc/svn-dirs/trunk/gcc/cp/name-lookup.c:3162 #4 0x000000000073e545 in push_class_level_binding (name=0x7ffff6534948, x=0x7ffff6521cf0) at /scratch/Gcc/svn-dirs/trunk/gcc/cp/name-lookup.c:3180 #5 0x000000000073d753 in pushdecl_class_level (x=0x7ffff6521cf0) at /scratch/Gcc/svn-dirs/trunk/gcc/cp/name-lookup.c:2914 #6 0x00000000006d9538 in finish_member_declaration (decl=0x7ffff6521cf0) at /scratch/Gcc/svn-dirs/trunk/gcc/cp/semantics.c:2668 #7 0x00000000007469c2 in pushtag_1 (name=0x7ffff6534948, type=0x7ffff6535498, scope=ts_current) at /scratch/Gcc/svn-dirs/trunk/gcc/cp/name-lookup.c:5787 #8 0x0000000000746d78 in pushtag (name=0x7ffff6534948, type=0x7ffff6535498, scope=ts_current) at /scratch/Gcc/svn-dirs/trunk/gcc/cp/name-lookup.c:5853 #9 0x0000000000570021 in start_enum (name=0x7ffff6534948, enumtype=0x7ffff6535498, underlying_type=0x0, scoped_enum_p=false, is_new=0x7fffffffd3f7) at /scratch/Gcc/svn-dirs/trunk/gcc/cp/decl.c:12146 #10 0x0000000000658dfc in cp_parser_enum_specifier (parser=0x7ffff6534b00) at /scratch/Gcc/svn-dirs/trunk/gcc/cp/parser.c:14509 Paolo.
. thus, in short, what is happening is that, for this testcase:
class B
{
protected:
enum E { E1, E2, E3 };
};
class D : private B
{
public:
using B::E;
private:
enum E { };
};
we parse the new declaration enum E { }; and we reach
supplement_binding_1 before setting the underlying type of the new
declaration. The old declaration is fine, would not ICE dependent_type_p.
Paolo.
On 08/22/2012 10:55 AM, Paolo Carlini wrote: > . thus, in short, what is happening is that, for this testcase: > > class B > { > protected: > enum E { E1, E2, E3 }; > }; > > class D : private B > { > public: > using B::E; > > private: > enum E { }; > }; > > we parse the new declaration enum E { }; and we reach > supplement_binding_1 before setting the underlying type of the new > declaration. The old declaration is fine, would not ICE dependent_type_p. So with your change would we still ICE if D were a template? It seems like what we should be checking for is null underlying type. Jason
Index: testsuite/g++.dg/lookup/using53.C =================================================================== --- testsuite/g++.dg/lookup/using53.C (revision 0) +++ testsuite/g++.dg/lookup/using53.C (revision 0) @@ -0,0 +1,31 @@ +// PR c++/20420 + +class B +{ +protected: + enum E { E1, E2, E3 }; + struct S { int i; E e; }; +}; + +class D : private B +{ +public: + using B::E; // { dg-message "previous" } + using B::S; // { dg-message "previous" } + +private: + enum E {}; // { dg-error "conflicts" } + struct S {}; // { dg-error "conflicts" } +}; + +namespace N +{ + int i; +} + +void +f () +{ + using N::i; + using N::i; // { dg-error "declared" } +} Index: cp/name-lookup.c =================================================================== --- cp/name-lookup.c (revision 190590) +++ cp/name-lookup.c (working copy) @@ -441,7 +441,8 @@ supplement_binding_1 (cxx_binding *binding, tree d template in order to handle late matching of underlying type on an opaque-enum-declaration followed by an enum-specifier. */ - || (TREE_CODE (TREE_TYPE (target_decl)) == ENUMERAL_TYPE + || (processing_template_decl + && TREE_CODE (TREE_TYPE (target_decl)) == ENUMERAL_TYPE && TREE_CODE (TREE_TYPE (target_bval)) == ENUMERAL_TYPE && (dependent_type_p (ENUM_UNDERLYING_TYPE (TREE_TYPE (target_decl))) @@ -2581,7 +2582,12 @@ do_local_using_decl (tree decl, tree scope, tree n decl = validate_nonmember_using_decl (decl, scope, name); if (decl == NULL_TREE) - return; + { + /* C++11 7.3.3/10. */ + if (TREE_CODE (orig_decl) == VAR_DECL) + error ("%qD is already declared in this scope", name); + return; + } if (building_stmt_list_p () && at_function_scope_p ())