diff mbox

PR c++/80544 strip cv-quals from cast results

Message ID 20170427165907.GL5109@redhat.com
State New
Headers show

Commit Message

Jonathan Wakely April 27, 2017, 4:59 p.m. UTC 
This is probably not the best way to do this, but it seems to work.

I also tried to add a warning like EDG's (see the PR) but it gave a
false positive for direct-list-init of scoped enums (P0138R2, r240449)
because that code goes through build_c_cast to perform the conversion,
so looks like a cast to my new warning.

Tested x86_64-linux, OK for trunk?


gcc/cp:

	PR c++/80544
	* typeck.c (build_static_cast_1, build_reinterpret_cast_1)
	(build_const_cast_1): Strip cv-quals from non-class prvalue results.

gcc/testsuite:

	PR c++/80544
	* g++.dg/expr/cast11.C: New test.

Comments

Paolo Carlini April 27, 2017, 6:41 p.m. UTC | #1 
Hi,

On 27/04/2017 18:59, Jonathan Wakely wrote:
> This is probably not the best way to do this, but it seems to work.
Eventually, if this is the way to go, a small maybe_strip_* helper could 
tidy a bit the code...

Paolo.
Jonathan Wakely May 18, 2017, 5:40 p.m. UTC | #2 
Ping for https://gcc.gnu.org/ml/gcc-patches/2017-04/msg01414.html ...

On 27/04/17 17:59 +0100, Jonathan Wakely wrote:
>This is probably not the best way to do this, but it seems to work.
>
>I also tried to add a warning like EDG's (see the PR) but it gave a
>false positive for direct-list-init of scoped enums (P0138R2, r240449)
>because that code goes through build_c_cast to perform the conversion,
>so looks like a cast to my new warning.
>
>Tested x86_64-linux, OK for trunk?
>
>
>gcc/cp:
>
>	PR c++/80544
>	* typeck.c (build_static_cast_1, build_reinterpret_cast_1)
>	(build_const_cast_1): Strip cv-quals from non-class prvalue results.
>
>gcc/testsuite:
>
>	PR c++/80544
>	* g++.dg/expr/cast11.C: New test.
>
>

