diff mbox series

handle member references in -Waddress [PR96507]

Message ID de150fbb-ea88-cd2e-5893-8c59a1cba697@gmail.com
State New
Headers
Series handle member references in -Waddress [PR96507] |

Commit Message

Martin Sebor Nov. 22, 2021, 11 p.m. UTC 
  While going through old -Waddress bug reports to close after
the recent improvements to the warning I came across PR 96507
that points out that member references aren't handled.  Since
testing the address of a reference for equality to null is
in general diagnosed, this seems like an oversight worth fixing.
  Attached is a change to the C++ front end to diagnose member
references as well.

Tested on x86_64-linux.

Martin

Comments

Marek Polacek Nov. 22, 2021, 11:21 p.m. UTC | #1 
On Mon, Nov 22, 2021 at 04:00:56PM -0700, Martin Sebor via Gcc-patches wrote:
> While going through old -Waddress bug reports to close after
> the recent improvements to the warning I came across PR 96507
> that points out that member references aren't handled.  Since
> testing the address of a reference for equality to null is
> in general diagnosed, this seems like an oversight worth fixing.
>  Attached is a change to the C++ front end to diagnose member
> references as well.
> 
> Tested on x86_64-linux.
> 
> Martin

> Issue -Waddress also for reference members [PR96507].
> 
> Resolves:
> PR c++/96507 - missing -Waddress for member references
> 
> gcc/cp/ChangeLog:
> 
> 	PR c++/96507
> 	* typeck.c (warn_for_null_address): Handle reference members.
> 
> gcc/testsuite/ChangeLog:
> 
> 	PR c++/96507
> 	* g++.dg/warn/Waddress-8.C: New test.
> 
> diff --git a/gcc/cp/typeck.c b/gcc/cp/typeck.c
> index 58919aaf13e..694c53eef8a 100644
> --- a/gcc/cp/typeck.c
> +++ b/gcc/cp/typeck.c
> @@ -4676,15 +4676,21 @@ warn_for_null_address (location_t location, tree op, tsubst_flags_t complain)
>  		    "addition %qE and NULL", cop);
>        return;
>      }
> -  else if (CONVERT_EXPR_P (op)
> -      && TYPE_REF_P (TREE_TYPE (TREE_OPERAND (op, 0))))
> +  else if (CONVERT_EXPR_P (op))
>      {
> -      STRIP_NOPS (op);
> +      tree op0 = TREE_OPERAND (op, 0);
> +      if (TYPE_REF_P (TREE_TYPE (op0)))
> +	{

Isn't this just REFERENCE_REF_P?

> +	  STRIP_NOPS (op);
> +
> +	  if (TREE_CODE (op) == COMPONENT_REF)
> +	    op = TREE_OPERAND (op, 1);
>  
> -      if (DECL_P (op))
> -	warned = warning_at (location, OPT_Waddress,
> -			     "the compiler can assume that the address of "
> -			     "%qD will never be NULL", op);
> +	  if (DECL_P (op))
> +	    warned = warning_at (location, OPT_Waddress,
> +				 "the compiler can assume that the address of "
> +				 "%qD will never be NULL", op);
> +	}
>      }
>  
>    if (warned && DECL_P (op))
> diff --git a/gcc/testsuite/g++.dg/warn/Waddress-8.C b/gcc/testsuite/g++.dg/warn/Waddress-8.C
> new file mode 100644
> index 00000000000..797102d6be4
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/warn/Waddress-8.C
> @@ -0,0 +1,85 @@
> +/* PR c++/96507 - missing -Waddress for member references
> +   { dg-do compile }
> +   { dg-options "-Wall" } */
> +
> +typedef void F ();
> +
> +extern F 𝔢
> +extern int &eir;
> +
> +bool warn_ext_rfun ()
> +{
> +  return &efr != 0;           // { dg-warning "-Waddress" }
> +}
> +
> +bool warn_ext_rvar ()
> +{
> +  return &eir != 0;           // { dg-warning "-Waddress" }
> +}
> +
> +
> +bool warn_parm_rfun (F &rf)
> +{
> +  return &rf != 0;            // { dg-warning "-Waddress" }
> +}
> +
> +bool warn_parm_rvar (int &ir)
> +{
> +  return &ir != 0;            // { dg-warning "-Waddress" }
> +}
> +
> +// Comparing the address of a reference argument to null also triggers
> +// a -Wnonnull-compare (that seems like a bug, hence PR 103363).
> +// { dg-prune-output "-Wnonnull-compare" }
> +
> +
> +struct S
> +{
> +  F &fr;
> +  int &ir;
> +};
> +
> +extern S es, esa[];
> +
> +bool warn_ext_mem_rfun ()
> +{
> +  return &es.fr != 0;         // { dg-warning "-Waddress" }
> +}
> +
> +bool warn_ext_mem_rvar ()
> +{
> +  return &es.ir != 0;         // { dg-warning "-Waddress" }
> +}
> +
> +
> +bool warn_ext_arr_mem_rfun (int i)
> +{
> +  return &esa[i].fr != 0;     // { dg-warning "-Waddress" }
> +}
> +
> +bool warn_ext_arr_mem_rvar (int i)
> +{
> +  return &esa[i].ir != 0;     // { dg-warning "-Waddress" }
> +}
> +
> +
> +bool warn_parm_mem_rfun (S &s)
> +{
> +  return &s.fr != 0;          // { dg-warning "-Waddress" }
> +}
> +
> +bool warn_parm_mem_rvar (S &s)
> +{
> +  return &s.ir != 0;          // { dg-warning "-Waddress" }
> +}
> +
> +
> +bool warn_parm_arr_mem_rfun (S sa[], int i)
> +{
> +  return &sa[i].fr != 0;      // { dg-warning "-Waddress" }
> +}
> +
> +bool warn_parm_arr_mem_rvar (S sa[], int i)
> +{
> +  return &sa[i].ir != 0;      // { dg-warning "-Waddress" }
> +}


