[gcc r13-3883] c++: Extend -Wdangling-reference for std::minmax

[gcc r13-3883] c++: Extend -Wdangling-reference for std::minmax

[Marek Polacek]

https://gcc.gnu.org/g:7e3ce73849fef8b50efb427ec96f317e88c0e6cf

commit r13-3883-g7e3ce73849fef8b50efb427ec96f317e88c0e6cf
Author: Marek Polacek <polacek@redhat.com>
Date:   Wed Nov 9 19:35:26 2022 -0500

    c++: Extend -Wdangling-reference for std::minmax
    
    This patch extends -Wdangling-reference to also warn for
    
      auto v = std::minmax(1, 2);
    
    which dangles because this overload of std::minmax returns
    a std::pair<const int&, const int&> where the two references are
    bound to the temporaries created for the arguments of std::minmax.
    This is a common footgun, also described at
    <https://en.cppreference.com/w/cpp/algorithm/minmax> in Notes.
    
    It works by extending do_warn_dangling_reference to also warn when the
    function returns a std::pair<const T&, const T&>.  std_pair_ref_ref_p
    is a new helper to check that.
    
    gcc/cp/ChangeLog:
    
            * call.cc (std_pair_ref_ref_p): New.
            (do_warn_dangling_reference): Also warn when the function returns
            std::pair<const T&, const T&>.  Recurse into TARGET_EXPR_INITIAL.
            (maybe_warn_dangling_reference): Don't return early if we're
            initializing a std_pair_ref_ref_p.
    
    gcc/ChangeLog:
    
            * doc/gcc/gcc-command-options/options-controlling-c++-dialect.rst:
            Extend the description of -Wdangling-reference.
    
    gcc/testsuite/ChangeLog:
    
            * g++.dg/warn/Wdangling-reference6.C: New test.

Diff:
---
 gcc/cp/call.cc                                     | 52 +++++++++++++++++++---
 .../options-controlling-c++-dialect.rst            | 10 +++++
 gcc/testsuite/g++.dg/warn/Wdangling-reference6.C   | 38 ++++++++++++++++
 3 files changed, 94 insertions(+), 6 deletions(-)

diff --git a/gcc/cp/call.cc b/gcc/cp/call.cc
index 492db9b59ad..bd3b64a7e26 100644
--- a/gcc/cp/call.cc
+++ b/gcc/cp/call.cc
@@ -13527,6 +13527,34 @@ initialize_reference (tree type, tree expr,
   return expr;
 }
 
+/* Return true if T is std::pair<const T&, const T&>.  */
+
+static bool
+std_pair_ref_ref_p (tree t)
+{
+  /* First, check if we have std::pair.  */
+  if (!NON_UNION_CLASS_TYPE_P (t)
+      || !CLASSTYPE_TEMPLATE_INSTANTIATION (t))
+    return false;
+  tree tdecl = TYPE_NAME (TYPE_MAIN_VARIANT (t));
+  if (!decl_in_std_namespace_p (tdecl))
+    return false;
+  tree name = DECL_NAME (tdecl);
+  if (!name || !id_equal (name, "pair"))
+    return false;
+
+  /* Now see if the template arguments are both const T&.  */
+  tree args = CLASSTYPE_TI_ARGS (t);
+  if (TREE_VEC_LENGTH (args) != 2)
+    return false;
+  for (int i = 0; i < 2; i++)
+    if (!TYPE_REF_OBJ_P (TREE_VEC_ELT (args, i))
+	|| !CP_TYPE_CONST_P (TREE_TYPE (TREE_VEC_ELT (args, i))))
+      return false;
+
+  return true;
+}
+
 /* Helper for maybe_warn_dangling_reference to find a problematic CALL_EXPR
    that initializes the LHS (and at least one of its arguments represents
    a temporary, as outlined in maybe_warn_dangling_reference), or NULL_TREE
@@ -13556,11 +13584,6 @@ do_warn_dangling_reference (tree expr)
 	    || warning_suppressed_p (fndecl, OPT_Wdangling_reference)
 	    || !warning_enabled_at (DECL_SOURCE_LOCATION (fndecl),
 				    OPT_Wdangling_reference)
-	    /* 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_OBJ_P (TREE_TYPE (TREE_TYPE (fndecl)))
 	    /* Don't emit a false positive for:
 		std::vector<int> v = ...;
 		std::vector<int>::const_iterator it = v.begin();
@@ -13573,6 +13596,20 @@ do_warn_dangling_reference (tree expr)
 		&& DECL_OVERLOADED_OPERATOR_IS (fndecl, INDIRECT_REF)))
 	  return NULL_TREE;
 
+	tree rettype = TREE_TYPE (TREE_TYPE (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.
+
+	   If the function returns a std::pair<const T&, const T&>, we
+	   warn, to detect e.g.
+	     std::pair<const int&, const int&> v = std::minmax(1, 2);
+	   which also creates a dangling reference, because std::minmax
+	   returns std::pair<const T&, const T&>(b, a).  */
+	if (!(TYPE_REF_OBJ_P (rettype) || std_pair_ref_ref_p (rettype)))
+	  return NULL_TREE;
+
 	/* Here we're looking to see if any of the arguments is a temporary
 	   initializing a reference parameter.  */
 	for (int i = 0; i < call_expr_nargs (expr); ++i)
