diff mbox series

C++ PATCHes to xvalue handling

Message ID CADzB+2kbdcbJ65BZsSKPw1bdmBOVwX--4Vdsznh62jtB+Sspaw@mail.gmail.com
State New
Headers show
Series C++ PATCHes to xvalue handling | expand

Commit Message

Jason Merrill May 23, 2018, 5:21 p.m. UTC
The first patch implements the adjustments from core issues 616 and
1213 to the value category of subobjects of class prvalues: they were
considered prvalues themselves, but that was kind of nonsensical.  Now
they are considered xvalues.  Along with this, I've removed the
diagnostic distinction between xvalues and prvalues when trying to use
one or the other as an lvalue; the important thing is that they are
rvalues.

The second patch corrects various issues with casts and xvalues/rvalue
references: we were treating an xvalue operand to dynamic_cast as an
lvalue, and we were objecting to casts from prvalue to rvalue
reference type.

Tested x86_64-pc-linux-gnu, applying to trunk.

Comments

Sudakshina Das May 25, 2018, 4:40 p.m. UTC | #1
On 23/05/18 18:21, Jason Merrill wrote:
> The first patch implements the adjustments from core issues 616 and
> 1213 to the value category of subobjects of class prvalues: they were
> considered prvalues themselves, but that was kind of nonsensical.  Now
> they are considered xvalues.  Along with this, I've removed the
> diagnostic distinction between xvalues and prvalues when trying to use
> one or the other as an lvalue; the important thing is that they are
> rvalues.
> 
> The second patch corrects various issues with casts and xvalues/rvalue
> references: we were treating an xvalue operand to dynamic_cast as an
> lvalue, and we were objecting to casts from prvalue to rvalue
> reference type.
> 

With the second patch:
commit f7d2790049fd1e59af4b69ee12f7c101cfe4cdab
Author: jason <jason@138bc75d-0d04-0410-961f-82ee72b054a4>
Date:   Wed May 23 17:21:39 2018 +0000

         Fix cast to rvalue reference from prvalue.

         * cvt.c (diagnose_ref_binding): Handle rvalue reference.
         * rtti.c (build_dynamic_cast_1): Don't try to build a reference to
         non-class type.  Handle xvalue argument.
         * typeck.c (build_reinterpret_cast_1): Allow cast from prvalue to
         rvalue reference.
         * semantics.c (finish_compound_literal): Do direct-initialization,
         not cast, to initialize a reference.

     git-svn-id: svn+ssh://gcc.gnu.org/svn/gcc/trunk@260622 
138bc75d-0d04-0410-961f-82ee72b054a4

I have observed the following failure in Spec2017 while building 
510.parest_r on aarch64-none-linux-gnu

aarch64-none-linux-gnu-g++ -c -o 
source/numerics/matrices.all_dimensions.o -DSPEC -DNDEBUG -Iinclude -I. 
-DSPEC_AUTO_SUPPRESS_OPENMP  -mcpu=cortex-a57+crypto -Ofast 
-fomit-frame-pointer     -fpermissive    -DSPEC_LP64 
source/numerics/matrices.all_dimensions.cc

source/numerics/matrices.all_dimensions.cc: In static member function 
'static void dealii::MatrixTools::apply_boundary_values(const 
std::map<unsigned int, double>&, dealii::BlockSparseMatrix<number>&, 
dealii::BlockVector<number>&, dealii::BlockVector<number>&, bool)':

source/numerics/matrices.all_dimensions.cc:469:50: error: lvalue 
required as unary '&' operand

         [this_sparsity.get_rowstart_indices()[row]];

                                                   ^

source/numerics/matrices.all_dimensions.cc:472:55: error: lvalue 
required as unary '&' operand

            [this_sparsity.get_rowstart_indices()[row]+1],

                                                        ^

source/numerics/matrices.all_dimensions.cc:474:55: error: lvalue 
required as unary '&' operand

            [this_sparsity.get_rowstart_indices()[row+1]],

                                                        ^

source/numerics/matrices.all_dimensions.cc:479:49: error: lvalue 
required as unary '&' operand

        [this_sparsity.get_rowstart_indices()[row]],

                                                  ^