Marek
Jason Merrill Nov. 23, 2021, 7:59 p.m. UTC | #2 
On 11/22/21 18:21, Marek Polacek wrote:
> On Mon, Nov 22, 2021 at 04:00:56PM -0700, Martin Sebor via Gcc-patches wrote:
>> While going through old -Waddress bug reports to close after
>> the recent improvements to the warning I came across PR 96507
>> that points out that member references aren't handled.  Since
>> testing the address of a reference for equality to null is
>> in general diagnosed, this seems like an oversight worth fixing.
>>   Attached is a change to the C++ front end to diagnose member
>> references as well.
>>
>> Tested on x86_64-linux.
>>
>> Martin
> 
>> Issue -Waddress also for reference members [PR96507].
>>
>> Resolves:
>> PR c++/96507 - missing -Waddress for member references
>>
>> gcc/cp/ChangeLog:
>>
>> 	PR c++/96507
>> 	* typeck.c (warn_for_null_address): Handle reference members.
>>
>> gcc/testsuite/ChangeLog:
>>
>> 	PR c++/96507
>> 	* g++.dg/warn/Waddress-8.C: New test.
>>
>> diff --git a/gcc/cp/typeck.c b/gcc/cp/typeck.c
>> index 58919aaf13e..694c53eef8a 100644
>> --- a/gcc/cp/typeck.c
>> +++ b/gcc/cp/typeck.c
>> @@ -4676,15 +4676,21 @@ warn_for_null_address (location_t location, tree op, tsubst_flags_t complain)
>>   		    "addition %qE and NULL", cop);
>>         return;
>>       }
>> -  else if (CONVERT_EXPR_P (op)
>> -      && TYPE_REF_P (TREE_TYPE (TREE_OPERAND (op, 0))))
>> +  else if (CONVERT_EXPR_P (op))
>>       {
>> -      STRIP_NOPS (op);
>> +      tree op0 = TREE_OPERAND (op, 0);
>> +      if (TYPE_REF_P (TREE_TYPE (op0)))
>> +	{
> 
> Isn't this just REFERENCE_REF_P?

No, there's no INDIRECT_REF here.

Martin, I think you don't need to change the test to two levels since 
you don't use the op0 variable again; I think these two lines:

> +	  if (TREE_CODE (op) == COMPONENT_REF)
> +	    op = TREE_OPERAND (op, 1);

are all the change you need for this fix.  OK that way.

Jason
Martin Sebor Nov. 23, 2021, 10:41 p.m. UTC | #3 
On 11/23/21 12:59 PM, Jason Merrill wrote:
> On 11/22/21 18:21, Marek Polacek wrote:
>> On Mon, Nov 22, 2021 at 04:00:56PM -0700, Martin Sebor via Gcc-patches 
>> wrote:
>>> While going through old -Waddress bug reports to close after
>>> the recent improvements to the warning I came across PR 96507
>>> that points out that member references aren't handled.  Since
>>> testing the address of a reference for equality to null is
>>> in general diagnosed, this seems like an oversight worth fixing.
>>>   Attached is a change to the C++ front end to diagnose member
>>> references as well.
>>>
>>> Tested on x86_64-linux.
>>>
>>> Martin
>>
>>> Issue -Waddress also for reference members [PR96507].
>>>
>>> Resolves:
>>> PR c++/96507 - missing -Waddress for member references
>>>
>>> gcc/cp/ChangeLog:
>>>
>>>     PR c++/96507
>>>     * typeck.c (warn_for_null_address): Handle reference members.
>>>
>>> gcc/testsuite/ChangeLog:
>>>
>>>     PR c++/96507
>>>     * g++.dg/warn/Waddress-8.C: New test.
>>>
>>> diff --git a/gcc/cp/typeck.c b/gcc/cp/typeck.c
>>> index 58919aaf13e..694c53eef8a 100644
>>> --- a/gcc/cp/typeck.c
>>> +++ b/gcc/cp/typeck.c
>>> @@ -4676,15 +4676,21 @@ warn_for_null_address (location_t location, 
>>> tree op, tsubst_flags_t complain)
>>>               "addition %qE and NULL", cop);
>>>         return;
>>>       }
>>> -  else if (CONVERT_EXPR_P (op)
>>> -      && TYPE_REF_P (TREE_TYPE (TREE_OPERAND (op, 0))))
>>> +  else if (CONVERT_EXPR_P (op))
>>>       {
>>> -      STRIP_NOPS (op);
>>> +      tree op0 = TREE_OPERAND (op, 0);
>>> +      if (TYPE_REF_P (TREE_TYPE (op0)))
>>> +    {
>>
>> Isn't this just REFERENCE_REF_P?
> 
> No, there's no INDIRECT_REF here.
> 
> Martin, I think you don't need to change the test to two levels since 
> you don't use the op0 variable again; I think these two lines:
> 
>> +      if (TREE_CODE (op) == COMPONENT_REF)
>> +        op = TREE_OPERAND (op, 1);
> 
> are all the change you need for this fix.  OK that way.

True.  I put it back the way it was and committed it in r12-5484.

Martin

> 
> Jason
>
diff mbox series

Patch

Issue -Waddress also for reference members [PR96507].

Resolves:
PR c++/96507 - missing -Waddress for member references

gcc/cp/ChangeLog:

	PR c++/96507
	* typeck.c (warn_for_null_address): Handle reference members.

gcc/testsuite/ChangeLog:

	PR c++/96507
	* g++.dg/warn/Waddress-8.C: New test.

diff --git a/gcc/cp/typeck.c b/gcc/cp/typeck.c
index 58919aaf13e..694c53eef8a 100644
--- a/gcc/cp/typeck.c
+++ b/gcc/cp/typeck.c
@@ -4676,15 +4676,21 @@  warn_for_null_address (location_t location, tree op, tsubst_flags_t complain)
 		    "addition %qE and NULL", cop);
       return;
     }