>commit 47763f3c84de86dd1ebbaac73e341e2de9b5be68
>Author: Jonathan Wakely <jwakely@redhat.com>
>Date:   Thu Apr 27 16:49:25 2017 +0100
>
>    PR c++/80544 strip cv-quals from cast results
>    
>    gcc/cp:
>    
>    	PR c++/80544
>    	* typeck.c (build_static_cast_1, build_reinterpret_cast_1)
>    	(build_const_cast_1): Strip cv-quals from non-class prvalue results.
>    
>    gcc/testsuite:
>    
>    	PR c++/80544
>    	* g++.dg/expr/cast11.C: New test.
>
>diff --git a/gcc/cp/typeck.c b/gcc/cp/typeck.c
>index 7aee0d6..e41b335 100644
>--- a/gcc/cp/typeck.c
>+++ b/gcc/cp/typeck.c
>@@ -6708,6 +6708,10 @@ build_static_cast_1 (tree type, tree expr, bool c_cast_p,
>   /* Save casted types in the function's used types hash table.  */
>   used_types_insert (type);
> 
>+  /* A prvalue of non-class type is cv-unqualified.  */
>+  if (TREE_CODE (type) != REFERENCE_TYPE && !CLASS_TYPE_P (type))
>+    type = cv_unqualified (type);
>+
>   /* [expr.static.cast]
> 
>      An lvalue of type "cv1 B", where B is a class type, can be cast
>@@ -7070,6 +7074,10 @@ build_reinterpret_cast_1 (tree type, tree expr, bool c_cast_p,
>   /* Save casted types in the function's used types hash table.  */
>   used_types_insert (type);
> 
>+  /* A prvalue of non-class type is cv-unqualified.  */
>+  if (TREE_CODE (type) != REFERENCE_TYPE && !CLASS_TYPE_P (type))
>+    type = cv_unqualified (type);
>+
>   /* [expr.reinterpret.cast]
>      An lvalue expression of type T1 can be cast to the type
>      "reference to T2" if an expression of type "pointer to T1" can be
>@@ -7300,6 +7308,10 @@ build_const_cast_1 (tree dst_type, tree expr, tsubst_flags_t complain,
>   /* Save casted types in the function's used types hash table.  */
>   used_types_insert (dst_type);
> 
>+  /* A prvalue of non-class type is cv-unqualified.  */
>+  if (TREE_CODE (dst_type) != REFERENCE_TYPE && !CLASS_TYPE_P (dst_type))
>+    dst_type = cv_unqualified (dst_type);
>+
>   src_type = TREE_TYPE (expr);
>   /* Expressions do not really have reference types.  */
>   if (TREE_CODE (src_type) == REFERENCE_TYPE)
>diff --git a/gcc/testsuite/g++.dg/expr/cast11.C b/gcc/testsuite/g++.dg/expr/cast11.C
>new file mode 100644
>index 0000000..642087e
>--- /dev/null
>+++ b/gcc/testsuite/g++.dg/expr/cast11.C
>@@ -0,0 +1,34 @@
>+// { dg-do compile { target c++11 } }
>+// c++/80544 cast expressions returned cv-qualified prvalues
>+
>+template<typename T> void f(T&&) { }
>+template<typename T> void f(T const&&) = delete;
>+
>+template<typename T> void g(T&&) = delete;
>+template<typename T> void g(T const&&) { }
>+
>+struct B { int i; } b;
>+
>+void f1()
>+{
>+  int i = 0;
>+  f((long const)i);
>+  f((int* const)&i);
>+  f((int const* const)&i);
>+  f((long* const)&i);
>+
>+  f(static_cast<long const>(i));
>+  f(reinterpret_cast<long const>(&i));
>+
>+  f(static_cast<int* const>(&i));
>+  f(const_cast<int* const>(&i));
>+  f(reinterpret_cast<long* const>(&i));
>+
>+  using ptrmem = int B::*;
>+  f(static_cast<ptrmem const>(&B::i));
>+  f(const_cast<ptrmem const>(&B::i));
>+  f(reinterpret_cast<ptrmem const>(&B::i));
>+
>+  // prvalue of class type can have cv-quals:
>+  g(static_cast<const B>(b));
>+}
Nathan Sidwell May 18, 2017, 5:44 p.m. UTC | #3 
On 05/18/2017 01:40 PM, Jonathan Wakely wrote:

>> +  /* A prvalue of non-class type is cv-unqualified.  */
>> +  if (TREE_CODE (type) != REFERENCE_TYPE && !CLASS_TYPE_P (type))
>> +    type = cv_unqualified (type);
>> +

References can't be CV qualified, so the REFERENCE_TYPE check seems 
superfluous?

nathan
Jason Merrill May 19, 2017, 7:14 p.m. UTC | #4 
On Thu, Apr 27, 2017 at 12:59 PM, Jonathan Wakely <jwakely@redhat.com> wrote:
> I also tried to add a warning like EDG's (see the PR) but it gave a
> false positive for direct-list-init of scoped enums (P0138R2, r240449)
> because that code goes through build_c_cast to perform the conversion,
> so looks like a cast to my new warning.

The enum init code could strip the cv-quals when calling build_c_cast
to avoid the warning.

Jason
Jonathan Wakely May 23, 2017, 5:56 p.m. UTC | #5 
On 18/05/17 13:44 -0400, Nathan Sidwell wrote:
>On 05/18/2017 01:40 PM, Jonathan Wakely wrote:
>
>>>+  /* A prvalue of non-class type is cv-unqualified.  */
>>>+  if (TREE_CODE (type) != REFERENCE_TYPE && !CLASS_TYPE_P (type))
>>>+    type = cv_unqualified (type);
>>>+
>
>References can't be CV qualified, so the REFERENCE_TYPE check seems 
>superfluous?

True. I did it because that matches the semantics of the cast
according to the standard, but it isn't needed here. Is it worth
keeping anyway, to avoid a redundant call to cv_unqualified?

It also occurs to me that checking for !CLASS_TYPE_P (type) isn't
needed in build_const_cast_1 because you can't const_cast to a class
type, only reference or pointer types.
Nathan Sidwell May 23, 2017, 5:58 p.m. UTC | #6 
On 05/23/2017 01:56 PM, Jonathan Wakely wrote:
> On 18/05/17 13:44 -0400, Nathan Sidwell wrote:

>> References can't be CV qualified, so the REFERENCE_TYPE check seems 
>> superfluous?
> 
> True. I did it because that matches the semantics of the cast
> according to the standard, but it isn't needed here. Is it worth
> keeping anyway, to avoid a redundant call to cv_unqualified?

I don't think it's worth checking.

> It also occurs to me that checking for !CLASS_TYPE_P (type) isn't
> needed in build_const_cast_1 because you can't const_cast to a class
> type, only reference or pointer types.

true.

nathan
Jonathan Wakely May 23, 2017, 6 p.m. UTC | #7 
On 19/05/17 15:14 -0400, Jason Merrill wrote:
>On Thu, Apr 27, 2017 at 12:59 PM, Jonathan Wakely <jwakely@redhat.com> wrote:
>> I also tried to add a warning like EDG's (see the PR) but it gave a
>> false positive for direct-list-init of scoped enums (P0138R2, r240449)
>> because that code goes through build_c_cast to perform the conversion,
>> so looks like a cast to my new warning.
>
>The enum init code could strip the cv-quals when calling build_c_cast
>to avoid the warning.

Thanks, that works. I don't think this warning fits under any existing
option, so I'll add a new one. -Wuseless-cast-qual maybe? Or is that
too close to -Wuseless-cast and -Wcast-qual and would cause confusion?
Jonathan Wakely May 23, 2017, 6:11 p.m. UTC | #8 
On 23/05/17 13:58 -0400, Nathan Sidwell wrote:
>On 05/23/2017 01:56 PM, Jonathan Wakely wrote:
>>On 18/05/17 13:44 -0400, Nathan Sidwell wrote:
>
>>>References can't be CV qualified, so the REFERENCE_TYPE check 
>>>seems superfluous?
>>
>>True. I did it because that matches the semantics of the cast
>>according to the standard, but it isn't needed here. Is it worth
>>keeping anyway, to avoid a redundant call to cv_unqualified?
>
>I don't think it's worth checking.

Ah yes, cp_build_qualified_type_real returns early if there's nothing
to do. OK, I'll remove those checks. Thanks.
Jason Merrill May 23, 2017, 8:26 p.m. UTC | #9 
On Tue, May 23, 2017 at 2:00 PM, Jonathan Wakely <jwakely@redhat.com> wrote:
> On 19/05/17 15:14 -0400, Jason Merrill wrote:
>>
>> On Thu, Apr 27, 2017 at 12:59 PM, Jonathan Wakely <jwakely@redhat.com>
>> wrote:
>>>
>>> I also tried to add a warning like EDG's (see the PR) but it gave a
>>> false positive for direct-list-init of scoped enums (P0138R2, r240449)
>>> because that code goes through build_c_cast to perform the conversion,
>>> so looks like a cast to my new warning.
>>
>>
>> The enum init code could strip the cv-quals when calling build_c_cast
>> to avoid the warning.
>
>
> Thanks, that works. I don't think this warning fits under any existing
> option, so I'll add a new one. -Wuseless-cast-qual maybe? Or is that
> too close to -Wuseless-cast and -Wcast-qual and would cause confusion?

-Wcast-rvalue-qual ?

Jason
diff mbox

Patch

commit 47763f3c84de86dd1ebbaac73e341e2de9b5be68
Author: Jonathan Wakely <jwakely@redhat.com>
Date:   Thu Apr 27 16:49:25 2017 +0100

    PR c++/80544 strip cv-quals from cast results
    
    gcc/cp:
    
    	PR c++/80544
    	* typeck.c (build_static_cast_1, build_reinterpret_cast_1)
    	(build_const_cast_1): Strip cv-quals from non-class prvalue results.
    
    gcc/testsuite:
    
    	PR c++/80544
    	* g++.dg/expr/cast11.C: New test.

diff --git a/gcc/cp/typeck.c b/gcc/cp/typeck.c
index 7aee0d6..e41b335 100644
--- a/gcc/cp/typeck.c
+++ b/gcc/cp/typeck.c
@@ -6708,6 +6708,10 @@  build_static_cast_1 (tree type, tree expr, bool c_cast_p,
   /* Save casted types in the function's used types hash table.  */
   used_types_insert (type);
 
+  /* A prvalue of non-class type is cv-unqualified.  */
+  if (TREE_CODE (type) != REFERENCE_TYPE && !CLASS_TYPE_P (type))
+    type = cv_unqualified (type);
+
   /* [expr.static.cast]
 
      An lvalue of type "cv1 B", where B is a class type, can be cast
@@ -7070,6 +7074,10 @@  build_reinterpret_cast_1 (tree type, tree expr, bool c_cast_p,
   /* Save casted types in the function's used types hash table.  */
   used_types_insert (type);
 
+  /* A prvalue of non-class type is cv-unqualified.  */
+  if (TREE_CODE (type) != REFERENCE_TYPE && !CLASS_TYPE_P (type))
+    type = cv_unqualified (type);
+
   /* [expr.reinterpret.cast]
      An lvalue expression of type T1 can be cast to the type
      "reference to T2" if an expression of type "pointer to T1" can be
@@ -7300,6 +7308,10 @@  build_const_cast_1 (tree dst_type, tree expr, tsubst_flags_t complain,
   /* Save casted types in the function's used types hash table.  */
   used_types_insert (dst_type);
 
+  /* A prvalue of non-class type is cv-unqualified.  */
+  if (TREE_CODE (dst_type) != REFERENCE_TYPE && !CLASS_TYPE_P (dst_type))
+    dst_type = cv_unqualified (dst_type);
+
   src_type = TREE_TYPE (expr);
   /* Expressions do not really have reference types.  */
   if (TREE_CODE (src_type) == REFERENCE_TYPE)
diff --git a/gcc/testsuite/g++.dg/expr/cast11.C b/gcc/testsuite/g++.dg/expr/cast11.C
new file mode 100644
index 0000000..642087e
--- /dev/null
+++ b/gcc/testsuite/g++.dg/expr/cast11.C
@@ -0,0 +1,34 @@ 
+// { dg-do compile { target c++11 } }
+// c++/80544 cast expressions returned cv-qualified prvalues
+
+template<typename T> void f(T&&) { }
+template<typename T> void f(T const&&) = delete;
+
+template<typename T> void g(T&&) = delete;
+template<typename T> void g(T const&&) { }
+
+struct B { int i; } b;
+
+void f1()
+{
+  int i = 0;
+  f((long const)i);
+  f((int* const)&i);
+  f((int const* const)&i);
+  f((long* const)&i);
+
+  f(static_cast<long const>(i));
+  f(reinterpret_cast<long const>(&i));
+
+  f(static_cast<int* const>(&i));
+  f(const_cast<int* const>(&i));
+  f(reinterpret_cast<long* const>(&i));
+
+  using ptrmem = int B::*;
+  f(static_cast<ptrmem const>(&B::i));
+  f(const_cast<ptrmem const>(&B::i));
+  f(reinterpret_cast<ptrmem const>(&B::i));
+
+  // prvalue of class type can have cv-quals:
+  g(static_cast<const B>(b));
+}