@@ -13614,6 +13651,8 @@ do_warn_dangling_reference (tree expr)
       return do_warn_dangling_reference (TREE_OPERAND (expr, 2));
     case PAREN_EXPR:
       return do_warn_dangling_reference (TREE_OPERAND (expr, 0));
+    case TARGET_EXPR:
+      return do_warn_dangling_reference (TARGET_EXPR_INITIAL (expr));
     default:
       return NULL_TREE;
     }
@@ -13640,7 +13679,8 @@ maybe_warn_dangling_reference (const_tree decl, tree init)
 {
   if (!warn_dangling_reference)
     return;
-  if (!TYPE_REF_P (TREE_TYPE (decl)))
+  if (!(TYPE_REF_OBJ_P (TREE_TYPE (decl))
+	|| std_pair_ref_ref_p (TREE_TYPE (decl))))
     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/doc/gcc/gcc-command-options/options-controlling-c++-dialect.rst b/gcc/doc/gcc/gcc-command-options/options-controlling-c++-dialect.rst
index 5b05d31aae9..8d2a2789ef6 100644
--- a/gcc/doc/gcc/gcc-command-options/options-controlling-c++-dialect.rst
+++ b/gcc/doc/gcc/gcc-command-options/options-controlling-c++-dialect.rst
@@ -855,6 +855,16 @@ In addition, these warning options have meanings only for C++ programs:
     const T& foo (const T&) { ... }
     #pragma GCC diagnostic pop
 
+  :option:`-Wdangling-reference` also warns about code like
+
+  .. code-block:: c++
+
+    auto p = std::minmax(1, 2);
+
+  where ``std::minmax`` returns ``std::pair<const int&, const int&>``, and
+  both references dangle after the end of the full expression that contains
+  the call to `std::minmax``.
+
   This warning is enabled by :option:`-Wall`.
 
 .. option:: -Wno-dangling-reference
diff --git a/gcc/testsuite/g++.dg/warn/Wdangling-reference6.C b/gcc/testsuite/g++.dg/warn/Wdangling-reference6.C
new file mode 100644
index 00000000000..bf849e290d9
--- /dev/null
+++ b/gcc/testsuite/g++.dg/warn/Wdangling-reference6.C
@@ -0,0 +1,38 @@
+// { dg-do compile { target c++17 } }
+// { dg-options "-Wdangling-reference" }
+// Test -Wdangling-reference with std::minmax.
+
+#include <algorithm>
+
+using U = std::pair<const int&, const int&>;
+
+int
+fn1 ()
+{
+  std::pair<const int&, const int&> v = std::minmax(1, 2); // { dg-warning "dangling reference" }
+  U v2 = std::minmax(1, 2); // { dg-warning "dangling reference" }
+  auto v3 = std::minmax(1, 2); // { dg-warning "dangling reference" }
+  return v.first + v2.second + v3.first;
+}
+
+int
+fn2 ()
+{
+  int n = 1;
+  auto p = std::minmax(n, n + 1); // { dg-warning "dangling reference" }
+  int m = p.first; // ok
+  int x = p.second; // undefined behavior
+
+  // Note that structured bindings have the same issue
+  auto [mm, xx] = std::minmax(n, n + 1); // { dg-warning "dangling reference" }
+  (void) xx; // undefined behavior
+
+  return m + x;
+}
+
+int
+fn3 ()
+{
+  auto v = std::minmax({1, 2});
+  return v.first;
+}