Re: [PATCH] Accept pmf-vbit-in-delta extra warning

[Alexandre Oliva <oliva@adacore.com>]

On Feb 17, 2023, Jason Merrill <jason@redhat.com> wrote:

> On 2/17/23 23:02, Alexandre Oliva wrote:
>> 
>> cp_build_binary_op, that issues -Waddress warnings, issues an extra
>> warning on arm targets, that g++.dg/warn/Waddress-5.C does not expect
>> when comparing a pointer-to-member-function literal with null.
>> 
>> The reason for the extra warning is that, on arm targets,
>> TARGET_PTRMEMFUNC_VBIT_LOCATION == ptrmemfunc_vbit_in_delta, which
>> causes a different path to be taken, that extracts the
>> pointer-to-function and the delta fields (minus the vbit) and compares
>> each one with zero.  It's when comparing this pointer-to-function with
>> zero, in a recursive cp_build_binary_op, that another warning is
>> issued.
>> 
>> I suppose there should be a way to skip the warning in this recursive
>> call, without disabling other warnings that might be issued there, but

> warning_sentinel ws (warn_address)

Oh, yeah, that will suppress the expected warning for pfn0, but isn't
there any risk whatsoever that it could suppress other -Waddress
warnings for tree operands of pfn0?

I see the cp_save_expr for side effects, but what if e.g. the pmfn we're
testing is an array element, and the index expression tests another pmfn
against NULL that should be warned about?  Or something else that
wouldn't have TREE_SIDE_EFFECTS, and would thus not go through
cp_save_expr.  Would we then warn for uses of both pfn0 and delta0?


Here's what I'm going to test for these concerns.  Ok to install if it
bootstraps successfully, and my concerns are unfounded?


[c++] suppress redundant null-addr warn in pfn from pmfn

From: Alexandre Oliva <oliva@adacore.com>

When TARGET_PTRMEMFUNC_VBIT_LOCATION == ptrmemfunc_vbit_in_delta, when
we warn about comparing a pointer-to-member-function with NULL, we
also warn about comparing the pointer-to-function extracted from it
with NULL, which is redundant.  Suppress the redundant warning.


for  gcc/cp/ChangeLog

	* typeck.cc (cp_build_binary_op): Suppress redundant warning
	for pfn null test in pmfn test with vbit-in-delta.
---
 gcc/cp/typeck.cc |   17 ++++++++++++-----
 1 file changed, 12 insertions(+), 5 deletions(-)

diff --git a/gcc/cp/typeck.cc b/gcc/cp/typeck.cc
index 4afb5e4f0d420..d5a3e501d8e91 100644
--- a/gcc/cp/typeck.cc
+++ b/gcc/cp/typeck.cc
@@ -5780,11 +5780,18 @@ cp_build_binary_op (const op_location_t &location,
 
 	      pfn0 = pfn_from_ptrmemfunc (op0);
 	      delta0 = delta_from_ptrmemfunc (op0);
-	      e1 = cp_build_binary_op (location,
-				       EQ_EXPR,
-	  			       pfn0,
-				       build_zero_cst (TREE_TYPE (pfn0)),
-				       complain);
+	      {
+		/* If we will warn below about a null-address compare
+		   involving the orig_op0 ptrmemfunc, we'd likely also
+		   warn about the pfn0's null-address compare, and
+		   that would be redundant, so suppress it.  */
+		warning_sentinel ws (warn_address);
+		e1 = cp_build_binary_op (location,
+					 EQ_EXPR,
+					 pfn0,
+					 build_zero_cst (TREE_TYPE (pfn0)),
+					 complain);
+	      }
 	      e2 = cp_build_binary_op (location,
 				       BIT_AND_EXPR,
 				       delta0,


-- 
Alexandre Oliva, happy hacker                https://FSFLA.org/blogs/lxo/
   Free Software Activist                       GNU Toolchain Engineer
Disinformation flourishes because many people care deeply about injustice
but very few check the facts.  Ask me about <https://stallmansupport.org>