[C++] Remove obsolete _vptr check (and fix location)

Message ID 098b6ade-1450-ffd4-5574-7661820a2b85@oracle.com
State New
Headers show
Series
  • [C++] Remove obsolete _vptr check (and fix location)
Related show

Commit Message

Paolo Carlini Nov. 17, 2018, 10 a.m.
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. 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).

Tested 86_64-linux.

Thanks, Paolo.

/////////////////////
/cp
2018-11-17  Paolo Carlini  <paolo.carlini@oracle.com>

	* decl2.c (grokfield): Remove obsolete _vptr check; fix
	explicit template argument list error location.

/testsuite
2018-11-17  Paolo Carlini  <paolo.carlini@oracle.com>

	* g++.dg/template/crash91.C: Check location too.

Comments

Paolo Carlini Nov. 17, 2018, 11:56 a.m. | #1
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
Paolo Carlini Nov. 17, 2018, 6:14 p.m. | #2
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" } 
 };
Jason Merrill Nov. 18, 2018, 10:56 p.m. | #3
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

Patch

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" } 
 };