source/numerics/matrices.all_dimensions.cc:481:51: error: lvalue 
required as unary '&' operand

        [this_sparsity.get_rowstart_indices()[row+1]],

                                                    ^

source/numerics/matrices.all_dimensions.cc:510:50: error: lvalue 
required as unary '&' operand

           [this_sparsity.get_rowstart_indices()[0]]);

Sudi

> Tested x86_64-pc-linux-gnu, applying to trunk.
>
Jason Merrill May 25, 2018, 8:08 p.m. UTC | #2
On Fri, May 25, 2018 at 12:40 PM, Sudakshina Das <sudi.das@arm.com> wrote:
> On 23/05/18 18:21, Jason Merrill wrote:
>>
>> The first patch implements the adjustments from core issues 616 and
>> 1213 to the value category of subobjects of class prvalues: they were
>> considered prvalues themselves, but that was kind of nonsensical.  Now
>> they are considered xvalues.  Along with this, I've removed the
>> diagnostic distinction between xvalues and prvalues when trying to use
>> one or the other as an lvalue; the important thing is that they are
>> rvalues.
>>
>> The second patch corrects various issues with casts and xvalues/rvalue
>> references: we were treating an xvalue operand to dynamic_cast as an
>> lvalue, and we were objecting to casts from prvalue to rvalue
>> reference type.
>>
>
> With the second patch:
> commit f7d2790049fd1e59af4b69ee12f7c101cfe4cdab
> Author: jason <jason@138bc75d-0d04-0410-961f-82ee72b054a4>
> Date:   Wed May 23 17:21:39 2018 +0000
>
>         Fix cast to rvalue reference from prvalue.
>
>         * cvt.c (diagnose_ref_binding): Handle rvalue reference.
>         * rtti.c (build_dynamic_cast_1): Don't try to build a reference to
>         non-class type.  Handle xvalue argument.
>         * typeck.c (build_reinterpret_cast_1): Allow cast from prvalue to
>         rvalue reference.
>         * semantics.c (finish_compound_literal): Do direct-initialization,
>         not cast, to initialize a reference.
>
>     git-svn-id: svn+ssh://gcc.gnu.org/svn/gcc/trunk@260622
> 138bc75d-0d04-0410-961f-82ee72b054a4
>
> I have observed the following failure in Spec2017 while building
> 510.parest_r on aarch64-none-linux-gnu
>
> aarch64-none-linux-gnu-g++ -c -o source/numerics/matrices.all_dimensions.o
> -DSPEC -DNDEBUG -Iinclude -I. -DSPEC_AUTO_SUPPRESS_OPENMP
> -mcpu=cortex-a57+crypto -Ofast -fomit-frame-pointer     -fpermissive
> -DSPEC_LP64 source/numerics/matrices.all_dimensions.cc
>
> source/numerics/matrices.all_dimensions.cc: In static member function
> 'static void dealii::MatrixTools::apply_boundary_values(const
> std::map<unsigned int, double>&, dealii::BlockSparseMatrix<number>&,
> dealii::BlockVector<number>&, dealii::BlockVector<number>&, bool)':
>
> source/numerics/matrices.all_dimensions.cc:469:50: error: lvalue required as
> unary '&' operand
>
>         [this_sparsity.get_rowstart_indices()[row]];
>
>                                                   ^
>
> source/numerics/matrices.all_dimensions.cc:472:55: error: lvalue required as
> unary '&' operand
>
>            [this_sparsity.get_rowstart_indices()[row]+1],
>
>                                                        ^
>
> source/numerics/matrices.all_dimensions.cc:474:55: error: lvalue required as
> unary '&' operand
>
>            [this_sparsity.get_rowstart_indices()[row+1]],
>
>                                                        ^
>
> source/numerics/matrices.all_dimensions.cc:479:49: error: lvalue required as
> unary '&' operand
>
>        [this_sparsity.get_rowstart_indices()[row]],
>
>                                                  ^
>
> source/numerics/matrices.all_dimensions.cc:481:51: error: lvalue required as
> unary '&' operand
>
>        [this_sparsity.get_rowstart_indices()[row+1]],
>
>                                                    ^
>
> source/numerics/matrices.all_dimensions.cc:510:50: error: lvalue required as
> unary '&' operand
>
>           [this_sparsity.get_rowstart_indices()[0]]);
>
> Sudi

