[C++] Allow p->~T() when T is a vector

Submitted by Marc Glisse on Aug. 2, 2012, 11:42 a.m.

Details

Message ID alpine.DEB.2.02.1208021333001.23019@stedding.saclay.inria.fr
State New
Headers show

Commit Message

Marc Glisse Aug. 2, 2012, 11:42 a.m.
Hello,

this patch allows p->~T() when T is (after substitution) a vector, which 
is necessary for use in std::vector for instance.

gcc/cp/ChangeLog
2012-08-02  Marc Glisse  <marc.glisse@inria.fr>

 	* pt.c (tsubst_copy_and_build): Handle VECTOR_TYPE like scalars.

gcc/testsuite/ChangeLog
2012-08-02  Marc Glisse  <marc.glisse@inria.fr>

 	* g++.dg/ext/vector17.C: New testcase.

Comments

Richard Guenther Aug. 2, 2012, 12:08 p.m.
On Thu, Aug 2, 2012 at 1:42 PM, Marc Glisse <marc.glisse@inria.fr> wrote:
> Hello,
>
> this patch allows p->~T() when T is (after substitution) a vector, which is
> necessary for use in std::vector for instance.

Why not include VECTOR_TYPE in ARITHMETIC_TYPE_P?

> gcc/cp/ChangeLog
> 2012-08-02  Marc Glisse  <marc.glisse@inria.fr>
>
>         * pt.c (tsubst_copy_and_build): Handle VECTOR_TYPE like scalars.
>
> gcc/testsuite/ChangeLog
> 2012-08-02  Marc Glisse  <marc.glisse@inria.fr>
>
>         * g++.dg/ext/vector17.C: New testcase.
>
> --
> Marc Glisse
> Index: testsuite/g++.dg/ext/vector17.C
> ===================================================================
> --- testsuite/g++.dg/ext/vector17.C     (revision 0)
> +++ testsuite/g++.dg/ext/vector17.C     (revision 0)
> @@ -0,0 +1,12 @@
> +/* { dg-do compile } */
> +typedef double __attribute__((vector_size(1024) )) vec;
> +
> +template <class T>
> +void f (T *p)
> +{
> +  p->~T();
> +}
> +void g (vec *p)
> +{
> +  f(p);
> +}
>
> Property changes on: testsuite/g++.dg/ext/vector17.C
> ___________________________________________________________________
> Added: svn:eol-style
>    + native
> Added: svn:keywords
>    + Author Date Id Revision URL
>
> Index: cp/pt.c
> ===================================================================
> --- cp/pt.c     (revision 190071)
> +++ cp/pt.c     (working copy)
> @@ -13844,21 +13844,22 @@ tsubst_copy_and_build (tree t,
>                                     args, complain, in_decl);
>         else
>           member = tsubst_copy (member, args, complain, in_decl);
>         if (member == error_mark_node)
>           return error_mark_node;
>
>         if (type_dependent_expression_p (object))
>           /* We can't do much here.  */;
>         else if (!CLASS_TYPE_P (object_type))
>           {
> -           if (SCALAR_TYPE_P (object_type))
> +           if (SCALAR_TYPE_P (object_type)
> +               || TREE_CODE (object_type) == VECTOR_TYPE)
>               {
>                 tree s = NULL_TREE;
>                 tree dtor = member;
>
>                 if (TREE_CODE (dtor) == SCOPE_REF)
>                   {
>                     s = TREE_OPERAND (dtor, 0);
>                     dtor = TREE_OPERAND (dtor, 1);
>                   }
>                 if (TREE_CODE (dtor) == BIT_NOT_EXPR)
>
Ramana Radhakrishnan Aug. 2, 2012, 12:15 p.m.
On 08/02/12 13:08, Richard Guenther wrote:
> On Thu, Aug 2, 2012 at 1:42 PM, Marc Glisse <marc.glisse@inria.fr> wrote:
>> Hello,
>>
>> this patch allows p->~T() when T is (after substitution) a vector, which is
>> necessary for use in std::vector for instance.
>
> Why not include VECTOR_TYPE in ARITHMETIC_TYPE_P?

Not really, because SCALAR_TYPE_P uses ARITHMETIC_TYPE_P just below in 
cp-tree.h to detect whether something is scalar or not. It would need 
more refactoring I'd suspect ....

Ramana

>
>> gcc/cp/ChangeLog
>> 2012-08-02  Marc Glisse  <marc.glisse@inria.fr>
>>
>>          * pt.c (tsubst_copy_and_build): Handle VECTOR_TYPE like scalars.
>>
>> gcc/testsuite/ChangeLog
>> 2012-08-02  Marc Glisse  <marc.glisse@inria.fr>
>>
>>          * g++.dg/ext/vector17.C: New testcase.
>>
>> --
>> Marc Glisse
>> Index: testsuite/g++.dg/ext/vector17.C
>> ===================================================================
>> --- testsuite/g++.dg/ext/vector17.C     (revision 0)
>> +++ testsuite/g++.dg/ext/vector17.C     (revision 0)
>> @@ -0,0 +1,12 @@
>> +/* { dg-do compile } */
>> +typedef double __attribute__((vector_size(1024) )) vec;
>> +
>> +template <class T>
>> +void f (T *p)
>> +{
>> +  p->~T();
>> +}
>> +void g (vec *p)
>> +{
>> +  f(p);
>> +}
>>
>> Property changes on: testsuite/g++.dg/ext/vector17.C
>> ___________________________________________________________________
>> Added: svn:eol-style
>>     + native
>> Added: svn:keywords
>>     + Author Date Id Revision URL
>>
>> Index: cp/pt.c
>> ===================================================================
>> --- cp/pt.c     (revision 190071)
>> +++ cp/pt.c     (working copy)
>> @@ -13844,21 +13844,22 @@ tsubst_copy_and_build (tree t,
>>                                      args, complain, in_decl);
>>          else
>>            member = tsubst_copy (member, args, complain, in_decl);
>>          if (member == error_mark_node)
>>            return error_mark_node;
>>
>>          if (type_dependent_expression_p (object))
>>            /* We can't do much here.  */;
>>          else if (!CLASS_TYPE_P (object_type))
>>            {
>> -           if (SCALAR_TYPE_P (object_type))
>> +           if (SCALAR_TYPE_P (object_type)
>> +               || TREE_CODE (object_type) == VECTOR_TYPE)
>>                {
>>                  tree s = NULL_TREE;
>>                  tree dtor = member;
>>
>>                  if (TREE_CODE (dtor) == SCOPE_REF)
>>                    {
>>                      s = TREE_OPERAND (dtor, 0);
>>                      dtor = TREE_OPERAND (dtor, 1);
>>                    }
>>                  if (TREE_CODE (dtor) == BIT_NOT_EXPR)
>>
>
Marc Glisse Aug. 2, 2012, 12:20 p.m.
On Thu, 2 Aug 2012, Richard Guenther wrote:

> On Thu, Aug 2, 2012 at 1:42 PM, Marc Glisse <marc.glisse@inria.fr> wrote:
>> Hello,
>>
>> this patch allows p->~T() when T is (after substitution) a vector, which is
>> necessary for use in std::vector for instance.
>
> Why not include VECTOR_TYPE in ARITHMETIC_TYPE_P?

I often aim for the least disruptive change...

I just tried to look at the uses of ARITHMETIC_TYPE_P, and the first one I 
could find was to detect whether the type is convertible to bool :-(
Gabriel Dos Reis Aug. 2, 2012, 2:19 p.m.
On Thu, Aug 2, 2012 at 7:08 AM, Richard Guenther
<richard.guenther@gmail.com> wrote:
> On Thu, Aug 2, 2012 at 1:42 PM, Marc Glisse <marc.glisse@inria.fr> wrote:
>> Hello,
>>
>> this patch allows p->~T() when T is (after substitution) a vector, which is
>> necessary for use in std::vector for instance.
>
> Why not include VECTOR_TYPE in ARITHMETIC_TYPE_P?

We have decided long ago that abstractions in the compiler
should use names and semantics following the standard texts
so that reading the source code should not reserve surprises
to anyone familiar with the standards, hence minimize help
subtle bugs and reduce efforts needed to understand the compiler.
Putting VECTOR_TYPE in ARITHMETIC_TYPE_P violates that
common sense principle.

-- Gaby
Jason Merrill Aug. 2, 2012, 2:33 p.m.
On 08/02/2012 07:42 AM, Marc Glisse wrote:
> +	    if (SCALAR_TYPE_P (object_type)
> +		|| TREE_CODE (object_type) == VECTOR_TYPE)

You can use "scalarish_type_p" for this test.  OK with that change.

Jason
Marc Glisse Aug. 2, 2012, 3:22 p.m.
On Thu, 2 Aug 2012, Jason Merrill wrote:

> On 08/02/2012 07:42 AM, Marc Glisse wrote:
>> +	    if (SCALAR_TYPE_P (object_type)
>> +		|| TREE_CODE (object_type) == VECTOR_TYPE)
>
> You can use "scalarish_type_p" for this test.  OK with that change.

Wow, I'd missed it, a function that does exactly what I need, thanks!

Patch hide | download patch | download mbox

Index: testsuite/g++.dg/ext/vector17.C
===================================================================
--- testsuite/g++.dg/ext/vector17.C	(revision 0)
+++ testsuite/g++.dg/ext/vector17.C	(revision 0)
@@ -0,0 +1,12 @@ 
+/* { dg-do compile } */
+typedef double __attribute__((vector_size(1024) )) vec;
+
+template <class T>
+void f (T *p)
+{
+  p->~T();
+}
+void g (vec *p)
+{
+  f(p);
+}

Property changes on: testsuite/g++.dg/ext/vector17.C
___________________________________________________________________
Added: svn:eol-style
   + native
Added: svn:keywords
   + Author Date Id Revision URL

Index: cp/pt.c
===================================================================
--- cp/pt.c	(revision 190071)
+++ cp/pt.c	(working copy)
@@ -13844,21 +13844,22 @@  tsubst_copy_and_build (tree t,
 				    args, complain, in_decl);
 	else
 	  member = tsubst_copy (member, args, complain, in_decl);
 	if (member == error_mark_node)
 	  return error_mark_node;
 
 	if (type_dependent_expression_p (object))
 	  /* We can't do much here.  */;
 	else if (!CLASS_TYPE_P (object_type))
 	  {
-	    if (SCALAR_TYPE_P (object_type))
+	    if (SCALAR_TYPE_P (object_type)
+		|| TREE_CODE (object_type) == VECTOR_TYPE)
 	      {
 		tree s = NULL_TREE;
 		tree dtor = member;
 
 		if (TREE_CODE (dtor) == SCOPE_REF)
 		  {
 		    s = TREE_OPERAND (dtor, 0);
 		    dtor = TREE_OPERAND (dtor, 1);
 		  }
 		if (TREE_CODE (dtor) == BIT_NOT_EXPR)