public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [PATCH] c++: Disable -Wdangling-reference when initing T&
@ 2022-11-11 20:22 Marek Polacek
  2022-11-14 23:16 ` Jason Merrill
  0 siblings, 1 reply; 2+ messages in thread
From: Marek Polacek @ 2022-11-11 20:22 UTC (permalink / raw)
  To: Jason Merrill, GCC Patches

Non-const lvalue references can't bind to a temporary, so the
warning should not be emitted if we're initializing something of that
type.  I'm not disabling the warning when the function itself returns
a non-const lvalue reference, that would regress at least

  const int &r = std::any_cast<int&>(std::any());

in Wdangling-reference2.C where the any_cast returns an int&.

Unfortunately, this patch means we'll stop diagnosing

  int& fn(int&& x) { return static_cast<int&>(x); }
  void test ()
  {
    int &r = fn(4);
  }

where there's a genuine dangling reference.  OTOH, the patch
should suppress false positives with iterators, like:

  auto &candidate = *candidates.begin ();

and arguably that's more important than detecting some relatively
obscure cases.  It's probably not worth it making the warning more
complicated by, for instance, not warning when a fn returns 'int&'
but takes 'const int&' (because then it can't return its argument).

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

gcc/cp/ChangeLog:

	* call.cc (maybe_warn_dangling_reference): Don't warn when initializing
	a non-const lvalue reference.

gcc/testsuite/ChangeLog:

	* g++.dg/cpp23/elision4.C: Remove dg-warning.
	* g++.dg/warn/Wdangling-reference1.C: Turn dg-warning into dg-bogus.
	* g++.dg/warn/Wdangling-reference7.C: New test.
---
 gcc/cp/call.cc                                   | 10 ++++++++--
 gcc/testsuite/g++.dg/cpp23/elision4.C            |  4 ++--
 gcc/testsuite/g++.dg/warn/Wdangling-reference1.C |  4 ++--
 gcc/testsuite/g++.dg/warn/Wdangling-reference7.C | 16 ++++++++++++++++
 4 files changed, 28 insertions(+), 6 deletions(-)
 create mode 100644 gcc/testsuite/g++.dg/warn/Wdangling-reference7.C

diff --git a/gcc/cp/call.cc b/gcc/cp/call.cc
index bd3b64a7e26..ef618d5c485 100644
--- a/gcc/cp/call.cc
+++ b/gcc/cp/call.cc
@@ -13679,8 +13679,14 @@ maybe_warn_dangling_reference (const_tree decl, tree init)
 {
   if (!warn_dangling_reference)
     return;
-  if (!(TYPE_REF_OBJ_P (TREE_TYPE (decl))
-	|| std_pair_ref_ref_p (TREE_TYPE (decl))))
+  tree type = TREE_TYPE (decl);
+  /* Only warn if what we're initializing has type T&& or const T&, or
+     std::pair<const T&, const T&>.  (A non-const lvalue reference can't
+     bind to a temporary.)  */
+  if (!((TYPE_REF_OBJ_P (type)
+	 && (TYPE_REF_IS_RVALUE (type)
+	     || CP_TYPE_CONST_P (TREE_TYPE (type))))
+	|| std_pair_ref_ref_p (type)))
     return;
   /* Don't suppress the diagnostic just because the call comes from
      a system header.  If the DECL is not in a system header, or if
diff --git a/gcc/testsuite/g++.dg/cpp23/elision4.C b/gcc/testsuite/g++.dg/cpp23/elision4.C
index d39053ad741..77dcffcdaad 100644
--- a/gcc/testsuite/g++.dg/cpp23/elision4.C
+++ b/gcc/testsuite/g++.dg/cpp23/elision4.C
@@ -34,6 +34,6 @@ T& temporary2(T&& x) { return static_cast<T&>(x); }
 void
 test ()
 {
-  int& r1 = temporary1 (42); // { dg-warning "dangling reference" }
-  int& r2 = temporary2 (42); // { dg-warning "dangling reference" }
+  int& r1 = temporary1 (42);
+  int& r2 = temporary2 (42);
 }
diff --git a/gcc/testsuite/g++.dg/warn/Wdangling-reference1.C b/gcc/testsuite/g++.dg/warn/Wdangling-reference1.C
index 97c81ee716c..1718c28165e 100644
--- a/gcc/testsuite/g++.dg/warn/Wdangling-reference1.C
+++ b/gcc/testsuite/g++.dg/warn/Wdangling-reference1.C
@@ -139,6 +139,6 @@ struct Y {
 // x1 = Y::operator int&& (&TARGET_EXPR <D.2410, {}>)
 int&& x1 = Y(); // { dg-warning "dangling reference" }
 int&& x2 = Y{}; // { dg-warning "dangling reference" }
-int& x3 = Y(); // { dg-warning "dangling reference" }
-int& x4 = Y{}; // { dg-warning "dangling reference" }
+int& x3 = Y(); // { dg-bogus "dangling reference" }
+int& x4 = Y{}; // { dg-bogus "dangling reference" }
 const int& t1 = Y().foo(10); // { dg-warning "dangling reference" }
diff --git a/gcc/testsuite/g++.dg/warn/Wdangling-reference7.C b/gcc/testsuite/g++.dg/warn/Wdangling-reference7.C
new file mode 100644
index 00000000000..4b0de2d8670
--- /dev/null
+++ b/gcc/testsuite/g++.dg/warn/Wdangling-reference7.C
@@ -0,0 +1,16 @@
+// { dg-do compile { target c++11 } }
+// { dg-options "-Wdangling-reference" }
+
+int& ref(const int&);
+int&& rref(const int&);
+
+void
+g ()
+{
+  const int& r1 = ref (1); // { dg-warning "dangling reference" }
+  int& r2 = ref (2); // { dg-bogus "dangling reference" }
+  auto& r3 = ref (3); // { dg-bogus "dangling reference" }
+  int&& r4 = rref (4); // { dg-warning "dangling reference" }
+  auto&& r5 = rref (5); // { dg-warning "dangling reference" }
+  const int&& r6 = rref (6); // { dg-warning "dangling reference" }
+}

base-commit: 0a7b437ca71e2721e9bcf070762fc54ef7991aeb
-- 
2.38.1


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

* Re: [PATCH] c++: Disable -Wdangling-reference when initing T&
  2022-11-11 20:22 [PATCH] c++: Disable -Wdangling-reference when initing T& Marek Polacek
@ 2022-11-14 23:16 ` Jason Merrill
  0 siblings, 0 replies; 2+ messages in thread