Thanks, investigating.

Jason
Jason Merrill May 25, 2018, 8:55 p.m. UTC | #3
On Fri, May 25, 2018 at 4:08 PM, Jason Merrill <jason@redhat.com> wrote:
> On Fri, May 25, 2018 at 12:40 PM, Sudakshina Das <sudi.das@arm.com> wrote:
>> On 23/05/18 18:21, Jason Merrill wrote:
>>>
>>> The first patch implements the adjustments from core issues 616 and
>>> 1213 to the value category of subobjects of class prvalues: they were
>>> considered prvalues themselves, but that was kind of nonsensical.  Now
>>> they are considered xvalues.  Along with this, I've removed the
>>> diagnostic distinction between xvalues and prvalues when trying to use
>>> one or the other as an lvalue; the important thing is that they are
>>> rvalues.
>>>
>>> The second patch corrects various issues with casts and xvalues/rvalue
>>> references: we were treating an xvalue operand to dynamic_cast as an
>>> lvalue, and we were objecting to casts from prvalue to rvalue
>>> reference type.
>>>
>>
>> With the second patch:
>> commit f7d2790049fd1e59af4b69ee12f7c101cfe4cdab
>> Author: jason <jason@138bc75d-0d04-0410-961f-82ee72b054a4>
>> Date:   Wed May 23 17:21:39 2018 +0000
>>
>>         Fix cast to rvalue reference from prvalue.
>>
>>         * cvt.c (diagnose_ref_binding): Handle rvalue reference.
>>         * rtti.c (build_dynamic_cast_1): Don't try to build a reference to
>>         non-class type.  Handle xvalue argument.
>>         * typeck.c (build_reinterpret_cast_1): Allow cast from prvalue to
>>         rvalue reference.
>>         * semantics.c (finish_compound_literal): Do direct-initialization,
>>         not cast, to initialize a reference.
>>
>>     git-svn-id: svn+ssh://gcc.gnu.org/svn/gcc/trunk@260622
>> 138bc75d-0d04-0410-961f-82ee72b054a4
>>
>> I have observed the following failure in Spec2017 while building
>> 510.parest_r on aarch64-none-linux-gnu
>>
>> aarch64-none-linux-gnu-g++ -c -o source/numerics/matrices.all_dimensions.o
>> -DSPEC -DNDEBUG -Iinclude -I. -DSPEC_AUTO_SUPPRESS_OPENMP
>> -mcpu=cortex-a57+crypto -Ofast -fomit-frame-pointer     -fpermissive
>> -DSPEC_LP64 source/numerics/matrices.all_dimensions.cc
>>
>> source/numerics/matrices.all_dimensions.cc: In static member function
>> 'static void dealii::MatrixTools::apply_boundary_values(const
>> std::map<unsigned int, double>&, dealii::BlockSparseMatrix<number>&,
>> dealii::BlockVector<number>&, dealii::BlockVector<number>&, bool)':
>>
>> source/numerics/matrices.all_dimensions.cc:469:50: error: lvalue required as
>> unary '&' operand
>>
>>         [this_sparsity.get_rowstart_indices()[row]];
>>
>>                                                   ^
>>
>> source/numerics/matrices.all_dimensions.cc:472:55: error: lvalue required as
>> unary '&' operand
>>
>>            [this_sparsity.get_rowstart_indices()[row]+1],
>>
>>                                                        ^
>>
>> source/numerics/matrices.all_dimensions.cc:474:55: error: lvalue required as
>> unary '&' operand
>>
>>            [this_sparsity.get_rowstart_indices()[row+1]],
>>
>>                                                        ^
>>
>> source/numerics/matrices.all_dimensions.cc:479:49: error: lvalue required as
>> unary '&' operand
>>
>>        [this_sparsity.get_rowstart_indices()[row]],
>>
>>                                                  ^
>>
>> source/numerics/matrices.all_dimensions.cc:481:51: error: lvalue required as
>> unary '&' operand
>>
>>        [this_sparsity.get_rowstart_indices()[row+1]],
>>
>>                                                    ^
>>
>> source/numerics/matrices.all_dimensions.cc:510:50: error: lvalue required as
>> unary '&' operand
>>
>>           [this_sparsity.get_rowstart_indices()[0]]);
>>
>> Sudi
>
> Thanks, investigating.

