public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
From: Marek Polacek <polacek@redhat.com>
To: Jason Merrill <jason@redhat.com>
Cc: GCC Patches <gcc-patches@gcc.gnu.org>,
	Jonathan Wakely <jwakely@redhat.com>
Subject: [PATCH v2] c++: Implement -Wdangling-reference [PR106393]
Date: Tue, 25 Oct 2022 11:21:15 -0400	[thread overview]
Message-ID: <Y1f+66oVJSTeTkCc@redhat.com> (raw)
In-Reply-To: <b2c6adcb-c4be-ee73-6380-8b67312147a6@redhat.com>

On Mon, Oct 24, 2022 at 01:30:42PM -0400, Jason Merrill wrote:
> On 10/21/22 19:28, Marek Polacek wrote:
> > This patch implements a new experimental warning (enabled by -Wextra) to
> > detect references bound to temporaries whose lifetime has ended.  The
> 
> Great!
> 
> > primary motivation is the Note in
> > <https://en.cppreference.com/w/cpp/algorithm/max>:
> > 
> >    Capturing the result of std::max by reference produces a dangling reference
> >    if one of the parameters is a temporary and that parameter is returned:
> > 
> >    int n = 1;
> >    const int& r = std::max(n-1, n+1); // r is dangling
> > 
> > That's because both temporaries for n-1 and n+1 are destroyed at the end
> > of the full expression.  With this warning enabled, you'll get:
> > 
> > g.C:3:12: warning: possibly dangling reference to a temporary [-Wdangling-reference]
> >      3 | const int& r = std::max(n-1, n+1);
> >        |            ^
> > g.C:3:24: note: the temporary was destroyed at the end of the full expression 'std::max<int>((n - 1), (n + 1))'
> >      3 | const int& r = std::max(n-1, n+1);
> >        |                ~~~~~~~~^~~~~~~~~~
> > 
> > The warning works by checking if a reference is initialized with a function
> > that returns a reference, and at least one parameter of the function is
> > a reference that is bound to a temporary.  It assumes that such a function
> > actually returns one of its arguments!  (I added code to check_return_expr
> > to suppress the warning when we've seen the definition of the function
> > and we can say that it can return something other than its parameter.)
> 
> Hmm, that misses returning a reference to a subobject or container element
> that will also go away when the object is destroyed.

