handle member references in -Waddress [PR96507]
Commit Message
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
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
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
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
>
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.
@@ -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))
new file mode 100644
@@ -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" }
+}