Fixed thus.
commit 2c71bc15d22c4b831ff2e27ef7759de4dc78ef6c
Author: Jason Merrill <jason@redhat.com>
Date:   Fri May 25 16:25:10 2018 -0400

            CWG 616, 1213 - value category of subobject references.
    
            * tree.c (lvalue_kind): Fix handling of ARRAY_REF of pointer.

diff --git a/gcc/cp/tree.c b/gcc/cp/tree.c
index 9d978160292..f21daaca1d0 100644
--- a/gcc/cp/tree.c
+++ b/gcc/cp/tree.c
@@ -95,14 +95,24 @@ lvalue_kind (const_tree ref)
     case TRY_CATCH_EXPR:
     case REALPART_EXPR:
     case IMAGPART_EXPR:
-    case ARRAY_REF:
     case VIEW_CONVERT_EXPR:
-      op1_lvalue_kind = lvalue_kind (TREE_OPERAND (ref, 0));
-      if (op1_lvalue_kind == clk_class)
-	/* in the case of an array operand, the result is an lvalue if that
-	   operand is an lvalue and an xvalue otherwise */
-	op1_lvalue_kind = clk_rvalueref;
-      return op1_lvalue_kind;
+      return lvalue_kind (TREE_OPERAND (ref, 0));
+
+    case ARRAY_REF:
+      {
+	tree op1 = TREE_OPERAND (ref, 0);
+	if (TREE_CODE (TREE_TYPE (op1)) == ARRAY_TYPE)
+	  {
+	    op1_lvalue_kind = lvalue_kind (op1);
+	    if (op1_lvalue_kind == clk_class)
+	      /* in the case of an array operand, the result is an lvalue if
+		 that operand is an lvalue and an xvalue otherwise */
+	      op1_lvalue_kind = clk_rvalueref;
+	    return op1_lvalue_kind;
+	  }
+	else
+	  return clk_ordinary;
+      }
 
     case MEMBER_REF:
     case DOTSTAR_EXPR:
diff --git a/gcc/testsuite/g++.dg/template/array31.C b/gcc/testsuite/g++.dg/template/array31.C
new file mode 100644
index 00000000000..007d0dccf89
--- /dev/null
+++ b/gcc/testsuite/g++.dg/template/array31.C
@@ -0,0 +1,7 @@
+int *g();
+
+template <class T> 
+void f(int i)
+{
+  int *p = &g()[3];
+}
Sudakshina Das May 29, 2018, 9:28 a.m. UTC | #4
Hi Jason

