[v2] c++: fix -Wdangling-reference false positive [PR115987]

On Thu, Aug 01, 2024 at 05:20:43PM -0400, Jason Merrill wrote:
> On 8/1/24 4:19 PM, Marek Polacek wrote:
> > Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk?
> > 
> > -- >8 --
> > This fixes another false positive.  When a function is taking a
> > temporary of scalar type that couldn't be bound to the return type
> > of the function, don't warn, such a program would be ill-formed.
> > 
> > Thanks to Jonathan for reporting the problem.
> > 
> > 	PR c++/115987
> > 
> > gcc/cp/ChangeLog:
> > 
> > 	* call.cc (do_warn_dangling_reference): Don't consider a
> > 	temporary with a scalar type that cannot bind to the return type.
> > 
> > gcc/testsuite/ChangeLog:
> > 
> > 	* g++.dg/warn/Wdangling-reference22.C: New test.
> > ---
> >   gcc/cp/call.cc                                | 15 +++++++++++++--
> >   .../g++.dg/warn/Wdangling-reference22.C       | 19 +++++++++++++++++++
> >   2 files changed, 32 insertions(+), 2 deletions(-)
> >   create mode 100644 gcc/testsuite/g++.dg/warn/Wdangling-reference22.C
> > 
> > diff --git a/gcc/cp/call.cc b/gcc/cp/call.cc
> > index 40cb582acc7..375256ebcc4 100644
> > --- a/gcc/cp/call.cc
> > +++ b/gcc/cp/call.cc
> > @@ -14290,8 +14290,19 @@ do_warn_dangling_reference (tree expr, bool arg_p)
> >   	    /* Recurse to see if the argument is a temporary.  It could also
> >   	       be another call taking a temporary and returning it and
> >   	       initializing this reference parameter.  */
> > -	    if (do_warn_dangling_reference (arg, /*arg_p=*/true))
> > -	      return expr;
> > +	    if ((arg = do_warn_dangling_reference (arg, /*arg_p=*/true)))
> > +	      {
> > +		/* If we know the temporary could not bind to the return type,
> > +		   don't warn.  This is for scalars only because for classes
> > +		   we can't be sure we are not returning its sub-object.  */
> > +		if (SCALAR_TYPE_P (TREE_TYPE (arg))
> > +		    && TYPE_REF_P (rettype)
> > +		    && SCALAR_TYPE_P (TREE_TYPE (rettype))
> 
> I don't think we need to check for scalar return type, only argument type.

Oh that was a late change to keep attr-no-dangling6.C working, i.e., to
keep warning for something like

  struct X { X(int); };
  const X& get (const int& i)
  {
     return i;
  }

  void test ()
  {
    [[maybe_unused]] const X& x2 = get (10);
  }

But we already emit a -Wreturn-local-addr warning there.  So, I've dropped
the SCALAR_TYPE_P check and adjusted attr-no-dangling6.C instead:

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

-- >8 --
This fixes another false positive.  When a function is taking a
temporary of scalar type that couldn't be bound to the return type
of the function, don't warn, such a program would be ill-formed.

Thanks to Jonathan for reporting the problem.

	PR c++/115987

gcc/cp/ChangeLog:

	* call.cc (do_warn_dangling_reference): Don't consider a
	temporary with a scalar type that cannot bind to the return type.

gcc/testsuite/ChangeLog:

	* g++.dg/ext/attr-no-dangling6.C: Adjust.
	* g++.dg/ext/attr-no-dangling7.C: Likewise.
	* g++.dg/warn/Wdangling-reference22.C: New test.
---
 gcc/cp/call.cc                                | 14 ++++++++++--
 gcc/testsuite/g++.dg/ext/attr-no-dangling6.C  | 22 +++++++++----------
 gcc/testsuite/g++.dg/ext/attr-no-dangling7.C  |  8 +++----
 .../g++.dg/warn/Wdangling-reference22.C       | 19 ++++++++++++++++
 4 files changed, 46 insertions(+), 17 deletions(-)
 create mode 100644 gcc/testsuite/g++.dg/warn/Wdangling-reference22.C


base-commit: 5ebfaf2d4994c124ce81aa0abd7eaa1529644749

On 8/2/24 3:22 PM, Marek Polacek wrote:
> On Thu, Aug 01, 2024 at 05:20:43PM -0400, Jason Merrill wrote:
>> On 8/1/24 4:19 PM, Marek Polacek wrote:
>>> Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk?
>>>
>>> -- >8 --
>>> This fixes another false positive.  When a function is taking a
>>> temporary of scalar type that couldn't be bound to the return type
>>> of the function, don't warn, such a program would be ill-formed.
>>>
>>> Thanks to Jonathan for reporting the problem.
>>>
>>> 	PR c++/115987
>>>
>>> gcc/cp/ChangeLog:
>>>
>>> 	* call.cc (do_warn_dangling_reference): Don't consider a
>>> 	temporary with a scalar type that cannot bind to the return type.
>>>
>>> gcc/testsuite/ChangeLog:
>>>
>>> 	* g++.dg/warn/Wdangling-reference22.C: New test.
>>> ---
>>>    gcc/cp/call.cc                                | 15 +++++++++++++--
>>>    .../g++.dg/warn/Wdangling-reference22.C       | 19 +++++++++++++++++++
>>>    2 files changed, 32 insertions(+), 2 deletions(-)
>>>    create mode 100644 gcc/testsuite/g++.dg/warn/Wdangling-reference22.C
>>>
>>> diff --git a/gcc/cp/call.cc b/gcc/cp/call.cc
>>> index 40cb582acc7..375256ebcc4 100644
>>> --- a/gcc/cp/call.cc
>>> +++ b/gcc/cp/call.cc
>>> @@ -14290,8 +14290,19 @@ do_warn_dangling_reference (tree expr, bool arg_p)
>>>    	    /* Recurse to see if the argument is a temporary.  It could also
>>>    	       be another call taking a temporary and returning it and
>>>    	       initializing this reference parameter.  */
>>> -	    if (do_warn_dangling_reference (arg, /*arg_p=*/true))
>>> -	      return expr;
>>> +	    if ((arg = do_warn_dangling_reference (arg, /*arg_p=*/true)))
>>> +	      {
>>> +		/* If we know the temporary could not bind to the return type,
>>> +		   don't warn.  This is for scalars only because for classes
>>> +		   we can't be sure we are not returning its sub-object.  */
>>> +		if (SCALAR_TYPE_P (TREE_TYPE (arg))
>>> +		    && TYPE_REF_P (rettype)
>>> +		    && SCALAR_TYPE_P (TREE_TYPE (rettype))
>>
>> I don't think we need to check for scalar return type, only argument type.
> 
> Oh that was a late change to keep attr-no-dangling6.C working, i.e., to
> keep warning for something like
> 
>    struct X { X(int); };
>    const X& get (const int& i)
>    {
>       return i;
>    }
> 
>    void test ()
>    {
>      [[maybe_unused]] const X& x2 = get (10);
>    }
> 
> But we already emit a -Wreturn-local-addr warning there.  So, I've dropped
> the SCALAR_TYPE_P check and adjusted attr-no-dangling6.C instead:
> 
> Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk?

OK.

> -- >8 --
> This fixes another false positive.  When a function is taking a
> temporary of scalar type that couldn't be bound to the return type
> of the function, don't warn, such a program would be ill-formed.
> 
> Thanks to Jonathan for reporting the problem.
> 
> 	PR c++/115987
> 
> gcc/cp/ChangeLog:
> 
> 	* call.cc (do_warn_dangling_reference): Don't consider a
> 	temporary with a scalar type that cannot bind to the return type.
> 
> gcc/testsuite/ChangeLog:
> 
> 	* g++.dg/ext/attr-no-dangling6.C: Adjust.
> 	* g++.dg/ext/attr-no-dangling7.C: Likewise.
> 	* g++.dg/warn/Wdangling-reference22.C: New test.
> ---
>   gcc/cp/call.cc                                | 14 ++++++++++--
>   gcc/testsuite/g++.dg/ext/attr-no-dangling6.C  | 22 +++++++++----------
>   gcc/testsuite/g++.dg/ext/attr-no-dangling7.C  |  8 +++----
>   .../g++.dg/warn/Wdangling-reference22.C       | 19 ++++++++++++++++
>   4 files changed, 46 insertions(+), 17 deletions(-)
>   create mode 100644 gcc/testsuite/g++.dg/warn/Wdangling-reference22.C
> 
> diff --git a/gcc/cp/call.cc b/gcc/cp/call.cc
> index 40cb582acc7..a75e2e5e3af 100644
> --- a/gcc/cp/call.cc
> +++ b/gcc/cp/call.cc
> @@ -14290,8 +14290,18 @@ do_warn_dangling_reference (tree expr, bool arg_p)
>   	    /* Recurse to see if the argument is a temporary.  It could also
>   	       be another call taking a temporary and returning it and
>   	       initializing this reference parameter.  */
> -	    if (do_warn_dangling_reference (arg, /*arg_p=*/true))
> -	      return expr;
> +	    if ((arg = do_warn_dangling_reference (arg, /*arg_p=*/true)))
> +	      {
> +		/* If we know the temporary could not bind to the return type,
> +		   don't warn.  This is for scalars only because for classes
> +		   we can't be sure we are not returning its sub-object.  */
> +		if (SCALAR_TYPE_P (TREE_TYPE (arg))
> +		    && TYPE_REF_P (rettype)
> +		    && !reference_related_p (TREE_TYPE (arg),
> +					     TREE_TYPE (rettype)))
> +		  continue;
> +		return expr;
> +	      }
>   	  /* Don't warn about member functions like:
>   	      std::any a(...);
>   	      S& s = a.emplace<S>({0}, 0);
> diff --git a/gcc/testsuite/g++.dg/ext/attr-no-dangling6.C b/gcc/testsuite/g++.dg/ext/attr-no-dangling6.C
> index 235a5fd86c5..5b349e8e682 100644
> --- a/gcc/testsuite/g++.dg/ext/attr-no-dangling6.C
> +++ b/gcc/testsuite/g++.dg/ext/attr-no-dangling6.C
> @@ -12,26 +12,26 @@ struct SF { static constexpr bool value = false; };
>   
>   template<typename T>
>   [[gnu::no_dangling(T::value)]]
> -const X& get (const int& i)
> +const X& get (const int& i, const X&)
>   {
>      return i == 0 ? x1 : x2;
>   }
>   
>   template<bool B = true>
>   [[gnu::no_dangling(B)]]
> -const X& foo (const int& i)
> +const X& foo (const int& i, const X&)
>   {
>      return i == 0 ? x1 : x2;
>   }
>   
>   [[gnu::no_dangling(val ())]]
> -const X& bar (const int& i)
> +const X& bar (const int& i, const X&)
>   {
>      return i == 0 ? x1 : x2;
>   }
>   
>   [[gnu::no_dangling(!val ())]]
> -const X& baz (const int& i)
> +const X& baz (const int& i, const X&)
>   {
>      return i == 0 ? x1 : x2;
>   }
> @@ -52,13 +52,13 @@ auto gety() -> Span<SF>;
>   void
>   test ()
>   {
> -  [[maybe_unused]] const X& x1 = get<ST> (10);	  // { dg-bogus "dangling" }
> -  [[maybe_unused]] const X& x2 = get<SF> (10);	  // { dg-warning "dangling" }
> -  [[maybe_unused]] const X& x3 = foo<true> (10);  // { dg-bogus "dangling" }
> -  [[maybe_unused]] const X& x4 = foo<false> (10); // { dg-warning "dangling" }
> -  [[maybe_unused]] const X& x7 = foo<> (10);	  // { dg-bogus "dangling" }
> -  [[maybe_unused]] const X& x5 = bar (10);	  // { dg-bogus "dangling" }
> -  [[maybe_unused]] const X& x6 = baz (10);	  // { dg-warning "dangling" }
> +  [[maybe_unused]] const X& x1 = get<ST> (10, X{});	  // { dg-bogus "dangling" }
> +  [[maybe_unused]] const X& x2 = get<SF> (10, X{});	  // { dg-warning "dangling" }
> +  [[maybe_unused]] const X& x3 = foo<true> (10, X{});  // { dg-bogus "dangling" }
> +  [[maybe_unused]] const X& x4 = foo<false> (10, X{}); // { dg-warning "dangling" }
> +  [[maybe_unused]] const X& x7 = foo<> (10, X{});	  // { dg-bogus "dangling" }
> +  [[maybe_unused]] const X& x5 = bar (10, X{});	  // { dg-bogus "dangling" }
> +  [[maybe_unused]] const X& x6 = baz (10, X{});	  // { dg-warning "dangling" }
>   
>     [[maybe_unused]] const auto &b1 = geti()[0];	  // { dg-bogus "dangling" }
>     [[maybe_unused]] const auto &b2 = gety()[0];	  // { dg-warning "dangling" }
> diff --git a/gcc/testsuite/g++.dg/ext/attr-no-dangling7.C b/gcc/testsuite/g++.dg/ext/attr-no-dangling7.C
> index 3c392ed409f..a5fb809e6bd 100644
> --- a/gcc/testsuite/g++.dg/ext/attr-no-dangling7.C
> +++ b/gcc/testsuite/g++.dg/ext/attr-no-dangling7.C
> @@ -16,16 +16,16 @@ const X& foo(const int& i);
>   bool val () { return true; }
>   
>   [[gnu::no_dangling(val ())]]   // { dg-error "call" }
> -const X& bar (const int& i);
> +const X& bar (const int& i, const X&);
>   
> -[[gnu::no_dangling(20)]] const X& fn1 (const int &);
> +[[gnu::no_dangling(20)]] const X& fn1 (const int &, const X&);
>   
>   void
>   test ()
>   {
> -  [[maybe_unused]] const X& x1 = bar (10);	  // { dg-warning "dangling" }
> +  [[maybe_unused]] const X& x1 = bar (10, X{});	  // { dg-warning "dangling" }
>     [[maybe_unused]] const X& x2 = foo<int> (10);	  // { dg-error "no matching" }
>     [[maybe_unused]] const X& x3			  // { dg-warning "dangling" }
> -    = fn1 (10);					  // { dg-error "narrowing" }
> +    = fn1 (10, X{});				  // { dg-error "narrowing" }
>   }
>   
> diff --git a/gcc/testsuite/g++.dg/warn/Wdangling-reference22.C b/gcc/testsuite/g++.dg/warn/Wdangling-reference22.C
> new file mode 100644
> index 00000000000..0381f9313fb
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/warn/Wdangling-reference22.C
> @@ -0,0 +1,19 @@
> +// PR c++/115987
> +// { dg-do compile { target c++11 } }
> +// { dg-options "-Wdangling-reference" }
> +
> +template <typename T>
> +struct Wrapper {
> +    T val;
> +};
> +
> +template <typename T, typename FUNC>
> +    const T& unwrap_2(const Wrapper<T>& r, FUNC&&) {
> +  return r.val;
> +}
> +
> +int main(int, char**) {
> +  const Wrapper<int> w{1234};
> +  const auto& u = unwrap_2(w, 1L);  // { dg-bogus "dangling reference" }
> +  __builtin_printf("Unwrapped value : %d\n", u);
> +}
> 
> base-commit: 5ebfaf2d4994c124ce81aa0abd7eaa1529644749