Yes :-(.  Fixed, tests added.  I'm just checking TREE_STATIC now.

> Does it also avoid a lot of false positives?

Not at all, I just thought it may be worth it.
 
> > It doesn't warn when the function in question is a member function, otherwise
> > it'd emit loads of warnings for valid code like obj.emplace<T>({0}, 0).
> 
> We had discussed warning if the object argument is a temporary (and for the
> above check, the function returns *this)?

Presumably you mean detecting something like this:

struct S {
  const S& self () { return *this; }
};
const S& s = S().self();

I don't currently have a way to detect it, can I steal a METHOD_TYPE flag
that says "this member function returns *this"?  Alternatively, walk its
DECL_SAVED_TREE and look for RETURN_EXPR?
 
> > It warns in member initializer lists as well:
> > 
> >    const int& f(const int& i) { return i; }
> >    struct S {
> >      const int &r; // { dg-warning "dangling reference" }
> >      S() : r(f(10)) { } // { dg-message "destroyed" }
> >    };
> > 
> > I've run the testsuite/bootstrap with the warning enabled by default.
> > There were just a few FAILs:
> > * g++.dg/warn/Wdangling-pointer-2.C
> > * 20_util/any/misc/any_cast.cc
> > * 20_util/forward/c_neg.cc
> > * 20_util/forward/f_neg.cc
> > * experimental/any/misc/any_cast.cc
> > all of these look like genuine bugs.  A bootstrap with the warning
> > enabled by default passed.
> > 
> > When testing a previous version of the patch, there were many FAILs in
> > libstdc++'s 22_locale/; all of them because the warning triggered on
> > 
> >    const test_type& obj = std::use_facet<test_type>(std::locale());
> > 
> > but this code looks valid -- std::use_facet doesn't return a reference
> > to its parameter.  Therefore I added code to suppress the warning when
> > the call is std::use_facet.  Now 22_locale/* pass even with the warning
> > on.  We could exclude more std:: functions like this if desirable.
> 
> Instead of adding special cases in the compiler, let's disable the warning
> around the definition of use_facet (and adjust the compiler as needed so
> that avoids the warning).

As I said in
<https://gcc.gnu.org/pipermail/gcc-patches/2022-October/604307.html>
I don't think it's possible without inventing an attribute (?).
 
> I was remembering range adaptors being a stated motivation for Nico's P2012,
> but looking back at the paper I now see that this problem was avoided for
> them by disallowing rvalue arguments to range composition.

Aha, and if you can't pass a temporary, you will not have this problem.

> > gcc/testsuite/ChangeLog:
> > 
> > 	* g++.dg/cpp23/elision4.C: Use -Wdangling-reference, add dg-warning.
> > 	* g++.dg/cpp23/elision7.C: Likewise.
> > 	* g++.dg/warn/Wdangling-reference1.C: New test.
> > 	* g++.dg/warn/Wdangling-reference2.C: New test.
> 
> Could use a test with a virtual base.

Added (fns yum/lox in Wdangling-reference1.C).
 
> > +static tree
> > +find_initializing_call_expr (tree expr)
> > +{
> > +  STRIP_NOPS (expr);
> > +  switch (TREE_CODE (expr))
> > +    {
> > +    case CALL_EXPR:
> > +      return expr;
> > +    case COMPOUND_EXPR:
> > +      return find_initializing_call_expr (TREE_OPERAND (expr, 1));
> > +    case COND_EXPR:
> > +      if (tree t = find_initializing_call_expr (TREE_OPERAND (expr, 1)))
> > +	return t;
> > +      return find_initializing_call_expr (TREE_OPERAND (expr, 2));
> 
> For COND_EXPR I think we want to check both sides, in case there are calls
> on both sides but only the second one has a problematic temporary.

Wow, good catch.  That was a bug.  I've fixed it by moving the
expr_represents_temporary_p checking into find_initializing_call_expr.
 
> > +    case PAREN_EXPR:
> > +      return find_initializing_call_expr (TREE_OPERAND (expr, 0));
> > +    default:
> > +      return NULL_TREE;
> > +    }
> > +}
> > +
> > +/* Implement -Wdangling-reference, to detect cases like
> > +
> > +     int n = 1;
> > +     const int& r = std::max(n - 1, n + 1); // r is dangling
> > +
> > +   This creates temporaries from the arguments, returns a reference to
> > +   one of the temporaries, but both temporaries are destroyed at the end
> > +   of the full expression.
> > +
> > +   This works by checking if a reference is initialized with a function
> > +   that returns a reference, and at least one parameter of the function
> > +   is a reference that is bound to a temporary.  It assumes that such a
> > +   function actually returns one of its arguments.
> > +
> > +   This warning doesn't warn when the function in question is a member
> > +   function.
> > +
> > +   DECL is the reference being initialized, CALL is the initializer.  */
> > +
> > +static void
> > +do_warn_dangling_reference (const_tree decl, tree call)
> > +{
> > +  if (!warn_dangling_reference)
> > +    return;
> > +  if (!TYPE_REF_P (TREE_TYPE (decl)))
> > +    return;
> > +  call = find_initializing_call_expr (call);
> > +  if (call == NULL_TREE)
> > +    return;
> > +
> > +  tree fndecl = cp_get_callee_fndecl_nofold (call);
> > +  if (!fndecl
> > +      || warning_suppressed_p (fndecl, OPT_Wdangling_reference)
> > +      /* Don't warn about member functions; the warning would trigger in
> > +	 valid code like
> > +	   std::any a(...);
> > +	   S& s = a.emplace<S>({0}, 0);
> > +	 which constructs a new object and returns a reference to it.  */
> > +      || DECL_NONSTATIC_MEMBER_FUNCTION_P (fndecl)
> > +      /* It seems unreasonable to warn about operator functions.  */
> > +      || DECL_OVERLOADED_OPERATOR_P (fndecl)
> 
> I guess I'd expect false positives on << and >> because of iostreams, do you
> see false positives with other operators?

This was just a guess.  The warning triggered in g++.dg/overload/operator6.C.
I suppose this could be limited to << and >>?  I'm not sure.

> > --- /dev/null
> > +++ b/gcc/testsuite/g++.dg/warn/Wdangling-reference1.C
> > @@ -0,0 +1,103 @@
> > +// PR c++/106393
> > +// { dg-do compile { target c++11 } }
> > +// { dg-options "-Wdangling-reference" }
> > +
> > +const int& f(const int& i) { return i; }
> > +const int& h(int);
> > +int g;
> > +const int& globref(const int&) { return g; }
> > +struct X {
> > +  int* i;
> > +  operator const int&() const { return *i; }
> > +};
> > +X x{&g};
> > +
> > +const int& r1 = f(10); // { dg-warning "dangling reference" }
> > +// r2 = _ZGR2r2_ = (int) *f ((const int &) &TARGET_EXPR <D.2429, 10>) + 1; (const int &) &_ZGR2r2_
> > +const int& r2 = f(10) + 1;
> > +// Don't warn here, we have
> > +//   r3 = f (X::operator const int& (&x))
> > +const int& r3 = f(x);
> > +// Don't warn here, because we've seen the definition of globref
> > +// and could figure out that it may not return one of its parms.
> > +// Questionable -- it can also hide bugs --, but it helps here.
> 
> We could suppress specifically for the case of returning a variable with
> static storage duration?

Done.
 
> > +const int& r4 = globref(1);
> > +const int& r5 = (42, f(10)); // { dg-warning "dangling reference" }
> > +const int& r6 = (f(10), 42);
> > +const int& r7 = (f(10)); // { dg-warning "dangling reference" }
> > +const int& r8 = g ? f(10) : f(9); // { dg-warning "dangling reference" }
> > +const int& r9 = (42, g ? f(10) : f(9)); // { dg-warning "dangling reference" }
> > +const int& r10 = (g ? f(10) : f(9), 42);
> > +// Binds to a reference temporary for r11.
> > +const int& r11 = g ? f(10) : 9;
> 
> Why no warning?

I don't think there's a dangling reference here, we get:
r11 = _ZGR3r11_ = g != 0 ? (int) *f ((const int &) &TARGET_EXPR <D.2389, 10>) : 9;, (const int &) &_ZGR3r11_;
 
> > +// Invalid, but we don't warn here yet.
> > +// r12 = f (f ((const int &) &TARGET_EXPR <D.2459, 1>))
> > +const int& r12 = f(f(1));
> 
> This should be a simple recursion?

Hmm, the inner call is just a sub-expression of the full-expression so
there you can still use the returned temporary.  But in this case the
temporary is used beyond the full-expression so it's invalid.  I've added

         if (tree r = find_initializing_call_expr (arg))
           if (cp_tree_equal (CALL_EXPR_FN (r), CALL_EXPR_FN (expr)))
             return expr;

so that we detect f(f(1)) but I'm dubious that this is actually useful.
I've added

  const int& r15 = rp(&f(1));
 
to the test where I think we can't warn.  Hence the cp_tree_equal.

Here's a v2 which fixes bugs in v1, but still doesn't handle member
functions in any way.

FWIW, bootstrapped/regtested on x86_64-pc-linux-gnu.

-- >8 --
This patch implements a new experimental warning (enabled by -Wextra) to
detect references bound to temporaries whose lifetime has ended.  The
primary motivation is the Note in
<https://en.cppreference.com/w/cpp/algorithm/max>:

  Capturing the result of std::max by reference produces a dangling reference
  if one of the parameters is a temporary and that parameter is returned:

  int n = 1;
  const int& r = std::max(n-1, n+1); // r is dangling

That's because both temporaries for n-1 and n+1 are destroyed at the end
of the full expression.  With this warning enabled, you'll get:

g.C:3:12: warning: possibly dangling reference to a temporary [-Wdangling-reference]
    3 | const int& r = std::max(n-1, n+1);
      |            ^
g.C:3:24: note: the temporary was destroyed at the end of the full expression 'std::max<int>((n - 1), (n + 1))'
    3 | const int& r = std::max(n-1, n+1);
      |                ~~~~~~~~^~~~~~~~~~

The warning works by checking if a reference is initialized with a function
that returns a reference, and at least one parameter of the function is
a reference that is bound to a temporary.  It assumes that such a function
actually returns one of its arguments!  (I added code to check_return_expr
to suppress the warning when we've seen the definition of the function
and we can say that it can return a variable with static storage
duration.)

It doesn't warn when the function in question is a member function, otherwise
it'd emit loads of warnings for valid code like obj.emplace<T>({0}, 0).

It warns in member initializer lists as well:

  const int& f(const int& i) { return i; }
  struct S {
    const int &r; // { dg-warning "dangling reference" }
    S() : r(f(10)) { } // { dg-message "destroyed" }
  };

I've run the testsuite/bootstrap with the warning enabled by default.
There were just a few FAILs:
* g++.dg/warn/Wdangling-pointer-2.C
* 20_util/any/misc/any_cast.cc
* 20_util/forward/c_neg.cc
* 20_util/forward/f_neg.cc
* experimental/any/misc/any_cast.cc
all of these look like genuine bugs.  A bootstrap with the warning
enabled by default passed.

When testing a previous version of the patch, there were many FAILs in
libstdc++'s 22_locale/; all of them because the warning triggered on

  const test_type& obj = std::use_facet<test_type>(std::locale());

but this code looks valid -- std::use_facet doesn't return a reference
to its parameter.  Therefore I added code to suppress the warning when
the call is std::use_facet.  Now 22_locale/* pass even with the warning
on.  We could exclude more std:: functions like this if desirable.

	PR c++/106393

gcc/c-family/ChangeLog:

	* c.opt (Wdangling-reference): New.

gcc/cp/ChangeLog:

	* call.cc (expr_represents_temporary_p): New, factored out of
	conv_binds_ref_to_temporary.
	(conv_binds_ref_to_temporary): Don't return false just because a ck_base
	is missing.  Use expr_represents_temporary_p.
	(find_initializing_call_expr): New.
	(do_warn_dangling_reference): New.
	(extend_ref_init_temps): Call do_warn_dangling_reference.
	* typeck.cc (check_return_expr): Suppress -Wdangling-reference
	warnings.

gcc/ChangeLog:

	* doc/invoke.texi: Document -Wdangling-reference.

gcc/testsuite/ChangeLog:

	* g++.dg/cpp23/elision4.C: Use -Wdangling-reference, add dg-warning.
	* g++.dg/cpp23/elision7.C: Likewise.
	* g++.dg/warn/Wdangling-reference1.C: New test.
	* g++.dg/warn/Wdangling-reference2.C: New test.
---
 gcc/c-family/c.opt                            |   4 +
 gcc/cp/call.cc                                | 144 ++++++++++++++++--
 gcc/cp/cp-tree.h                              |   4 +-
 gcc/cp/typeck.cc                              |  10 ++
 gcc/doc/invoke.texi                           |  34 ++++-
 gcc/testsuite/g++.dg/cpp23/elision4.C         |   5 +-
 gcc/testsuite/g++.dg/cpp23/elision7.C         |   3 +-
 .../g++.dg/warn/Wdangling-reference1.C        | 128 ++++++++++++++++
 .../g++.dg/warn/Wdangling-reference2.C        |  28 ++++
 9 files changed, 343 insertions(+), 17 deletions(-)
 create mode 100644 gcc/testsuite/g++.dg/warn/Wdangling-reference1.C
 create mode 100644 gcc/testsuite/g++.dg/warn/Wdangling-reference2.C

diff --git a/gcc/c-family/c.opt b/gcc/c-family/c.opt
index 01d480759ae..02d79991aeb 100644
--- a/gcc/c-family/c.opt
+++ b/gcc/c-family/c.opt
@@ -555,6 +555,10 @@ Wdangling-pointer=
 C ObjC C++ ObjC++ Joined RejectNegative UInteger Var(warn_dangling_pointer) Warning LangEnabledBy(C ObjC C++ ObjC++,Wall, 2, 0) IntegerRange(0, 2)
 Warn for uses of pointers to auto variables whose lifetime has ended.
 
+Wdangling-reference
+C++ ObjC++ Var(warn_dangling_reference) Warning LangEnabledBy(C++ ObjC++, Wextra)
+Warn when a reference is bound to a temporary whose lifetime has ended.
+
 Wdate-time
 C ObjC C++ ObjC++ CPP(warn_date_time) CppReason(CPP_W_DATE_TIME) Var(cpp_warn_date_time) Init(0) Warning
 Warn about __TIME__, __DATE__ and __TIMESTAMP__ usage.
diff --git a/gcc/cp/call.cc b/gcc/cp/call.cc
index 6a34e9c2ae1..cf17ee04cf7 100644
--- a/gcc/cp/call.cc
+++ b/gcc/cp/call.cc
@@ -9313,6 +9313,16 @@ conv_binds_ref_to_prvalue (conversion *c)
   return conv_is_prvalue (next_conversion (c));
 }
 
+/* True iff EXPR represents a (subobject of a) temporary.  */
+
+static bool
+expr_represents_temporary_p (tree expr)
+{
+  while (handled_component_p (expr))
+    expr = TREE_OPERAND (expr, 0);
+  return TREE_CODE (expr) == TARGET_EXPR;
+}
+
 /* True iff C is a conversion that binds a reference to a temporary.
    This is a superset of conv_binds_ref_to_prvalue: here we're also
    interested in xvalues.  */
@@ -9330,18 +9340,14 @@ conv_binds_ref_to_temporary (conversion *c)
        struct Derived : Base {};
        const Base& b(Derived{});
      where we bind 'b' to the Base subobject of a temporary object of type
-     Derived.  The subobject is an xvalue; the whole object is a prvalue.  */
-  if (c->kind != ck_base)
-    return false;
-  c = next_conversion (c);
-  if (c->kind == ck_identity && c->u.expr)
-    {
-      tree expr = c->u.expr;
-      while (handled_component_p (expr))
-	expr = TREE_OPERAND (expr, 0);
-      if (TREE_CODE (expr) == TARGET_EXPR)
-	return true;
-    }
+     Derived.  The subobject is an xvalue; the whole object is a prvalue.
+
+     The ck_base doesn't have to be present for cases like X{}.m.  */
+  if (c->kind == ck_base)
+    c = next_conversion (c);
+  if (c->kind == ck_identity && c->u.expr
+      && expr_represents_temporary_p (c->u.expr))
+    return true;
   return false;
 }
 
@@ -13428,6 +13434,117 @@ initialize_reference (tree type, tree expr,
   return expr;
 }
 
+/* Helper for do_warn_dangling_reference to find a non-nested CALL_EXPR
+   that initializes the LHS (and at least one of its arguments represents
+   a temporary), or NULL_TREE if none found.  For instance:
+
+     const int& r = (42, f(1)); // f(1)
+     const int& t = b ? f(1) : f(2); // f(1)
+     const int& u = b ? f(1) : f(g); // f(1)
+     const int& v = b ? f(g) : f(2); // f(2)
+     const int& w = b ? f(g) : f(g); // NULL_TREE
+     const int& y = (f(1), 42); // NULL_TREE
+     const int& z = f(f(1)); // f(f(1))
+
+   EXPR is the initializer.  */
+
+static tree
+find_initializing_call_expr (tree expr)
+{
+  STRIP_NOPS (expr);
+  switch (TREE_CODE (expr))
+    {
+    case CALL_EXPR:
+      for (int i = 0; i < call_expr_nargs (expr); ++i)
+	{
+	  /* We're looking to see if ARG is something like
+	      (const int &) &TARGET_EXPR <...>.  */
+	  tree arg = CALL_EXPR_ARG (expr, i);
+	  /* It could also be the same call taking a temporary.  */
+	  if (tree r = find_initializing_call_expr (arg))
+	    if (cp_tree_equal (CALL_EXPR_FN (r), CALL_EXPR_FN (expr)))
+	      return expr;
+	  STRIP_NOPS (arg);
+	  if (TREE_CODE (arg) == ADDR_EXPR)
+	    arg = TREE_OPERAND (arg, 0);
+	  if (expr_represents_temporary_p (arg))
+	    return expr;
+	}
+      return NULL_TREE;
+    case COMPOUND_EXPR:
+      return find_initializing_call_expr (TREE_OPERAND (expr, 1));
+    case COND_EXPR:
+      if (tree t = find_initializing_call_expr (TREE_OPERAND (expr, 1)))
+	return t;
+      return find_initializing_call_expr (TREE_OPERAND (expr, 2));
+    case PAREN_EXPR:
+      return find_initializing_call_expr (TREE_OPERAND (expr, 0));
+    default:
+      return NULL_TREE;
+    }
+}
+
+/* Implement -Wdangling-reference, to detect cases like
+
+     int n = 1;
+     const int& r = std::max(n - 1, n + 1); // r is dangling
+
+   This creates temporaries from the arguments, returns a reference to
+   one of the temporaries, but both temporaries are destroyed at the end
+   of the full expression.
+
+   This works by checking if a reference is initialized with a function
+   that returns a reference, and at least one parameter of the function
+   is a reference that is bound to a temporary.  It assumes that such a
+   function actually returns one of its arguments.
+
+   This warning doesn't warn when the function in question is a member
+   function.
+
+   DECL is the reference being initialized, CALL is the initializer.  */
+
+static void
+do_warn_dangling_reference (const_tree decl, tree call)
+{
+  if (!warn_dangling_reference)
+    return;
+  if (!TYPE_REF_P (TREE_TYPE (decl)))
+    return;
+  call = find_initializing_call_expr (call);
+  if (call == NULL_TREE)
+    return;
+
+  tree fndecl = cp_get_callee_fndecl_nofold (call);
+  if (!fndecl
+      || warning_suppressed_p (fndecl, OPT_Wdangling_reference)
+      /* Don't warn about member functions; the warning would trigger in
+	 valid code like
+	   std::any a(...);
+	   S& s = a.emplace<S>({0}, 0);
+	 which constructs a new object and returns a reference to it.  */
+      || DECL_NONSTATIC_MEMBER_FUNCTION_P (fndecl)
+      /* It seems unreasonable to warn about operator functions.  */
+      || DECL_OVERLOADED_OPERATOR_P (fndecl)
+      /* If the function doesn't return a reference, don't warn.  This can
+	 be e.g.
+	   const int& z = std::min({1, 2, 3, 4, 5, 6, 7});
+	 which doesn't dangle: std::min here returns an int.  */
+      || !TYPE_REF_P (TREE_TYPE (TREE_TYPE (fndecl))))
+    return;
+
+  /* Mitigate some known cases.  */
+  if (decl_in_std_namespace_p (fndecl))
+    if (tree name = DECL_NAME (fndecl))
+      if (id_equal (name, "use_facet"))
+	return;
+
+  auto_diagnostic_group d;
+  if (warning_at (DECL_SOURCE_LOCATION (decl), OPT_Wdangling_reference,
+		  "possibly dangling reference to a temporary"))
+    inform (EXPR_LOCATION (call), "the temporary was destroyed at "
+	    "the end of the full expression %qE", call);
+}
+
 /* If *P is an xvalue expression, prevent temporary lifetime extension if it
    gets used to initialize a reference.  */
 
@@ -13525,6 +13642,9 @@ extend_ref_init_temps (tree decl, tree init, vec<tree, va_gc> **cleanups,
   tree type = TREE_TYPE (init);
   if (processing_template_decl)
     return init;
+
+  do_warn_dangling_reference (decl, init);
+
   if (TYPE_REF_P (type))
     init = extend_ref_init_temps_1 (decl, init, cleanups, cond_guard);
   else
diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h
index 2cca20be6c1..d22c5e1b731 100644
--- a/gcc/cp/cp-tree.h
+++ b/gcc/cp/cp-tree.h
@@ -459,7 +459,6 @@ extern GTY(()) tree cp_global_trees[CPTI_MAX];
       TI_PENDING_TEMPLATE_FLAG.
       TEMPLATE_PARMS_FOR_INLINE.
       DELETE_EXPR_USE_VEC (in DELETE_EXPR).
-      (TREE_CALLS_NEW) (in _EXPR or _REF) (commented-out).
       ICS_ELLIPSIS_FLAG (in _CONV)
       DECL_INITIALIZED_P (in VAR_DECL)
       TYPENAME_IS_CLASS_P (in TYPENAME_TYPE)
@@ -4567,6 +4566,9 @@ get_vec_init_expr (tree t)
    When appearing in a CONSTRUCTOR, the expression is an unconverted
    compound literal.
 
+   When appearing in a CALL_EXPR, it means that it is a call to
+   a constructor.
+
    When appearing in a FIELD_DECL, it means that this field
    has been duly initialized in its constructor.  */
 #define TREE_HAS_CONSTRUCTOR(NODE) (TREE_LANG_FLAG_4 (NODE))
diff --git a/gcc/cp/typeck.cc b/gcc/cp/typeck.cc
index ab6979bcc50..54fac880d8c 100644
--- a/gcc/cp/typeck.cc
+++ b/gcc/cp/typeck.cc
@@ -11246,6 +11246,16 @@ check_return_expr (tree retval, bool *no_warning)
   if (processing_template_decl)
     return saved_retval;
 
+  /* A naive attempt to reduce the number of -Wdangling-reference false
+     positives: if we know that this function can return a variable with
+     static storage duration rather than one of its parameters, suppress
+     the warning.  */
+  if (warn_dangling_reference
+      && TYPE_REF_P (functype)
+      && bare_retval
+      && TREE_STATIC (bare_retval))
+    suppress_warning (current_function_decl, OPT_Wdangling_reference);
+
   /* Actually copy the value returned into the appropriate location.  */
   if (retval && retval != result)
     retval = cp_build_init_expr (result, retval);
diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
index 64f77e8367a..e94fe20779a 100644
--- a/gcc/doc/invoke.texi
+++ b/gcc/doc/invoke.texi
@@ -249,7 +249,8 @@ in the following sections.
 -Wno-class-conversion  -Wclass-memaccess @gol
 -Wcomma-subscript  -Wconditionally-supported @gol
 -Wno-conversion-null  -Wctad-maybe-unsupported @gol
--Wctor-dtor-privacy  -Wno-delete-incomplete @gol
+-Wctor-dtor-privacy  -Wdangling-reference @gol
+-Wno-delete-incomplete @gol
 -Wdelete-non-virtual-dtor  -Wno-deprecated-array-compare @gol
 -Wdeprecated-copy -Wdeprecated-copy-dtor @gol
 -Wno-deprecated-enum-enum-conversion -Wno-deprecated-enum-float-conversion @gol
@@ -3627,6 +3628,36 @@ public static member functions.  Also warn if there are no non-private
 methods, and there's at least one private member function that isn't
 a constructor or destructor.
 
+@item -Wdangling-reference @r{(C++ and Objective-C++ only)}
+@opindex Wdangling-reference
+@opindex Wno-dangling-reference
+Warn when a reference is bound to a temporary whose lifetime has ended.
+For example:
+
+@smallexample
+int n = 1;
+const int& r = std::max(n - 1, n + 1); // r is dangling
+@end smallexample
+
+In the example above, two temporaries are created, one for each
+argument, and a reference to one of the temporaries is returned.
+However, both temporaries are destroyed at the end of the full
+expression, so the reference @code{r} is dangling.  This warning
+also detects dangling references in member initializer lists:
+
+@smallexample
+const int& f(const int& i) @{ return i; @}
+struct S @{
+  const int &r; // r is dangling
+  S() : r(f(10)) @{ @}
+@};
+@end smallexample
+
+Member functions are not checked.  Certain standard functions, such
+as @code{std::use_facet}, are also excluded from checking.
+
+This warning is enabled by @option{-Wextra}.
+
 @item -Wdelete-non-virtual-dtor @r{(C++ and Objective-C++ only)}
 @opindex Wdelete-non-virtual-dtor
 @opindex Wno-delete-non-virtual-dtor
@@ -5936,6 +5967,7 @@ name is still supported, but the newer name is more descriptive.)
 
 @gccoptlist{-Wclobbered  @gol
 -Wcast-function-type  @gol
+-Wdangling-reference @r{(C++ only)} @gol
 -Wdeprecated-copy @r{(C++ only)} @gol
 -Wempty-body  @gol
 -Wenum-conversion @r{(C only)} @gol
diff --git a/gcc/testsuite/g++.dg/cpp23/elision4.C b/gcc/testsuite/g++.dg/cpp23/elision4.C
index c19b86b8b5f..d39053ad741 100644
--- a/gcc/testsuite/g++.dg/cpp23/elision4.C
+++ b/gcc/testsuite/g++.dg/cpp23/elision4.C
@@ -1,5 +1,6 @@
 // PR c++/101165 - P2266R1 - Simpler implicit move
 // { dg-do compile { target c++23 } }
+// { dg-options "-Wdangling-reference" }
 // Test from P2266R1, $ 5.2. LibreOffice OString constructor.
 
 struct X {
@@ -33,6 +34,6 @@ T& temporary2(T&& x) { return static_cast<T&>(x); }
 void
 test ()
 {
-  int& r1 = temporary1 (42);
-  int& r2 = temporary2 (42);
+  int& r1 = temporary1 (42); // { dg-warning "dangling reference" }
+  int& r2 = temporary2 (42); // { dg-warning "dangling reference" }
 }
diff --git a/gcc/testsuite/g++.dg/cpp23/elision7.C b/gcc/testsuite/g++.dg/cpp23/elision7.C
index 19fa89ae133..0045842b34f 100644
--- a/gcc/testsuite/g++.dg/cpp23/elision7.C
+++ b/gcc/testsuite/g++.dg/cpp23/elision7.C
@@ -1,5 +1,6 @@
 // PR c++/101165 - P2266R1 - Simpler implicit move
 // { dg-do compile { target c++23 } }
+// { dg-options "-Wdangling-reference" }
 
 struct X {
   X ();
@@ -68,5 +69,5 @@ f7 (T &&t)
 void
 do_f7 ()
 {
-  const int &x = f7 (0);
+  const int &x = f7 (0); // { dg-warning "dangling reference" }
 }
diff --git a/gcc/testsuite/g++.dg/warn/Wdangling-reference1.C b/gcc/testsuite/g++.dg/warn/Wdangling-reference1.C
new file mode 100644
index 00000000000..00bee232d9f
--- /dev/null
+++ b/gcc/testsuite/g++.dg/warn/Wdangling-reference1.C
@@ -0,0 +1,128 @@
+// PR c++/106393
+// { dg-do compile { target c++11 } }
+// { dg-options "-Wdangling-reference" }
+
+const int& f(const int& i) { return i; }
+const int& h(int);
+const int& rp(const int *);
+int g;
+const int& globref(const int&) { return g; }
+struct X {
+  int* i;
+  operator const int&() const { return *i; }
+};
+X x{&g};
+
+const int& r1 = f(10); // { dg-warning "dangling reference" }
+// r2 = _ZGR2r2_ = (int) *f ((const int &) &TARGET_EXPR <D.2429, 10>) + 1; (const int &) &_ZGR2r2_
+const int& r2 = f(10) + 1;
+// Don't warn here, we have
+//   r3 = f (X::operator const int& (&x))
+const int& r3 = f(x);
+// Don't warn here, because we've seen the definition of globref
+// and could figure out that it may not return one of its parms.
+// Questionable -- it can also hide bugs --, but it helps here.
+const int& r4 = globref(1);
+const int& r5 = (42, f(10)); // { dg-warning "dangling reference" }
+const int& r6 = (f(10), 42);
+const int& r7 = (f(10)); // { dg-warning "dangling reference" }
+const int& r8 = g ? f(10) : f(9); // { dg-warning "dangling reference" }
+const int& r9 = (42, g ? f(10) : f(9)); // { dg-warning "dangling reference" }
+const int& r10 = (g ? f(10) : f(9), 42);
+// Binds to a reference temporary for r11.
+const int& r11 = g ? f(10) : 9;
+const int& r11_ = g ? 9 : f(10);
+// r12 = f (f ((const int &) &TARGET_EXPR <D.2459, 1>))
+const int& r12 = f(f(1)); // { dg-warning "dangling reference" }
+const int& r13 = f(g ? f(1) : f(2)); // { dg-warning "dangling reference" }
+const int& r14 = f(*&f(1)); // { dg-warning "dangling reference" }
+const int& r15 = rp(&f(1));
+const int& r16 = rp(&f(g));
+const int& r17 = h(f(1));
+// Other forms of initializers.
+const int& r18(f(10)); // { dg-warning "dangling reference" }
+const int& r19(f(10)); // { dg-warning "dangling reference" }
+// Returns a ref, but doesn't have a parameter of reference type.
+const int& r20 = h(10);
+const int& r21 = g ? h(10) : f(10); // { dg-warning "dangling reference" }
+const int& r22 = g ? f(10) : h(10); // { dg-warning "dangling reference" }
+const int& r23 = g ? h(10) : (1, f(10)); // { dg-warning "dangling reference" }
+const int& r24 = g ? (1, f(10)) : h(10); // { dg-warning "dangling reference" }
+
+// OK: the reference is bound to the 10 so still valid at the point
+// where it's copied into i1.
+int i1 = f(10);
+
+int
+test1 ()
+{
+  const int &lr = f(10); // { dg-warning "dangling reference" }
+  int i2 = f(10);
+  return lr;
+}
+
+struct B { };
+struct D : B { };
+struct C {
+  D d;
+};
+
+C c;
+D d;
+
+using U = D[3];
+
+const B& frotz(const D&);
+const B& b1 = frotz(C{}.d); // { dg-warning "dangling reference" }
+const B& b2 = frotz(D{}); // { dg-warning "dangling reference" }
+const B& b3 = frotz(c.d);
+const B& b4 = frotz(d);
+const B& b5 = frotz(U{}[0]); // { dg-warning "dangling reference" }
+
+// Try returning a subobject.
+const B& bar (const D& d) { return d; }
+const B& b6 = bar (D{}); // { dg-warning "dangling reference" }
+const B& baz (const C& c) { return c.d; }
+const B& b7 = baz (C{}); // { dg-warning "dangling reference" }
+const D& qux (const C& c) { return c.d; }
+const D& d1 = qux (C{}); // { dg-warning "dangling reference" }
+
+struct E {
+  E(int);
+};
+const E& operator*(const E&);
+const E& b8 = *E(1);
+
+struct F : virtual B { };
+struct G : virtual B { };
+struct H : F, G { };
+const B& yum (const F& f) { return f; }
+const B& b9 = yum (F{}); // { dg-warning "dangling reference" }
+const B& lox (const H& h) { return h; }
+const B& b10 = lox (H{}); // { dg-warning "dangling reference" }
+
+struct S {
+  const int &r; // { dg-warning "dangling reference" }
+  S() : r(f(10)) { } // { dg-message "destroyed" }
+};
+
+// From cppreference.
+template<class T>
+const T& max(const T& a, const T& b)
+{
+    return (a < b) ? b : a;
+}
+
+int n = 1;
+const int& refmax = max(n - 1, n + 1); // { dg-warning "dangling reference" }
+
+// Don't warn about member functions.
+struct Y {
+  operator int&&();
+  const int& foo(const int&);
+};
+
+// x1 = Y::operator int&& (&TARGET_EXPR <D.2410, {}>)
+int&& x1 = Y();
+int&& x2 = Y{};
+const int& t1 = Y().foo(10);
diff --git a/gcc/testsuite/g++.dg/warn/Wdangling-reference2.C b/gcc/testsuite/g++.dg/warn/Wdangling-reference2.C
new file mode 100644
index 00000000000..dafdb43f1b9
--- /dev/null
+++ b/gcc/testsuite/g++.dg/warn/Wdangling-reference2.C
@@ -0,0 +1,28 @@
+// PR c++/106393
+// { dg-do compile { target c++11 } }
+// { dg-options "-Wdangling-reference" }
+
+namespace std {
+struct any {};
+template <typename _ValueType> _ValueType any_cast(any &&);
+template <typename _Tp> struct remove_reference { using type = _Tp; };
+template <typename _Tp> _Tp forward(typename remove_reference<_Tp>::type);
+template <typename _Tp> typename remove_reference<_Tp>::type move(_Tp);
+} // namespace std
+
+const int &r = std::any_cast<int&>(std::any()); // { dg-warning "dangling reference" }
+
+template <class T> struct C {
+  T t_; // { dg-warning "dangling reference" }
+  C(T);
+  template <class U> C(U c) : t_(std::forward<T>(c.t_)) {}
+};
+struct A {};
+struct B {
+  B(A);
+};
+int main() {
+  A a;
+  C<A> ca(a);
+  C<B &&>(std::move(ca));
+}

base-commit: 4c5b1160776382772fc0a33130dfaf621699fdbf
-- 
2.37.3


  parent reply	other threads:[~2022-10-25 15:21 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-10-21 23:28 [PATCH] " Marek Polacek
2022-10-24 17:30 ` Jason Merrill
2022-10-25 11:34   ` Jonathan Wakely
2022-10-25 13:14     ` Marek Polacek
2022-10-25 15:39       ` Jason Merrill
2022-10-25 16:35         ` Marek Polacek
2022-10-25 15:21   ` Marek Polacek [this message]
2022-10-25 15:53     ` [PATCH v2] " Jason Merrill
2022-10-26 16:10       ` [PATCH v3] " Marek Polacek
2022-10-26 16:42         ` Jason Merrill
2022-10-26 18:26           ` [PATCH v4] " Marek Polacek
2022-10-26 18:42             ` Jason Merrill
2022-10-25 11:50 ` [PATCH] " Jonathan Wakely
2022-10-25 15:24   ` Marek Polacek

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=Y1f+66oVJSTeTkCc@redhat.com \
    --to=polacek@redhat.com \
    --cc=gcc-patches@gcc.gnu.org \
    --cc=jason@redhat.com \
    --cc=jwakely@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).