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.
This commit is contained in:
parent
740cf7d6ab
commit
7e3ce73849
3 changed files with 94 additions and 6 deletions
|
@ -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
|
||||
|
|
|
@ -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
|
||||
|
|
38
gcc/testsuite/g++.dg/warn/Wdangling-reference6.C
Normal file
38
gcc/testsuite/g++.dg/warn/Wdangling-reference6.C
Normal file
|
@ -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;
|
||||
}
|
Loading…
Add table
Reference in a new issue