On 25/05/18 21:55, Jason Merrill wrote:
> On Fri, May 25, 2018 at 4:08 PM, Jason Merrill <jason@redhat.com> wrote:
>> On Fri, May 25, 2018 at 12:40 PM, Sudakshina Das <sudi.das@arm.com> wrote:
>>> On 23/05/18 18:21, Jason Merrill wrote:
>>>>
>>>> The first patch implements the adjustments from core issues 616 and
>>>> 1213 to the value category of subobjects of class prvalues: they were
>>>> considered prvalues themselves, but that was kind of nonsensical.  Now
>>>> they are considered xvalues.  Along with this, I've removed the
>>>> diagnostic distinction between xvalues and prvalues when trying to use
>>>> one or the other as an lvalue; the important thing is that they are
>>>> rvalues.
>>>>
>>>> The second patch corrects various issues with casts and xvalues/rvalue
>>>> references: we were treating an xvalue operand to dynamic_cast as an
>>>> lvalue, and we were objecting to casts from prvalue to rvalue
>>>> reference type.
>>>>
>>>
>>> With the second patch:
>>> commit f7d2790049fd1e59af4b69ee12f7c101cfe4cdab
>>> Author: jason <jason@138bc75d-0d04-0410-961f-82ee72b054a4>
>>> Date:   Wed May 23 17:21:39 2018 +0000
>>>
>>>          Fix cast to rvalue reference from prvalue.
>>>
>>>          * cvt.c (diagnose_ref_binding): Handle rvalue reference.
>>>          * rtti.c (build_dynamic_cast_1): Don't try to build a reference to
>>>          non-class type.  Handle xvalue argument.
>>>          * typeck.c (build_reinterpret_cast_1): Allow cast from prvalue to
>>>          rvalue reference.
>>>          * semantics.c (finish_compound_literal): Do direct-initialization,
>>>          not cast, to initialize a reference.
>>>
>>>      git-svn-id: svn+ssh://gcc.gnu.org/svn/gcc/trunk@260622
>>> 138bc75d-0d04-0410-961f-82ee72b054a4
>>>
>>> I have observed the following failure in Spec2017 while building
>>> 510.parest_r on aarch64-none-linux-gnu
>>>
>>> aarch64-none-linux-gnu-g++ -c -o source/numerics/matrices.all_dimensions.o
>>> -DSPEC -DNDEBUG -Iinclude -I. -DSPEC_AUTO_SUPPRESS_OPENMP
>>> -mcpu=cortex-a57+crypto -Ofast -fomit-frame-pointer     -fpermissive
>>> -DSPEC_LP64 source/numerics/matrices.all_dimensions.cc
>>>
>>> source/numerics/matrices.all_dimensions.cc: In static member function
>>> 'static void dealii::MatrixTools::apply_boundary_values(const
>>> std::map<unsigned int, double>&, dealii::BlockSparseMatrix<number>&,
>>> dealii::BlockVector<number>&, dealii::BlockVector<number>&, bool)':
>>>
>>> source/numerics/matrices.all_dimensions.cc:469:50: error: lvalue required as
>>> unary '&' operand
>>>
>>>          [this_sparsity.get_rowstart_indices()[row]];
>>>
>>>                                                    ^
>>>
>>> source/numerics/matrices.all_dimensions.cc:472:55: error: lvalue required as
>>> unary '&' operand
>>>
>>>             [this_sparsity.get_rowstart_indices()[row]+1],
>>>
>>>                                                         ^
>>>
>>> source/numerics/matrices.all_dimensions.cc:474:55: error: lvalue required as
>>> unary '&' operand
>>>
>>>             [this_sparsity.get_rowstart_indices()[row+1]],
>>>
>>>                                                         ^
>>>
>>> source/numerics/matrices.all_dimensions.cc:479:49: error: lvalue required as
>>> unary '&' operand
>>>
>>>         [this_sparsity.get_rowstart_indices()[row]],
>>>
>>>                                                   ^
>>>
>>> source/numerics/matrices.all_dimensions.cc:481:51: error: lvalue required as
>>> unary '&' operand
>>>
>>>         [this_sparsity.get_rowstart_indices()[row+1]],
>>>
>>>                                                     ^
>>>
>>> source/numerics/matrices.all_dimensions.cc:510:50: error: lvalue required as
>>> unary '&' operand
>>>
>>>            [this_sparsity.get_rowstart_indices()[0]]);
>>>
>>> Sudi
>>
>> Thanks, investigating.
> 
> Fixed thus.

Thanks for fixing this!

Sudi
>
diff mbox series

Patch

commit 23a88cb6641f097dd0a9faebfafbe2f1e876714b
Author: Jason Merrill <jason@redhat.com>
Date:   Thu May 3 08:39:26 2018 -0400

            Fix cast to rvalue reference from prvalue.
    
            * cvt.c (diagnose_ref_binding): Handle rvalue reference.
            * rtti.c (build_dynamic_cast_1): Don't try to build a reference to
            non-class type.  Handle xvalue argument.
            * typeck.c (build_reinterpret_cast_1): Allow cast from prvalue to
            rvalue reference.
            * semantics.c (finish_compound_literal): Do direct-initialization,
            not cast, to initialize a reference.

