Message ID | de150fbb-ea88-cd2e-5893-8c59a1cba697@gmail.com |
---|---|
State | New |
Headers | show |
Series | handle member references in -Waddress [PR96507] | expand |
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. 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" } +}