-  else if (CONVERT_EXPR_P (op)
-      && TYPE_REF_P (TREE_TYPE (TREE_OPERAND (op, 0))))
+  else if (CONVERT_EXPR_P (op))
     {
-      STRIP_NOPS (op);
+      tree op0 = TREE_OPERAND (op, 0);
+      if (TYPE_REF_P (TREE_TYPE (op0)))
+	{
+	  STRIP_NOPS (op);
+
+	  if (TREE_CODE (op) == COMPONENT_REF)
+	    op = TREE_OPERAND (op, 1);
 
-      if (DECL_P (op))
-	warned = warning_at (location, OPT_Waddress,
-			     "the compiler can assume that the address of "
-			     "%qD will never be NULL", op);
+	  if (DECL_P (op))
+	    warned = warning_at (location, OPT_Waddress,
+				 "the compiler can assume that the address of "
+				 "%qD will never be NULL", op);
+	}
     }
 
   if (warned && DECL_P (op))
diff --git a/gcc/testsuite/g++.dg/warn/Waddress-8.C b/gcc/testsuite/g++.dg/warn/Waddress-8.C
new file mode 100644
index 00000000000..797102d6be4
--- /dev/null
+++ b/gcc/testsuite/g++.dg/warn/Waddress-8.C
@@ -0,0 +1,85 @@ 
+/* PR c++/96507 - missing -Waddress for member references
+   { dg-do compile }
+   { dg-options "-Wall" } */
+
+typedef void F ();
+
+extern F 𝔢
+extern int &eir;
+
+bool warn_ext_rfun ()
+{
+  return &efr != 0;           // { dg-warning "-Waddress" }
+}
+
+bool warn_ext_rvar ()
+{
+  return &eir != 0;           // { dg-warning "-Waddress" }
+}
+
+
+bool warn_parm_rfun (F &rf)
+{
+  return &rf != 0;            // { dg-warning "-Waddress" }
+}
+
+bool warn_parm_rvar (int &ir)
+{
+  return &ir != 0;            // { dg-warning "-Waddress" }
+}
+
+// Comparing the address of a reference argument to null also triggers
+// a -Wnonnull-compare (that seems like a bug, hence PR 103363).
+// { dg-prune-output "-Wnonnull-compare" }
+
+
+struct S
+{
+  F &fr;
+  int &ir;
+};
+
+extern S es, esa[];
+
+bool warn_ext_mem_rfun ()
+{
+  return &es.fr != 0;         // { dg-warning "-Waddress" }
+}
+
+bool warn_ext_mem_rvar ()
+{
+  return &es.ir != 0;         // { dg-warning "-Waddress" }
+}
+
+
+bool warn_ext_arr_mem_rfun (int i)
+{
+  return &esa[i].fr != 0;     // { dg-warning "-Waddress" }
+}
+
+bool warn_ext_arr_mem_rvar (int i)
+{
+  return &esa[i].ir != 0;     // { dg-warning "-Waddress" }
+}
+
+
+bool warn_parm_mem_rfun (S &s)
+{
+  return &s.fr != 0;          // { dg-warning "-Waddress" }
+}
+
+bool warn_parm_mem_rvar (S &s)
+{
+  return &s.ir != 0;          // { dg-warning "-Waddress" }
+}
+
+
+bool warn_parm_arr_mem_rfun (S sa[], int i)
+{
+  return &sa[i].fr != 0;      // { dg-warning "-Waddress" }
+}
+
+bool warn_parm_arr_mem_rvar (S sa[], int i)
+{
+  return &sa[i].ir != 0;      // { dg-warning "-Waddress" }
+}