diff --git a/gcc/cp/cvt.c b/gcc/cp/cvt.c
index 30b44b7d7ea..3f87317a47d 100644
--- a/gcc/cp/cvt.c
+++ b/gcc/cp/cvt.c
@@ -381,7 +381,8 @@  diagnose_ref_binding (location_t loc, tree reftype, tree intype, tree decl)
 {
   tree ttl = TREE_TYPE (reftype);
 
-  if (!CP_TYPE_CONST_NON_VOLATILE_P (ttl))
+  if (!TYPE_REF_IS_RVALUE (reftype)
+      && !CP_TYPE_CONST_NON_VOLATILE_P (ttl))
     {
       const char *msg;
 
diff --git a/gcc/cp/rtti.c b/gcc/cp/rtti.c
index 426a23276e0..6692fb7ff86 100644
--- a/gcc/cp/rtti.c
+++ b/gcc/cp/rtti.c
@@ -616,22 +616,22 @@  build_dynamic_cast_1 (tree type, tree expr, tsubst_flags_t complain)
   else
     {
       expr = mark_lvalue_use (expr);
-
-      exprtype = build_reference_type (TREE_TYPE (expr));
+      exprtype = TREE_TYPE (expr);
 
       /* T is a reference type, v shall be an lvalue of a complete class
 	 type, and the result is an lvalue of the type referred to by T.  */
-
-      if (! MAYBE_CLASS_TYPE_P (TREE_TYPE (exprtype)))
+      if (! MAYBE_CLASS_TYPE_P (exprtype))
 	{
 	  errstr = _("source is not of class type");
 	  goto fail;
 	}
-      if (!COMPLETE_TYPE_P (complete_type (TREE_TYPE (exprtype))))
+      if (!COMPLETE_TYPE_P (complete_type (exprtype)))
 	{
 	  errstr = _("source is of incomplete class type");
 	  goto fail;
 	}
+
+      exprtype = cp_build_reference_type (exprtype, !lvalue_p (expr));
     }
 
   /* The dynamic_cast operator shall not cast away constness.  */
diff --git a/gcc/cp/semantics.c b/gcc/cp/semantics.c
index 94e8f54254d..46251deaa6c 100644
--- a/gcc/cp/semantics.c
+++ b/gcc/cp/semantics.c
@@ -2734,7 +2734,10 @@  finish_compound_literal (tree type, tree compound_literal,
       compound_literal
 	= finish_compound_literal (TREE_TYPE (type), compound_literal,
 				   complain, fcl_context);
-      return cp_build_c_cast (type, compound_literal, complain);
+      /* The prvalue is then used to direct-initialize the reference.  */
+      tree r = (perform_implicit_conversion_flags
+		(type, compound_literal, complain, LOOKUP_NORMAL));
+      return convert_from_reference (r);
     }
 
   if (!TYPE_OBJ_P (type))
diff --git a/gcc/cp/typeck.c b/gcc/cp/typeck.c
index 82089c45105..a694499190f 100644
--- a/gcc/cp/typeck.c
+++ b/gcc/cp/typeck.c
@@ -7315,13 +7315,19 @@  build_reinterpret_cast_1 (tree type, tree expr, bool c_cast_p,
     type = cv_unqualified (type);
 
   /* [expr.reinterpret.cast]
-     An lvalue expression of type T1 can be cast to the type
+     A glvalue expression of type T1 can be cast to the type
      "reference to T2" if an expression of type "pointer to T1" can be
      explicitly converted to the type "pointer to T2" using a
      reinterpret_cast.  */
   if (TYPE_REF_P (type))
     {
-      if (! lvalue_p (expr))
+      if (TYPE_REF_IS_RVALUE (type))
+	{
+	  if (!obvalue_p (expr))
+	    /* Perform the temporary materialization conversion.  */
+	    expr = get_target_expr_sfinae (expr, complain);
+	}
+      else if (!lvalue_p (expr))
 	{
           if (complain & tf_error)
             error ("invalid cast of an rvalue expression of type "
diff --git a/gcc/testsuite/g++.dg/cpp0x/rv-cast6.C b/gcc/testsuite/g++.dg/cpp0x/rv-cast6.C
new file mode 100644
index 00000000000..3ae5691c5fd
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp0x/rv-cast6.C
@@ -0,0 +1,11 @@ 
+// Test that a prvalue can be used where a glvalue is expected.
+// { dg-do compile { target c++11 } }
+
+struct A { virtual void f(); };
+struct B : A {};
+
+auto && a = static_cast<A&&>(B());
+auto && b = reinterpret_cast<A&&>(B());
+auto && c = dynamic_cast<A&&>(B());
+auto && d = dynamic_cast<B&&>(static_cast<A&&>(B()));
+auto && e = const_cast<B&&>(B());