[PATCH] c++: Only reject reinterpret casts from pointers to integers for manifestly_const_eval evaluation [PR99456]

[PATCH] c++: Only reject reinterpret casts from pointers to integers for manifestly_const_eval evaluation [PR99456]

[Jakub Jelinek]

Hi!

My PR82304/PR95307 fix moved reinterpret cast from pointer to integer
diagnostics from cxx_eval_outermost_constant_expr where it caught
invalid code only at the outermost level down into
cxx_eval_constant_expression.
Unfortunately, it regressed following testcase, we emit worse code
including dynamic initialization of some vars.
While the initializers are not constant expressions due to the
reinterpret_cast in there, there is no reason not to fold them as an
optimization.

I've tried to make this dependent on !ctx->quiet, but that regressed
two further tests, so this patch bases that on manifestly_const_eval.

The new testcase is now optimized as much as it used to be in GCC 10
and the only regression it causes is an extra -Wnarrowing warning
on vla22.C test on invalid code (which the patch adjusts).

Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

2021-03-09  Jakub Jelinek  <jakub@redhat.com>

	PR c++/99456
	* constexpr.c (cxx_eval_constant_expression): For CONVERT_EXPR from
	INDIRECT_TYPE_P to ARITHMETIC_TYPE_P, when !ctx->manifestly_const_eval
	don't diagnose it, set *non_constant_p nor return t.

	* g++.dg/opt/pr99456.C: New test.
	* g++.dg/ext/vla22.C: Expect a -Wnarrowing warning for c++11 and
	later.

