| Message ID | 098b6ade-1450-ffd4-5574-7661820a2b85@oracle.com |
|---|---|
| State | New |
| Headers | show |
| Series | [C++] Remove obsolete _vptr check (and fix location) | expand |
Hi, Il 17 Novembre 2018 11:00:45 CET, Paolo Carlini <paolo.carlini@oracle.com> ha scritto: >Hi, > >while I was working on some location issues I noticed this check which >seems obsolete to me and means that we unnecessarily reject kosher >code: >indeed we don't test for it anywhere and neither ICC nor clang enforce >it. Never mind, got confused for various reasons... too bad anyway that we unconditionally reject such member name even when there is no virtual pointer around. Anyway, I'll send a new, straightforward patch only fixing the two locations. Thanks, Paolo
Hi again... On 17/11/18 12:56, Paolo Carlini wrote: > Never mind, got confused for various reasons... too bad anyway that we > unconditionally reject such member name even when there is no virtual > pointer around. Anyway, I'll send a new, straightforward patch only > fixing the two locations. ... earlier today - I was running some errands - it occurred to me that, for example, gdb would have issues with a _vptr member, but that's not the case. Thus, all in all, I'm still not convinced that we want that old check. Anyway, in case we want to go for something super-safe at this stage, I'm attaching the promised patchlet only taking care of the locations. Paolo. ///////////////// Index: cp/decl2.c =================================================================== --- cp/decl2.c (revision 266233) +++ cp/decl2.c (working copy) @@ -836,14 +836,16 @@ grokfield (const cp_declarator *declarator, { if (TREE_CODE (name) == TEMPLATE_ID_EXPR) { - error ("explicit template argument list not allowed"); + error_at (declarator->id_loc, + "explicit template argument list not allowed"); return error_mark_node; } if (IDENTIFIER_POINTER (name)[0] == '_' && id_equal (name, "_vptr")) - error ("member %qD conflicts with virtual function table field name", - value); + error_at (declarator->id_loc, + "member %qD conflicts with virtual function " + "table field name", value); } /* Stash away type declarations. */ Index: testsuite/g++.dg/other/vptr.C =================================================================== --- testsuite/g++.dg/other/vptr.C (nonexistent) +++ testsuite/g++.dg/other/vptr.C (working copy) @@ -0,0 +1,4 @@ +struct S +{ + virtual void _vptr() = 0; // { dg-error "16:member .virtual void S::_vptr\\(\\). conflicts with virtual function table field name" } +}; Index: testsuite/g++.dg/template/crash91.C =================================================================== --- testsuite/g++.dg/template/crash91.C (revision 266231) +++ testsuite/g++.dg/template/crash91.C (working copy) @@ -4,5 +4,5 @@ template<int> void foo(); struct A { - typedef void foo<0>(); // { dg-error "explicit template argument list not allowed" } + typedef void foo<0>(); // { dg-error "16:explicit template argument list not allowed" } };
On Sat, Nov 17, 2018 at 5:01 AM Paolo Carlini <paolo.carlini@oracle.com> wrote: > while I was working on some location issues I noticed this check which > seems obsolete to me and means that we unnecessarily reject kosher code: > indeed we don't test for it anywhere and neither ICC nor clang enforce > it. I also went through the SVN history and the check is *extremely* > old, I think rms is the last person who touched it. The location fix is > rather obvious (in principle we could do the same for the check I'm > proposing to remove). OK. The test is obsolete, as you say: the vptr name cannot be written by the user, as it contains either . or $. Jason
Index: cp/decl2.c =================================================================== --- cp/decl2.c (revision 266233) +++ cp/decl2.c (working copy) @@ -804,7 +804,6 @@ grokfield (const cp_declarator *declarator, tree value; const char *asmspec = 0; int flags; - tree name; if (init && TREE_CODE (init) == TREE_LIST @@ -829,21 +828,12 @@ grokfield (const cp_declarator *declarator, if (value == void_type_node) return value; - - name = DECL_NAME (value); - - if (name != NULL_TREE) + if (DECL_NAME (value) + && TREE_CODE (DECL_NAME (value)) == TEMPLATE_ID_EXPR) { - if (TREE_CODE (name) == TEMPLATE_ID_EXPR) - { - error ("explicit template argument list not allowed"); - return error_mark_node; - } - - if (IDENTIFIER_POINTER (name)[0] == '_' - && id_equal (name, "_vptr")) - error ("member %qD conflicts with virtual function table field name", - value); + error_at (declarator->id_loc, + "explicit template argument list not allowed"); + return error_mark_node; } /* Stash away type declarations. */ Index: testsuite/g++.dg/template/crash91.C =================================================================== --- testsuite/g++.dg/template/crash91.C (revision 266231) +++ testsuite/g++.dg/template/crash91.C (working copy) @@ -4,5 +4,5 @@ template<int> void foo(); struct A { - typedef void foo<0>(); // { dg-error "explicit template argument list not allowed" } + typedef void foo<0>(); // { dg-error "16:explicit template argument list not allowed" } };