public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [PATCH] c++: suppress -Waddress warnings with *_cast [PR105569]
@ 2022-05-16 17:06 Marek Polacek
  2022-05-18 13:43 ` Jason Merrill
  0 siblings, 1 reply; 6+ messages in thread
From: Marek Polacek @ 2022-05-16 17:06 UTC (permalink / raw)
  To: Jason Merrill, GCC Patches

dynamic_cast can legally return nullptr, so I don't think it's helpful
for -Waddress to warn for

  if (dynamic_cast<A*>(&ref))
    // ...

More generally, it's likely not useful to warn for the artificial
POINTER_PLUS_EXPRs created by build_base_path.

Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk?

	PR c++/105569

gcc/cp/ChangeLog:

	* class.cc (build_base_path): Suppress -Waddress warning.

gcc/testsuite/ChangeLog:

	* g++.dg/warn/Waddress-9.C: New test.
---
 gcc/cp/class.cc                        |  2 ++
 gcc/testsuite/g++.dg/warn/Waddress-9.C | 34 ++++++++++++++++++++++++++
 2 files changed, 36 insertions(+)
 create mode 100644 gcc/testsuite/g++.dg/warn/Waddress-9.C

diff --git a/gcc/cp/class.cc b/gcc/cp/class.cc
index 3c195b35396..06167380423 100644
--- a/gcc/cp/class.cc
+++ b/gcc/cp/class.cc
@@ -518,6 +518,8 @@ build_base_path (enum tree_code code,
 
   expr = build1 (NOP_EXPR, ptr_target_type, expr);
 
+  suppress_warning (expr, OPT_Waddress);
+
  indout:
   if (!want_pointer)
     {
diff --git a/gcc/testsuite/g++.dg/warn/Waddress-9.C b/gcc/testsuite/g++.dg/warn/Waddress-9.C
new file mode 100644
index 00000000000..2ec41949ccf
--- /dev/null
+++ b/gcc/testsuite/g++.dg/warn/Waddress-9.C
@@ -0,0 +1,34 @@
+// PR c++/105569
+// { dg-do compile { target c++11 } }
+// { dg-options -Waddress }
+
+class A {};
+
+class B : public virtual A {};
+
+class C : public A {};
+
+int main() {
+    B* object = new B();
+    B &ref = *object;
+
+    // -Waddress warns here
+    bool b = nullptr == dynamic_cast<A*>(&ref); // { dg-bogus "-Waddress" }
+    bool b4 = nullptr == static_cast<A*>(&ref); // { dg-bogus "-Waddress" }
+    if (dynamic_cast<A*>(&ref)) // { dg-bogus "-Waddress" }
+      {
+      }
+    if (static_cast<A*>(&ref)) // { dg-bogus "-Waddress" }
+      {
+      }
+
+    // -Waddress doesn't warn anymore
+    auto ptr = dynamic_cast<A*>(&ref); 
+    bool b2 = ptr == nullptr;
+
+    C* cobject = new C();
+    C &cref = *cobject;
+
+    // -Waddress also doesn't warn anymore
+    bool b3 = nullptr == dynamic_cast<A*>(&cref);
+}

base-commit: 682e587f1021241758f7dfe0b22651008622a312
-- 
2.36.1


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] c++: suppress -Waddress warnings with *_cast [PR105569]
  2022-05-16 17:06 [PATCH] c++: suppress -Waddress warnings with *_cast [PR105569] Marek Polacek
@ 2022-05-18 13:43 ` Jason Merrill
  2022-05-25 20:25   ` [PATCH v2] " Marek Polacek
  0 siblings, 1 reply; 6+ messages in thread
From: Jason Merrill @ 2022-05-18 13:43 UTC (permalink / raw)
  To: Marek Polacek, GCC Patches

On 5/16/22 13:06, Marek Polacek wrote:
> dynamic_cast can legally return nullptr, so I don't think it's helpful
> for -Waddress to warn for
> 
>    if (dynamic_cast<A*>(&ref))
>      // ...
> 
> More generally, it's likely not useful to warn for the artificial
> POINTER_PLUS_EXPRs created by build_base_path.

Normally the POINTER_PLUS_EXPR is guarded by if (nonnull).  But 
build_base_path isn't adding that guard in this case because the operand 
is known to be a reference, which cannot be null 
(http://eel.is/c++draft/dcl.ref#5).  So a warning is indicated for this 
testcase, though it would be good to give a more informative one 
("comparing address of reference to null").

> Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk?
> 
> 	PR c++/105569
> 
> gcc/cp/ChangeLog:
> 
> 	* class.cc (build_base_path): Suppress -Waddress warning.
> 
> gcc/testsuite/ChangeLog:
> 
> 	* g++.dg/warn/Waddress-9.C: New test.
> ---
>   gcc/cp/class.cc                        |  2 ++
>   gcc/testsuite/g++.dg/warn/Waddress-9.C | 34 ++++++++++++++++++++++++++
>   2 files changed, 36 insertions(+)
>   create mode 100644 gcc/testsuite/g++.dg/warn/Waddress-9.C
> 
> diff --git a/gcc/cp/class.cc b/gcc/cp/class.cc
> index 3c195b35396..06167380423 100644
> --- a/gcc/cp/class.cc
> +++ b/gcc/cp/class.cc
> @@ -518,6 +518,8 @@ build_base_path (enum tree_code code,
>   
>     expr = build1 (NOP_EXPR, ptr_target_type, expr);
>   
> +  suppress_warning (expr, OPT_Waddress);
> +
>    indout:
>     if (!want_pointer)
>       {
> diff --git a/gcc/testsuite/g++.dg/warn/Waddress-9.C b/gcc/testsuite/g++.dg/warn/Waddress-9.C
> new file mode 100644
> index 00000000000..2ec41949ccf
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/warn/Waddress-9.C
> @@ -0,0 +1,34 @@
> +// PR c++/105569
> +// { dg-do compile { target c++11 } }
> +// { dg-options -Waddress }
> +
> +class A {};
> +
> +class B : public virtual A {};
> +
> +class C : public A {};
> +
> +int main() {
> +    B* object = new B();
> +    B &ref = *object;
> +
> +    // -Waddress warns here
> +    bool b = nullptr == dynamic_cast<A*>(&ref); // { dg-bogus "-Waddress" }
> +    bool b4 = nullptr == static_cast<A*>(&ref); // { dg-bogus "-Waddress" }
> +    if (dynamic_cast<A*>(&ref)) // { dg-bogus "-Waddress" }
> +      {
> +      }
> +    if (static_cast<A*>(&ref)) // { dg-bogus "-Waddress" }
> +      {
> +      }
> +
> +    // -Waddress doesn't warn anymore
> +    auto ptr = dynamic_cast<A*>(&ref);
> +    bool b2 = ptr == nullptr;
> +
> +    C* cobject = new C();
> +    C &cref = *cobject;
> +
> +    // -Waddress also doesn't warn anymore
> +    bool b3 = nullptr == dynamic_cast<A*>(&cref);
> +}
> 
> base-commit: 682e587f1021241758f7dfe0b22651008622a312


^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH v2] c++: suppress -Waddress warnings with *_cast [PR105569]
  2022-05-18 13:43 ` Jason Merrill
