Re: [PATCH v2] c++: suppress -Waddress warnings with *_cast [PR105569]

[Jason Merrill <jason@redhat.com>]

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