From: Jason Merrill @ 2022-11-14 23:16 UTC (permalink / raw)
  To: Marek Polacek, GCC Patches

On 11/11/22 10:22, Marek Polacek wrote:
> Non-const lvalue references can't bind to a temporary, so the
> warning should not be emitted if we're initializing something of that
> type.  I'm not disabling the warning when the function itself returns
> a non-const lvalue reference, that would regress at least
> 
>    const int &r = std::any_cast<int&>(std::any());
> 
> in Wdangling-reference2.C where the any_cast returns an int&.
> 
> Unfortunately, this patch means we'll stop diagnosing
> 
>    int& fn(int&& x) { return static_cast<int&>(x); }
>    void test ()
>    {
>      int &r = fn(4);
>    }
> 
> where there's a genuine dangling reference.  OTOH, the patch
> should suppress false positives with iterators, like:
> 
>    auto &candidate = *candidates.begin ();
> 
> and arguably that's more important than detecting some relatively
> obscure cases.  It's probably not worth it making the warning more
> complicated by, for instance, not warning when a fn returns 'int&'
> but takes 'const int&' (because then it can't return its argument).
> 
> Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk?

OK.

> gcc/cp/ChangeLog:
> 
> 	* call.cc (maybe_warn_dangling_reference): Don't warn when initializing
> 	a non-const lvalue reference.
> 
> gcc/testsuite/ChangeLog:
> 
> 	* g++.dg/cpp23/elision4.C: Remove dg-warning.
> 	* g++.dg/warn/Wdangling-reference1.C: Turn dg-warning into dg-bogus.
> 	* g++.dg/warn/Wdangling-reference7.C: New test.
> ---
>   gcc/cp/call.cc                                   | 10 ++++++++--
>   gcc/testsuite/g++.dg/cpp23/elision4.C            |  4 ++--
>   gcc/testsuite/g++.dg/warn/Wdangling-reference1.C |  4 ++--
>   gcc/testsuite/g++.dg/warn/Wdangling-reference7.C | 16 ++++++++++++++++
>   4 files changed, 28 insertions(+), 6 deletions(-)
>   create mode 100644 gcc/testsuite/g++.dg/warn/Wdangling-reference7.C
> 
> diff --git a/gcc/cp/call.cc b/gcc/cp/call.cc
> index bd3b64a7e26..ef618d5c485 100644
> --- a/gcc/cp/call.cc
> +++ b/gcc/cp/call.cc
> @@ -13679,8 +13679,14 @@ maybe_warn_dangling_reference (const_tree decl, tree init)
>   {
>     if (!warn_dangling_reference)
>       return;
> -  if (!(TYPE_REF_OBJ_P (TREE_TYPE (decl))
> -	|| std_pair_ref_ref_p (TREE_TYPE (decl))))
> +  tree type = TREE_TYPE (decl);
> +  /* Only warn if what we're initializing has type T&& or const T&, or
> +     std::pair<const T&, const T&>.  (A non-const lvalue reference can't
> +     bind to a temporary.)  */
> +  if (!((TYPE_REF_OBJ_P (type)
> +	 && (TYPE_REF_IS_RVALUE (type)
> +	     || CP_TYPE_CONST_P (TREE_TYPE (type))))
> +	|| std_pair_ref_ref_p (type)))
>       return;
>     /* Don't suppress the diagnostic just because the call comes from
>        a system header.  If the DECL is not in a system header, or if
> diff --git a/gcc/testsuite/g++.dg/cpp23/elision4.C b/gcc/testsuite/g++.dg/cpp23/elision4.C
> index d39053ad741..77dcffcdaad 100644
> --- a/gcc/testsuite/g++.dg/cpp23/elision4.C
> +++ b/gcc/testsuite/g++.dg/cpp23/elision4.C
> @@ -34,6 +34,6 @@ T& temporary2(T&& x) { return static_cast<T&>(x); }
>   void
>   test ()
>   {
> -  int& r1 = temporary1 (42); // { dg-warning "dangling reference" }
> -  int& r2 = temporary2 (42); // { dg-warning "dangling reference" }
> +  int& r1 = temporary1 (42);
> +  int& r2 = temporary2 (42);
>   }
> diff --git a/gcc/testsuite/g++.dg/warn/Wdangling-reference1.C b/gcc/testsuite/g++.dg/warn/Wdangling-reference1.C
> index 97c81ee716c..1718c28165e 100644
> --- a/gcc/testsuite/g++.dg/warn/Wdangling-reference1.C
> +++ b/gcc/testsuite/g++.dg/warn/Wdangling-reference1.C
> @@ -139,6 +139,6 @@ struct Y {
>   // x1 = Y::operator int&& (&TARGET_EXPR <D.2410, {}>)
>   int&& x1 = Y(); // { dg-warning "dangling reference" }
>   int&& x2 = Y{}; // { dg-warning "dangling reference" }
> -int& x3 = Y(); // { dg-warning "dangling reference" }
> -int& x4 = Y{}; // { dg-warning "dangling reference" }
> +int& x3 = Y(); // { dg-bogus "dangling reference" }
> +int& x4 = Y{}; // { dg-bogus "dangling reference" }
>   const int& t1 = Y().foo(10); // { dg-warning "dangling reference" }
> diff --git a/gcc/testsuite/g++.dg/warn/Wdangling-reference7.C b/gcc/testsuite/g++.dg/warn/Wdangling-reference7.C
> new file mode 100644
> index 00000000000..4b0de2d8670
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/warn/Wdangling-reference7.C
> @@ -0,0 +1,16 @@
> +// { dg-do compile { target c++11 } }
> +// { dg-options "-Wdangling-reference" }
> +
> +int& ref(const int&);
> +int&& rref(const int&);
> +
> +void
> +g ()
> +{
> +  const int& r1 = ref (1); // { dg-warning "dangling reference" }
> +  int& r2 = ref (2); // { dg-bogus "dangling reference" }
> +  auto& r3 = ref (3); // { dg-bogus "dangling reference" }
> +  int&& r4 = rref (4); // { dg-warning "dangling reference" }
> +  auto&& r5 = rref (5); // { dg-warning "dangling reference" }
> +  const int&& r6 = rref (6); // { dg-warning "dangling reference" }
> +}
> 
> base-commit: 0a7b437ca71e2721e9bcf070762fc54ef7991aeb


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

end of thread, other threads:[~2022-11-14 23:16 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-11-11 20:22 [PATCH] c++: Disable -Wdangling-reference when initing T& Marek Polacek
2022-11-14 23:16 ` 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).