--- gcc/cp/constexpr.c.jj	2021-03-08 23:40:28.334509562 +0100
+++ gcc/cp/constexpr.c	2021-03-09 11:50:08.721716460 +0100
@@ -6656,7 +6656,8 @@ cxx_eval_constant_expression (const cons
 
 	if (TREE_CODE (t) == CONVERT_EXPR
 	    && ARITHMETIC_TYPE_P (type)
-	    && INDIRECT_TYPE_P (TREE_TYPE (op)))
+	    && INDIRECT_TYPE_P (TREE_TYPE (op))
+	    && ctx->manifestly_const_eval)
 	  {
 	    if (!ctx->quiet)
 	      error_at (loc,
--- gcc/testsuite/g++.dg/opt/pr99456.C.jj	2021-03-09 11:43:56.452862770 +0100
+++ gcc/testsuite/g++.dg/opt/pr99456.C	2021-03-09 11:43:56.452862770 +0100
@@ -0,0 +1,33 @@
+// PR c++/99456
+// { dg-do compile { target c++17 } }
+// { dg-options "-g0" }
+// { dg-final { scan-assembler-not "PR99456Var0\[1234]" } }
+// { dg-final { scan-assembler-not "__static_initialization_and_destruction" } }
+// { dg-final { scan-assembler-not "_GLOBAL__sub_I" } }
+// { dg-final { scan-assembler-not "_ZGV12PR99456Var1\[1234]" } }
+
+typedef __UINTPTR_TYPE__ uintptr_t;
+
+class Container
+{
+public:
+  uintptr_t m;
+};
+
+extern unsigned desc;
+static constexpr unsigned &descRef = desc;
+
+inline Container PR99456Var01 {reinterpret_cast<uintptr_t> (&descRef)};
+inline Container PR99456Var02 {reinterpret_cast<uintptr_t> (&desc)};
+inline uintptr_t PR99456Var03 {reinterpret_cast<uintptr_t> (&descRef)};
+inline uintptr_t PR99456Var04 {reinterpret_cast<uintptr_t> (&desc)};
+
+inline Container PR99456Var11 {reinterpret_cast<uintptr_t> (&descRef)};
+inline Container PR99456Var12 {reinterpret_cast<uintptr_t> (&desc)};
+inline uintptr_t PR99456Var13 {reinterpret_cast<uintptr_t> (&descRef)};
+inline uintptr_t PR99456Var14 {reinterpret_cast<uintptr_t> (&desc)};
+
+auto *PR99456Ref11 = &PR99456Var11;
+auto *PR99456Ref12 = &PR99456Var12;
+auto *PR99456Ref13 = &PR99456Var13;
+auto *PR99456Ref14 = &PR99456Var14;
--- gcc/testsuite/g++.dg/ext/vla22.C.jj	2020-02-27 09:28:46.396956140 +0100
+++ gcc/testsuite/g++.dg/ext/vla22.C	2021-03-09 12:00:58.275482884 +0100
@@ -6,4 +6,4 @@ void
 f ()
 {
   const int tbl[(long) "h"] = { 12 }; // { dg-error "size of array .tbl. is not an integral constant-expression" }
-}
+}				      // { dg-warning "narrowing conversion" "" { target c++11 } .-1 }

	Jakub

Re: [PATCH] c++: Only reject reinterpret casts from pointers to integers for manifestly_const_eval evaluation [PR99456]

[Nathan Sidwell]

On 3/9/21 10:31 AM, Jakub Jelinek wrote:
> Hi!
> 
> My PR82304/PR95307 fix moved reinterpret cast from pointer to integer
> diagnostics from cxx_eval_outermost_constant_expr where it caught
> invalid code only at the outermost level down into
> cxx_eval_constant_expression.
> Unfortunately, it regressed following testcase, we emit worse code
> including dynamic initialization of some vars.
> While the initializers are not constant expressions due to the
> reinterpret_cast in there, there is no reason not to fold them as an
> optimization.
> 
> I've tried to make this dependent on !ctx->quiet, but that regressed
> two further tests, so this patch bases that on manifestly_const_eval.
> 
> The new testcase is now optimized as much as it used to be in GCC 10
> and the only regression it causes is an extra -Wnarrowing warning
> on vla22.C test on invalid code (which the patch adjusts).
> 
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

good for me, but I think Jason should also comment.  Weaving both 
consteval semantics and the need for sym+addend folding is tricky.

for avoidance of doubt, as mentioned in the PR, the inline variable case 
is an ABI issue, not 'just nice'.  I guess the ABI document should 
specify. It doesn't appear to. 
https://github.com/itanium-cxx-abi/cxx-abi/issues/121 created.

> 
> 2021-03-09  Jakub Jelinek  <jakub@redhat.com>
> 
> 	PR c++/99456
> 	* constexpr.c (cxx_eval_constant_expression): For CONVERT_EXPR from
> 	INDIRECT_TYPE_P to ARITHMETIC_TYPE_P, when !ctx->manifestly_const_eval
> 	don't diagnose it, set *non_constant_p nor return t.
> 
> 	* g++.dg/opt/pr99456.C: New test.
> 	* g++.dg/ext/vla22.C: Expect a -Wnarrowing warning for c++11 and
> 	later.
> 
> --- gcc/cp/constexpr.c.jj	2021-03-08 23:40:28.334509562 +0100
> +++ gcc/cp/constexpr.c	2021-03-09 11:50:08.721716460 +0100
> @@ -6656,7 +6656,8 @@ cxx_eval_constant_expression (const cons
>   
>   	if (TREE_CODE (t) == CONVERT_EXPR
>   	    && ARITHMETIC_TYPE_P (type)
> -	    && INDIRECT_TYPE_P (TREE_TYPE (op)))
> +	    && INDIRECT_TYPE_P (TREE_TYPE (op))
> +	    && ctx->manifestly_const_eval)
>   	  {
>   	    if (!ctx->quiet)
>   	      error_at (loc,
> --- gcc/testsuite/g++.dg/opt/pr99456.C.jj	2021-03-09 11:43:56.452862770 +0100
> +++ gcc/testsuite/g++.dg/opt/pr99456.C	2021-03-09 11:43:56.452862770 +0100
> @@ -0,0 +1,33 @@
> +// PR c++/99456
> +// { dg-do compile { target c++17 } }
> +// { dg-options "-g0" }
> +// { dg-final { scan-assembler-not "PR99456Var0\[1234]" } }
> +// { dg-final { scan-assembler-not "__static_initialization_and_destruction" } }
> +// { dg-final { scan-assembler-not "_GLOBAL__sub_I" } }
> +// { dg-final { scan-assembler-not "_ZGV12PR99456Var1\[1234]" } }
> +
> +typedef __UINTPTR_TYPE__ uintptr_t;
> +
> +class Container
> +{
> +public:
> +  uintptr_t m;
> +};
> +
> +extern unsigned desc;
> +static constexpr unsigned &descRef = desc;
> +
> +inline Container PR99456Var01 {reinterpret_cast<uintptr_t> (&descRef)};
> +inline Container PR99456Var02 {reinterpret_cast<uintptr_t> (&desc)};
> +inline uintptr_t PR99456Var03 {reinterpret_cast<uintptr_t> (&descRef)};
> +inline uintptr_t PR99456Var04 {reinterpret_cast<uintptr_t> (&desc)};
> +
> +inline Container PR99456Var11 {reinterpret_cast<uintptr_t> (&descRef)};
> +inline Container PR99456Var12 {reinterpret_cast<uintptr_t> (&desc)};
> +inline uintptr_t PR99456Var13 {reinterpret_cast<uintptr_t> (&descRef)};
> +inline uintptr_t PR99456Var14 {reinterpret_cast<uintptr_t> (&desc)};
> +
> +auto *PR99456Ref11 = &PR99456Var11;
> +auto *PR99456Ref12 = &PR99456Var12;
> +auto *PR99456Ref13 = &PR99456Var13;
> +auto *PR99456Ref14 = &PR99456Var14;
> --- gcc/testsuite/g++.dg/ext/vla22.C.jj	2020-02-27 09:28:46.396956140 +0100
> +++ gcc/testsuite/g++.dg/ext/vla22.C	2021-03-09 12:00:58.275482884 +0100
> @@ -6,4 +6,4 @@ void
>   f ()
>   {
>     const int tbl[(long) "h"] = { 12 }; // { dg-error "size of array .tbl. is not an integral constant-expression" }
> -}
> +}				      // { dg-warning "narrowing conversion" "" { target c++11 } .-1 }
> 
> 	Jakub
> 


-- 
Nathan Sidwell

Re: [PATCH] c++: Only reject reinterpret casts from pointers to integers for manifestly_const_eval evaluation [PR99456]

[Jason Merrill]

On 3/9/21 10:31 AM, Jakub Jelinek wrote:
> Hi!
> 
> My PR82304/PR95307 fix moved reinterpret cast from pointer to integer
> diagnostics from cxx_eval_outermost_constant_expr where it caught
> invalid code only at the outermost level down into
> cxx_eval_constant_expression.
> Unfortunately, it regressed following testcase, we emit worse code
> including dynamic initialization of some vars.
> While the initializers are not constant expressions due to the
> reinterpret_cast in there, there is no reason not to fold them as an
> optimization.
> 
> I've tried to make this dependent on !ctx->quiet, but that regressed
> two further tests, so this patch bases that on manifestly_const_eval.

Did you try using ctx->strict?

Though perhaps for GCC 12 the strict flag should be dropped entirely in 
favor of manifestly_const_eval.

> The new testcase is now optimized as much as it used to be in GCC 10
> and the only regression it causes is an extra -Wnarrowing warning
> on vla22.C test on invalid code (which the patch adjusts).
> 
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
> 
> 2021-03-09  Jakub Jelinek  <jakub@redhat.com>
> 
> 	PR c++/99456
> 	* constexpr.c (cxx_eval_constant_expression): For CONVERT_EXPR from
> 	INDIRECT_TYPE_P to ARITHMETIC_TYPE_P, when !ctx->manifestly_const_eval
> 	don't diagnose it, set *non_constant_p nor return t.
> 
> 	* g++.dg/opt/pr99456.C: New test.
> 	* g++.dg/ext/vla22.C: Expect a -Wnarrowing warning for c++11 and
> 	later.
> 
> --- gcc/cp/constexpr.c.jj	2021-03-08 23:40:28.334509562 +0100
> +++ gcc/cp/constexpr.c	2021-03-09 11:50:08.721716460 +0100
> @@ -6656,7 +6656,8 @@ cxx_eval_constant_expression (const cons
>   
>   	if (TREE_CODE (t) == CONVERT_EXPR
>   	    && ARITHMETIC_TYPE_P (type)
> -	    && INDIRECT_TYPE_P (TREE_TYPE (op)))
> +	    && INDIRECT_TYPE_P (TREE_TYPE (op))
> +	    && ctx->manifestly_const_eval)
>   	  {
>   	    if (!ctx->quiet)
>   	      error_at (loc,
> --- gcc/testsuite/g++.dg/opt/pr99456.C.jj	2021-03-09 11:43:56.452862770 +0100
> +++ gcc/testsuite/g++.dg/opt/pr99456.C	2021-03-09 11:43:56.452862770 +0100
> @@ -0,0 +1,33 @@
> +// PR c++/99456
> +// { dg-do compile { target c++17 } }
> +// { dg-options "-g0" }
> +// { dg-final { scan-assembler-not "PR99456Var0\[1234]" } }
> +// { dg-final { scan-assembler-not "__static_initialization_and_destruction" } }
> +// { dg-final { scan-assembler-not "_GLOBAL__sub_I" } }
> +// { dg-final { scan-assembler-not "_ZGV12PR99456Var1\[1234]" } }
> +
> +typedef __UINTPTR_TYPE__ uintptr_t;
> +
> +class Container
> +{
> +public:
> +  uintptr_t m;
> +};
> +
> +extern unsigned desc;
> +static constexpr unsigned &descRef = desc;
> +
> +inline Container PR99456Var01 {reinterpret_cast<uintptr_t> (&descRef)};
> +inline Container PR99456Var02 {reinterpret_cast<uintptr_t> (&desc)};
> +inline uintptr_t PR99456Var03 {reinterpret_cast<uintptr_t> (&descRef)};
> +inline uintptr_t PR99456Var04 {reinterpret_cast<uintptr_t> (&desc)};
> +
> +inline Container PR99456Var11 {reinterpret_cast<uintptr_t> (&descRef)};
> +inline Container PR99456Var12 {reinterpret_cast<uintptr_t> (&desc)};
> +inline uintptr_t PR99456Var13 {reinterpret_cast<uintptr_t> (&descRef)};
> +inline uintptr_t PR99456Var14 {reinterpret_cast<uintptr_t> (&desc)};
> +
> +auto *PR99456Ref11 = &PR99456Var11;
> +auto *PR99456Ref12 = &PR99456Var12;
> +auto *PR99456Ref13 = &PR99456Var13;
> +auto *PR99456Ref14 = &PR99456Var14;
> --- gcc/testsuite/g++.dg/ext/vla22.C.jj	2020-02-27 09:28:46.396956140 +0100
> +++ gcc/testsuite/g++.dg/ext/vla22.C	2021-03-09 12:00:58.275482884 +0100
> @@ -6,4 +6,4 @@ void
>   f ()
>   {
>     const int tbl[(long) "h"] = { 12 }; // { dg-error "size of array .tbl. is not an integral constant-expression" }
> -}
> +}				      // { dg-warning "narrowing conversion" "" { target c++11 } .-1 }
> 
> 	Jakub
> 

Re: [PATCH] c++: Only reject reinterpret casts from pointers to integers for manifestly_const_eval evaluation [PR99456]

[Jakub Jelinek]

On Thu, Mar 18, 2021 at 05:20:54PM -0400, Jason Merrill wrote:
> On 3/9/21 10:31 AM, Jakub Jelinek wrote:
> > Hi!
> > 
> > My PR82304/PR95307 fix moved reinterpret cast from pointer to integer
> > diagnostics from cxx_eval_outermost_constant_expr where it caught
> > invalid code only at the outermost level down into
> > cxx_eval_constant_expression.
> > Unfortunately, it regressed following testcase, we emit worse code
> > including dynamic initialization of some vars.
> > While the initializers are not constant expressions due to the
> > reinterpret_cast in there, there is no reason not to fold them as an
> > optimization.
> > 
> > I've tried to make this dependent on !ctx->quiet, but that regressed
> > two further tests, so this patch bases that on manifestly_const_eval.
> 
> Did you try using ctx->strict?

Tried that now, it seems to be even worse than the original && !ctx->quiet
approach, regresses:
FAIL: g++.dg/cpp0x/constexpr-95307.C  -std=c++11  (test for errors, line 5)
FAIL: g++.dg/cpp0x/constexpr-95307.C  -std=c++14  (test for errors, line 5)
FAIL: g++.dg/cpp0x/constexpr-95307.C  -std=c++17  (test for errors, line 5)
FAIL: g++.dg/cpp0x/constexpr-95307.C  -std=c++2a  (test for errors, line 5)
FAIL: g++.dg/cpp0x/constexpr-ex1.C  -std=c++11  (test for errors, line 73)
FAIL: g++.dg/cpp0x/constexpr-ex1.C  -std=c++14  (test for errors, line 73)
FAIL: g++.dg/cpp0x/constexpr-ex1.C  -std=c++17  (test for errors, line 73)
FAIL: g++.dg/cpp0x/constexpr-ex1.C  -std=c++2a  (test for errors, line 73)
FAIL: g++.dg/cpp1y/constexpr-82304.C  -std=c++14  (test for errors, line 9)
FAIL: g++.dg/cpp1y/constexpr-82304.C  -std=c++17  (test for errors, line 9)
FAIL: g++.dg/cpp1y/constexpr-82304.C  -std=c++2a  (test for errors, line 9)
FAIL: g++.dg/cpp1y/constexpr-shift1.C  -std=c++14  (test for errors, line 6)
FAIL: g++.dg/cpp1y/constexpr-shift1.C  -std=c++14 (test for excess errors)
FAIL: g++.dg/cpp1y/constexpr-shift1.C  -std=c++17  (test for errors, line 6)
FAIL: g++.dg/cpp1y/constexpr-shift1.C  -std=c++17 (test for excess errors)
FAIL: g++.dg/cpp1y/constexpr-shift1.C  -std=c++2a  (test for errors, line 6)
FAIL: g++.dg/cpp1y/constexpr-shift1.C  -std=c++2a (test for excess errors)

	Jakub

Re: [PATCH] c++: Only reject reinterpret casts from pointers to integers for manifestly_const_eval evaluation [PR99456]

[Jason Merrill]

On 3/19/21 5:03 AM, Jakub Jelinek wrote:
> On Thu, Mar 18, 2021 at 05:20:54PM -0400, Jason Merrill wrote:
>> On 3/9/21 10:31 AM, Jakub Jelinek wrote:
>>> Hi!
>>>
>>> My PR82304/PR95307 fix moved reinterpret cast from pointer to integer
>>> diagnostics from cxx_eval_outermost_constant_expr where it caught
>>> invalid code only at the outermost level down into
>>> cxx_eval_constant_expression.
>>> Unfortunately, it regressed following testcase, we emit worse code
>>> including dynamic initialization of some vars.
>>> While the initializers are not constant expressions due to the
>>> reinterpret_cast in there, there is no reason not to fold them as an
>>> optimization.
>>>
>>> I've tried to make this dependent on !ctx->quiet, but that regressed
>>> two further tests, so this patch bases that on manifestly_const_eval.
>>
>> Did you try using ctx->strict?
> 
> Tried that now, it seems to be even worse than the original && !ctx->quiet
> approach, regresses:
> FAIL: g++.dg/cpp0x/constexpr-95307.C  -std=c++11  (test for errors, line 5)
> FAIL: g++.dg/cpp0x/constexpr-95307.C  -std=c++14  (test for errors, line 5)
> FAIL: g++.dg/cpp0x/constexpr-95307.C  -std=c++17  (test for errors, line 5)
> FAIL: g++.dg/cpp0x/constexpr-95307.C  -std=c++2a  (test for errors, line 5)
> FAIL: g++.dg/cpp0x/constexpr-ex1.C  -std=c++11  (test for errors, line 73)
> FAIL: g++.dg/cpp0x/constexpr-ex1.C  -std=c++14  (test for errors, line 73)
> FAIL: g++.dg/cpp0x/constexpr-ex1.C  -std=c++17  (test for errors, line 73)
> FAIL: g++.dg/cpp0x/constexpr-ex1.C  -std=c++2a  (test for errors, line 73)
> FAIL: g++.dg/cpp1y/constexpr-82304.C  -std=c++14  (test for errors, line 9)
> FAIL: g++.dg/cpp1y/constexpr-82304.C  -std=c++17  (test for errors, line 9)
> FAIL: g++.dg/cpp1y/constexpr-82304.C  -std=c++2a  (test for errors, line 9)
> FAIL: g++.dg/cpp1y/constexpr-shift1.C  -std=c++14  (test for errors, line 6)
> FAIL: g++.dg/cpp1y/constexpr-shift1.C  -std=c++14 (test for excess errors)
> FAIL: g++.dg/cpp1y/constexpr-shift1.C  -std=c++17  (test for errors, line 6)
> FAIL: g++.dg/cpp1y/constexpr-shift1.C  -std=c++17 (test for excess errors)
> FAIL: g++.dg/cpp1y/constexpr-shift1.C  -std=c++2a  (test for errors, line 6)
> FAIL: g++.dg/cpp1y/constexpr-shift1.C  -std=c++2a (test for excess errors)

Then the patch is OK, thanks.

Jason