@ 2022-05-25 20:25   ` Marek Polacek
  2022-05-26 13:26     ` Jason Merrill
  0 siblings, 1 reply; 6+ messages in thread
From: Marek Polacek @ 2022-05-25 20:25 UTC (permalink / raw)
  To: Jason Merrill; +Cc: GCC Patches

On Wed, May 18, 2022 at 09:43:47AM -0400, Jason Merrill wrote:
> On 5/16/22 13:06, Marek Polacek wrote:
> > dynamic_cast can legally return nullptr, so I don't think it's helpful
> > for -Waddress to warn for
> > 
> >    if (dynamic_cast<A*>(&ref))
> >      // ...
> > 
> > More generally, it's likely not useful to warn for the artificial
> > POINTER_PLUS_EXPRs created by build_base_path.
> 
> Normally the POINTER_PLUS_EXPR is guarded by if (nonnull).  But
> build_base_path isn't adding that guard in this case because the operand is
> known to be a reference, which cannot be null
> (http://eel.is/c++draft/dcl.ref#5).  So a warning is indicated for this
> testcase, though it would be good to give a more informative one ("comparing
> address of reference to null").

Ah, got it.  How about this patch instead?  Thanks,

-- >8 --
This patch improves the diagnostic for -Waddress when it warns for

  if (dynamic_cast<A*>(&ref))
    // ...

where 'ref' is a reference, which cannot be null.  In particular, it
changes
warning: comparing the result of pointer addition '(((A*)ref) + ((sizetype)(*(long int*)((& ref)->B::_vptr.B + -24))))' and NULL
to
warning: comparing address of reference 'ref' to null

	PR c++/105569

gcc/cp/ChangeLog:

	* typeck.cc (warn_for_null_address): Improve the warning when
	the POINTER_PLUS_EXPR's base is of reference type.

gcc/testsuite/ChangeLog:

	* g++.dg/warn/Waddress-9.C: New test.
---
 gcc/cp/typeck.cc                       | 12 ++++++++--
 gcc/testsuite/g++.dg/warn/Waddress-9.C | 31 ++++++++++++++++++++++++++
 2 files changed, 41 insertions(+), 2 deletions(-)
 create mode 100644 gcc/testsuite/g++.dg/warn/Waddress-9.C

diff --git a/gcc/cp/typeck.cc b/gcc/cp/typeck.cc
index 385cdf4d733..0837f2484ba 100644
--- a/gcc/cp/typeck.cc
+++ b/gcc/cp/typeck.cc
@@ -4757,8 +4757,16 @@ warn_for_null_address (location_t location, tree op, tsubst_flags_t complain)
       tree off = TREE_OPERAND (cop, 1);
       if (!integer_zerop (off)
 	  && !warning_suppressed_p (cop, OPT_Waddress))
-	warning_at (location, OPT_Waddress, "comparing the result of pointer "
-		    "addition %qE and NULL", cop);
+	{
+	  tree base = TREE_OPERAND (cop, 0);
+	  STRIP_NOPS (base);
+	  if (TYPE_REF_P (TREE_TYPE (base)))
+	    warning_at (location, OPT_Waddress, "comparing address of "
+			"reference %qE and NULL", base);
+	  else
+	    warning_at (location, OPT_Waddress, "comparing the result of "
+			"pointer addition %qE and NULL", cop);
+	}
       return;
     }
   else if (CONVERT_EXPR_P (op)
diff --git a/gcc/testsuite/g++.dg/warn/Waddress-9.C b/gcc/testsuite/g++.dg/warn/Waddress-9.C
new file mode 100644
index 00000000000..a3654ff1f91
--- /dev/null
+++ b/gcc/testsuite/g++.dg/warn/Waddress-9.C
@@ -0,0 +1,31 @@
+// PR c++/105569
+// { dg-do compile { target c++11 } }
+// { dg-options -Waddress }
+
+class A {};
+
+class B : public virtual A {};
+
+class C : public A {};
+
+int main() {
+    B* object = new B();
+    B &ref = *object;
+
+    bool b = nullptr == dynamic_cast<A*>(&ref); // { dg-warning "comparing address of reference .ref. and NULL" }
+    bool b4 = nullptr == static_cast<A*>(&ref); // { dg-warning "comparing address of reference .ref. and NULL" }
+    if (dynamic_cast<A*>(&ref)) // { dg-warning "comparing address of reference .ref. and NULL" }
+      {
+      }
+    if (static_cast<A*>(&ref)) // { dg-warning "comparing address of reference .ref. and NULL" }
+      {
+      }
+
+    auto ptr = dynamic_cast<A*>(&ref);
+    bool b2 = ptr == nullptr;
+
+    C* cobject = new C();
+    C &cref = *cobject;
+
+    bool b3 = nullptr == dynamic_cast<A*>(&cref);
+}

base-commit: 34970d08c6297e12f3f9117b6ac19fb2de522e24
-- 
2.36.1


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v2] c++: suppress -Waddress warnings with *_cast [PR105569]
  2022-05-25 20:25   ` [PATCH v2] " Marek Polacek
@ 2022-05-26 13:26     ` Jason Merrill
  2022-05-26 13:33       ` Marek Polacek
  0 siblings, 1 reply; 6+ messages in thread
From: Jason Merrill @ 2022-05-26 13:26 UTC (permalink / raw)
  To: Marek Polacek; +Cc: GCC Patches

On 5/25/22 16:25, Marek Polacek wrote:
> On Wed, May 18, 2022 at 09:43:47AM -0400, Jason Merrill wrote:
>> On 5/16/22 13:06, Marek Polacek wrote:
>>> dynamic_cast can legally return nullptr, so I don't think it's helpful
>>> for -Waddress to warn for
>>>
>>>     if (dynamic_cast<A*>(&ref))
>>>       // ...
>>>
>>> More generally, it's likely not useful to warn for the artificial
>>> POINTER_PLUS_EXPRs created by build_base_path.
>>
>> Normally the POINTER_PLUS_EXPR is guarded by if (nonnull).  But
>> build_base_path isn't adding that guard in this case because the operand is
>> known to be a reference, which cannot be null
>> (http://eel.is/c++draft/dcl.ref#5).  So a warning is indicated for this
>> testcase, though it would be good to give a more informative one ("comparing
>> address of reference to null").
> 
> Ah, got it.  How about this patch instead?  Thanks,

Do we also warn about plain &ref == nullptr?

> -- >8 --
> This patch improves the diagnostic for -Waddress when it warns for
> 
>    if (dynamic_cast<A*>(&ref))
>      // ...
> 
> where 'ref' is a reference, which cannot be null.  In particular, it
> changes
> warning: comparing the result of pointer addition '(((A*)ref) + ((sizetype)(*(long int*)((& ref)->B::_vptr.B + -24))))' and NULL
> to
> warning: comparing address of reference 'ref' to null
> 
> 	PR c++/105569
> 
> gcc/cp/ChangeLog:
> 
> 	* typeck.cc (warn_for_null_address): Improve the warning when
> 	the POINTER_PLUS_EXPR's base is of reference type.
> 
> gcc/testsuite/ChangeLog:
> 
> 	* g++.dg/warn/Waddress-9.C: New test.
> ---
>   gcc/cp/typeck.cc                       | 12 ++++++++--
>   gcc/testsuite/g++.dg/warn/Waddress-9.C | 31 ++++++++++++++++++++++++++
>   2 files changed, 41 insertions(+), 2 deletions(-)
>   create mode 100644 gcc/testsuite/g++.dg/warn/Waddress-9.C
> 
> diff --git a/gcc/cp/typeck.cc b/gcc/cp/typeck.cc
> index 385cdf4d733..0837f2484ba 100644
> --- a/gcc/cp/typeck.cc
> +++ b/gcc/cp/typeck.cc
> @@ -4757,8 +4757,16 @@ warn_for_null_address (location_t location, tree op, tsubst_flags_t complain)
>         tree off = TREE_OPERAND (cop, 1);
>         if (!integer_zerop (off)
>   	  && !warning_suppressed_p (cop, OPT_Waddress))
> -	warning_at (location, OPT_Waddress, "comparing the result of pointer "
> -		    "addition %qE and NULL", cop);
> +	{
> +	  tree base = TREE_OPERAND (cop, 0);
> +	  STRIP_NOPS (base);
> +	  if (TYPE_REF_P (TREE_TYPE (base)))
> +	    warning_at (location, OPT_Waddress, "comparing address of "
> +			"reference %qE and NULL", base);
> +	  else
> +	    warning_at (location, OPT_Waddress, "comparing the result of "
> +			"pointer addition %qE and NULL", cop);
> +	}
>         return;
>       }
>     else if (CONVERT_EXPR_P (op)
> diff --git a/gcc/testsuite/g++.dg/warn/Waddress-9.C b/gcc/testsuite/g++.dg/warn/Waddress-9.C
> new file mode 100644
> index 00000000000..a3654ff1f91
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/warn/Waddress-9.C
> @@ -0,0 +1,31 @@
> +// PR c++/105569
> +// { dg-do compile { target c++11 } }
> +// { dg-options -Waddress }
> +
> +class A {};
> +
> +class B : public virtual A {};
> +
> +class C : public A {};
> +
> +int main() {
> +    B* object = new B();
> +    B &ref = *object;
> +
> +    bool b = nullptr == dynamic_cast<A*>(&ref); // { dg-warning "comparing address of reference .ref. and NULL" }
> +    bool b4 = nullptr == static_cast<A*>(&ref); // { dg-warning "comparing address of reference .ref. and NULL" }
> +    if (dynamic_cast<A*>(&ref)) // { dg-warning "comparing address of reference .ref. and NULL" }
> +      {
> +      }
> +    if (static_cast<A*>(&ref)) // { dg-warning "comparing address of reference .ref. and NULL" }
> +      {
> +      }
> +
> +    auto ptr = dynamic_cast<A*>(&ref);
> +    bool b2 = ptr == nullptr;
> +
> +    C* cobject = new C();
> +    C &cref = *cobject;
> +
> +    bool b3 = nullptr == dynamic_cast<A*>(&cref);
> +}
> 
> base-commit: 34970d08c6297e12f3f9117b6ac19fb2de522e24


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v2] c++: suppress -Waddress warnings with *_cast [PR105569]
  2022-05-26 13:26     ` Jason Merrill
@ 2022-05-26 13:33       ` Marek Polacek
  2022-05-26 20:34         ` Jason Merrill
  0 siblings, 1 reply; 6+ messages in thread
From: Marek Polacek @ 2022-05-26 13:33 UTC (permalink / raw)
  To: Jason Merrill; +Cc: GCC Patches

On Thu, May 26, 2022 at 09:26:16AM -0400, Jason Merrill wrote:
> On 5/25/22 16:25, Marek Polacek wrote:
> > On Wed, May 18, 2022 at 09:43:47AM -0400, Jason Merrill wrote:
> > > On 5/16/22 13:06, Marek Polacek wrote:
> > > > dynamic_cast can legally return nullptr, so I don't think it's helpful
> > > > for -Waddress to warn for
> > > > 
> > > >     if (dynamic_cast<A*>(&ref))
> > > >       // ...
> > > > 
> > > > More generally, it's likely not useful to warn for the artificial
> > > > POINTER_PLUS_EXPRs created by build_base_path.
> > > 
> > > Normally the POINTER_PLUS_EXPR is guarded by if (nonnull).  But
> > > build_base_path isn't adding that guard in this case because the operand is
> > > known to be a reference, which cannot be null
> > > (http://eel.is/c++draft/dcl.ref#5).  So a warning is indicated for this
> > > testcase, though it would be good to give a more informative one ("comparing
> > > address of reference to null").
> > 
> > Ah, got it.  How about this patch instead?  Thanks,
> 
> Do we also warn about plain &ref == nullptr?

Yeah, with different wording:

p.C:5:12: warning: the compiler can assume that the address of ‘ref’ will never be NULL [-Waddress]
    5 |   if (&ref == nullptr)
      |       ~~~~~^~~~~~~~~~

warn/Walways-true-3.C tests this.

> > -- >8 --
> > This patch improves the diagnostic for -Waddress when it warns for
> > 
> >    if (dynamic_cast<A*>(&ref))
> >      // ...
> > 
> > where 'ref' is a reference, which cannot be null.  In particular, it
> > changes
> > warning: comparing the result of pointer addition '(((A*)ref) + ((sizetype)(*(long int*)((& ref)->B::_vptr.B + -24))))' and NULL
> > to
> > warning: comparing address of reference 'ref' to null
> > 
> > 	PR c++/105569
> > 
> > gcc/cp/ChangeLog:
> > 
> > 	* typeck.cc (warn_for_null_address): Improve the warning when
> > 	the POINTER_PLUS_EXPR's base is of reference type.
> > 
> > gcc/testsuite/ChangeLog:
> > 
> > 	* g++.dg/warn/Waddress-9.C: New test.
> > ---
> >   gcc/cp/typeck.cc                       | 12 ++++++++--
> >   gcc/testsuite/g++.dg/warn/Waddress-9.C | 31 ++++++++++++++++++++++++++
> >   2 files changed, 41 insertions(+), 2 deletions(-)
> >   create mode 100644 gcc/testsuite/g++.dg/warn/Waddress-9.C
> > 
> > diff --git a/gcc/cp/typeck.cc b/gcc/cp/typeck.cc
> > index 385cdf4d733..0837f2484ba 100644
> > --- a/gcc/cp/typeck.cc
> > +++ b/gcc/cp/typeck.cc
> > @@ -4757,8 +4757,16 @@ warn_for_null_address (location_t location, tree op, tsubst_flags_t complain)
> >         tree off = TREE_OPERAND (cop, 1);
> >         if (!integer_zerop (off)
> >   	  && !warning_suppressed_p (cop, OPT_Waddress))
> > -	warning_at (location, OPT_Waddress, "comparing the result of pointer "
> > -		    "addition %qE and NULL", cop);
> > +	{
> > +	  tree base = TREE_OPERAND (cop, 0);
> > +	  STRIP_NOPS (base);
> > +	  if (TYPE_REF_P (TREE_TYPE (base)))
> > +	    warning_at (location, OPT_Waddress, "comparing address of "
> > +			"reference %qE and NULL", base);
> > +	  else
> > +	    warning_at (location, OPT_Waddress, "comparing the result of "
> > +			"pointer addition %qE and NULL", cop);
> > +	}
> >         return;
> >       }
> >     else if (CONVERT_EXPR_P (op)
> > diff --git a/gcc/testsuite/g++.dg/warn/Waddress-9.C b/gcc/testsuite/g++.dg/warn/Waddress-9.C
> > new file mode 100644
> > index 00000000000..a3654ff1f91
> > --- /dev/null
> > +++ b/gcc/testsuite/g++.dg/warn/Waddress-9.C
> > @@ -0,0 +1,31 @@
> > +// PR c++/105569
> > +// { dg-do compile { target c++11 } }
> > +// { dg-options -Waddress }
> > +
> > +class A {};
> > +
> > +class B : public virtual A {};
> > +
> > +class C : public A {};
> > +
> > +int main() {
> > +    B* object = new B();
> > +    B &ref = *object;
> > +
> > +    bool b = nullptr == dynamic_cast<A*>(&ref); // { dg-warning "comparing address of reference .ref. and NULL" }
> > +    bool b4 = nullptr == static_cast<A*>(&ref); // { dg-warning "comparing address of reference .ref. and NULL" }
> > +    if (dynamic_cast<A*>(&ref)) // { dg-warning "comparing address of reference .ref. and NULL" }
> > +      {
> > +      }
> > +    if (static_cast<A*>(&ref)) // { dg-warning "comparing address of reference .ref. and NULL" }
> > +      {
> > +      }
> > +
> > +    auto ptr = dynamic_cast<A*>(&ref);
> > +    bool b2 = ptr == nullptr;
> > +
> > +    C* cobject = new C();
> > +    C &cref = *cobject;
> > +
> > +    bool b3 = nullptr == dynamic_cast<A*>(&cref);
> > +}
> > 
> > base-commit: 34970d08c6297e12f3f9117b6ac19fb2de522e24
> 

Marek


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v2] c++: suppress -Waddress warnings with *_cast [PR105569]
  2022-05-26 13:33       ` Marek Polacek
@ 2022-05-26 20:34         ` Jason Merrill
  0 siblings, 0 replies; 6+ messages in thread
From: Jason Merrill @ 2022-05-26 20:34 UTC (permalink / raw)
  To: Marek Polacek; +Cc: GCC Patches

On 5/26/22 09:33, Marek Polacek wrote:
> On Thu, May 26, 2022 at 09:26:16AM -0400, Jason Merrill wrote:
>> On 5/25/22 16:25, Marek Polacek wrote:
>>> On Wed, May 18, 2022 at 09:43:47AM -0400, Jason Merrill wrote:
>>>> On 5/16/22 13:06, Marek Polacek wrote:
>>>>> dynamic_cast can legally return nullptr, so I don't think it's helpful
>>>>> for -Waddress to warn for
>>>>>
>>>>>      if (dynamic_cast<A*>(&ref))
>>>>>        // ...
>>>>>
>>>>> More generally, it's likely not useful to warn for the artificial
>>>>> POINTER_PLUS_EXPRs created by build_base_path.
>>>>
>>>> Normally the POINTER_PLUS_EXPR is guarded by if (nonnull).  But
>>>> build_base_path isn't adding that guard in this case because the operand is
>>>> known to be a reference, which cannot be null
>>>> (http://eel.is/c++draft/dcl.ref#5).  So a warning is indicated for this
>>>> testcase, though it would be good to give a more informative one ("comparing
>>>> address of reference to null").
>>>
>>> Ah, got it.  How about this patch instead?  Thanks,
>>
>> Do we also warn about plain &ref == nullptr?
> 
> Yeah, with different wording:
> 
> p.C:5:12: warning: the compiler can assume that the address of ‘ref’ will never be NULL [-Waddress]
>      5 |   if (&ref == nullptr)
>        |       ~~~~~^~~~~~~~~~
> 
> warn/Walways-true-3.C tests this.

Let's use that wording in this case as well, then.  OK with that change.

>>> -- >8 --
>>> This patch improves the diagnostic for -Waddress when it warns for
>>>
>>>     if (dynamic_cast<A*>(&ref))
>>>       // ...
>>>
>>> where 'ref' is a reference, which cannot be null.  In particular, it
>>> changes
>>> warning: comparing the result of pointer addition '(((A*)ref) + ((sizetype)(*(long int*)((& ref)->B::_vptr.B + -24))))' and NULL
>>> to
>>> warning: comparing address of reference 'ref' to null
>>>
>>> 	PR c++/105569
>>>
>>> gcc/cp/ChangeLog:
>>>
>>> 	* typeck.cc (warn_for_null_address): Improve the warning when
>>> 	the POINTER_PLUS_EXPR's base is of reference type.
>>>
>>> gcc/testsuite/ChangeLog:
>>>
>>> 	* g++.dg/warn/Waddress-9.C: New test.
>>> ---
>>>    gcc/cp/typeck.cc                       | 12 ++++++++--
>>>    gcc/testsuite/g++.dg/warn/Waddress-9.C | 31 ++++++++++++++++++++++++++
>>>    2 files changed, 41 insertions(+), 2 deletions(-)
>>>    create mode 100644 gcc/testsuite/g++.dg/warn/Waddress-9.C
>>>
>>> diff --git a/gcc/cp/typeck.cc b/gcc/cp/typeck.cc
>>> index 385cdf4d733..0837f2484ba 100644
>>> --- a/gcc/cp/typeck.cc
>>> +++ b/gcc/cp/typeck.cc
>>> @@ -4757,8 +4757,16 @@ warn_for_null_address (location_t location, tree op, tsubst_flags_t complain)
>>>          tree off = TREE_OPERAND (cop, 1);
>>>          if (!integer_zerop (off)
>>>    	  && !warning_suppressed_p (cop, OPT_Waddress))
>>> -	warning_at (location, OPT_Waddress, "comparing the result of pointer "
>>> -		    "addition %qE and NULL", cop);
>>> +	{
>>> +	  tree base = TREE_OPERAND (cop, 0);
>>> +	  STRIP_NOPS (base);
>>> +	  if (TYPE_REF_P (TREE_TYPE (base)))
>>> +	    warning_at (location, OPT_Waddress, "comparing address of "
>>> +			"reference %qE and NULL", base);
>>> +	  else
>>> +	    warning_at (location, OPT_Waddress, "comparing the result of "
>>> +			"pointer addition %qE and NULL", cop);
>>> +	}
>>>          return;
>>>        }
>>>      else if (CONVERT_EXPR_P (op)
>>> diff --git a/gcc/testsuite/g++.dg/warn/Waddress-9.C b/gcc/testsuite/g++.dg/warn/Waddress-9.C
>>> new file mode 100644
>>> index 00000000000..a3654ff1f91
>>> --- /dev/null
>>> +++ b/gcc/testsuite/g++.dg/warn/Waddress-9.C
>>> @@ -0,0 +1,31 @@
>>> +// PR c++/105569
>>> +// { dg-do compile { target c++11 } }
>>> +// { dg-options -Waddress }
>>> +
>>> +class A {};
>>> +
>>> +class B : public virtual A {};
>>> +
>>> +class C : public A {};
>>> +
>>> +int main() {
>>> +    B* object = new B();
>>> +    B &ref = *object;
>>> +
>>> +    bool b = nullptr == dynamic_cast<A*>(&ref); // { dg-warning "comparing address of reference .ref. and NULL" }
>>> +    bool b4 = nullptr == static_cast<A*>(&ref); // { dg-warning "comparing address of reference .ref. and NULL" }
>>> +    if (dynamic_cast<A*>(&ref)) // { dg-warning "comparing address of reference .ref. and NULL" }
>>> +      {
>>> +      }
>>> +    if (static_cast<A*>(&ref)) // { dg-warning "comparing address of reference .ref. and NULL" }
>>> +      {
>>> +      }
>>> +
>>> +    auto ptr = dynamic_cast<A*>(&ref);
>>> +    bool b2 = ptr == nullptr;
>>> +
>>> +    C* cobject = new C();
>>> +    C &cref = *cobject;
>>> +
>>> +    bool b3 = nullptr == dynamic_cast<A*>(&cref);
>>> +}
>>>
>>> base-commit: 34970d08c6297e12f3f9117b6ac19fb2de522e24
>>
> 
> Marek
> 


^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2022-05-26 20:35 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-05-16 17:06 [PATCH] c++: suppress -Waddress warnings with *_cast [PR105569] Marek Polacek
2022-05-18 13:43 ` Jason Merrill
2022-05-25 20:25   ` [PATCH v2] " Marek Polacek
2022-05-26 13:26     ` Jason Merrill
2022-05-26 13:33       ` Marek Polacek
2022-05-26 20:34         ` Jason Merrill

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).