* [PATCH] c++: Implement -Wdangling-reference [PR106393]
@ 2022-10-21 23:28 Marek Polacek
2022-10-24 17:30 ` Jason Merrill
2022-10-25 11:50 ` [PATCH] " Jonathan Wakely
0 siblings, 2 replies; 14+ messages in thread
From: Marek Polacek @ 2022-10-21 23:28 UTC (permalink / raw)
To: Jason Merrill, GCC Patches, Jonathan Wakely
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 something other than its parameter.)
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.
Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk?
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 | 138 ++++++++++++++++--
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 | 103 +++++++++++++
.../g++.dg/warn/Wdangling-reference2.C | 28 ++++
9 files changed, 312 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..43e607987a0 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,111 @@ 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, 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& z = (f(1), 42); // NULL_TREE
+
+ EXPR is the initializer. */
+
+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));
+ 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;
+
+ for (int i = 0; i < call_expr_nargs (call); ++i)
+ {
+ /* We're looking to see if ARG is something like
+ (const int &) &TARGET_EXPR <...>. */
+ tree arg = CALL_EXPR_ARG (call, i);
+ STRIP_NOPS (arg);
+ if (TREE_CODE (arg) == ADDR_EXPR)
+ arg = TREE_OPERAND (arg, 0);
+ if (expr_represents_temporary_p (arg))
+ {
+ 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);
+ return;
+ }
+ }
+}
+
/* If *P is an xvalue expression, prevent temporary lifetime extension if it
gets used to initialize a reference. */
@@ -13525,6 +13636,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 60a25101049..8ddf55ce2b0 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)
@@ -4558,6 +4557,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 16e7d85793d..5a22eee8ebf 100644
--- a/gcc/cp/typeck.cc
+++ b/gcc/cp/typeck.cc
@@ -11238,6 +11238,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 something other
+ than one of its parameters, suppress the warning. */
+ if (warn_dangling_reference
+ && TYPE_REF_P (functype)
+ && bare_retval
+ && (!REFERENCE_REF_P (bare_retval)
+ || TREE_CODE (TREE_OPERAND (bare_retval, 0)) != PARM_DECL))
+ 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 09548c4528c..28bb395ce6c 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..e0324d38f67
--- /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.
+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;
+// Invalid, but we don't warn here yet.
+// r12 = f (f ((const int &) &TARGET_EXPR <D.2459, 1>))
+const int& r12 = f(f(1));
+const int& r13 = h(f(1));
+// Other forms of initializers.
+const int& r14(f(10)); // { dg-warning "dangling reference" }
+const int& r15(f(10)); // { dg-warning "dangling reference" }
+// Returns a ref, but doesn't have a parameter of reference type.
+const int& r16 = h(10);
+
+// 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" }
+
+struct E {
+ E(int);
+};
+const E& operator*(const E&);
+const E& b6 = *E(1);
+
+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& r20 = 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: 5792208f5124f687376f25798668d105d7ddb270
--
2.37.3
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] c++: Implement -Wdangling-reference [PR106393]
2022-10-21 23:28 [PATCH] c++: Implement -Wdangling-reference [PR106393] Marek Polacek
@ 2022-10-24 17:30 ` Jason Merrill
2022-10-25 11:34 ` Jonathan Wakely
2022-10-25 15:21 ` [PATCH v2] " Marek Polacek
2022-10-25 11:50 ` [PATCH] " Jonathan Wakely
1 sibling, 2 replies; 14+ messages in thread
From: Jason Merrill @ 2022-10-24 17:30 UTC (permalink / raw)
To: Marek Polacek, GCC Patches, Jonathan Wakely
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. Does it
also avoid a lot of false positives?
> 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)?
> 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).
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.
> Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk?
>
> 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.
Could use a test with a virtual base.
> ---
> gcc/c-family/c.opt | 4 +
> gcc/cp/call.cc | 138 ++++++++++++++++--
> 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 | 103 +++++++++++++
> .../g++.dg/warn/Wdangling-reference2.C | 28 ++++
> 9 files changed, 312 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..43e607987a0 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,111 @@ 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, 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& z = (f(1), 42); // NULL_TREE
> +
> + EXPR is the initializer. */
> +
> +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.
> + 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?
> + /* 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;
> +
> + for (int i = 0; i < call_expr_nargs (call); ++i)
> + {
> + /* We're looking to see if ARG is something like
> + (const int &) &TARGET_EXPR <...>. */
> + tree arg = CALL_EXPR_ARG (call, i);
> + STRIP_NOPS (arg);
> + if (TREE_CODE (arg) == ADDR_EXPR)
> + arg = TREE_OPERAND (arg, 0);
> + if (expr_represents_temporary_p (arg))
> + {
> + 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);
> + return;
> + }
> + }
> +}
> +
> /* If *P is an xvalue expression, prevent temporary lifetime extension if it
> gets used to initialize a reference. */
>
> @@ -13525,6 +13636,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 60a25101049..8ddf55ce2b0 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)
> @@ -4558,6 +4557,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 16e7d85793d..5a22eee8ebf 100644
> --- a/gcc/cp/typeck.cc
> +++ b/gcc/cp/typeck.cc
> @@ -11238,6 +11238,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 something other
> + than one of its parameters, suppress the warning. */
> + if (warn_dangling_reference
> + && TYPE_REF_P (functype)
> + && bare_retval
> + && (!REFERENCE_REF_P (bare_retval)
> + || TREE_CODE (TREE_OPERAND (bare_retval, 0)) != PARM_DECL))
> + 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 09548c4528c..28bb395ce6c 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..e0324d38f67
> --- /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?
> +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?
> +// 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?
> +const int& r13 = h(f(1));
> +// Other forms of initializers.
> +const int& r14(f(10)); // { dg-warning "dangling reference" }
> +const int& r15(f(10)); // { dg-warning "dangling reference" }
> +// Returns a ref, but doesn't have a parameter of reference type.
> +const int& r16 = h(10);
> +
> +// 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" }
> +
> +struct E {
> + E(int);
> +};
> +const E& operator*(const E&);
> +const E& b6 = *E(1);
> +
> +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& r20 = 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: 5792208f5124f687376f25798668d105d7ddb270
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] c++: Implement -Wdangling-reference [PR106393]
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:21 ` [PATCH v2] " Marek Polacek
1 sibling, 1 reply; 14+ messages in thread
From: Jonathan Wakely @ 2022-10-25 11:34 UTC (permalink / raw)
To: Jason Merrill; +Cc: Marek Polacek, GCC Patches
On Mon, 24 Oct 2022 at 18:30, Jason Merrill wrote:
>
> On 10/21/22 19:28, Marek Polacek wrote:
> > 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).
I assume you mean using #pragma here. If we disable it around the
definition of use_facet, will that disable it for callers of
use_facet, or only within the definition of use_facet itself?
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] c++: Implement -Wdangling-reference [PR106393]
2022-10-21 23:28 [PATCH] c++: Implement -Wdangling-reference [PR106393] Marek Polacek
2022-10-24 17:30 ` Jason Merrill
@ 2022-10-25 11:50 ` Jonathan Wakely
2022-10-25 15:24 ` Marek Polacek
1 sibling, 1 reply; 14+ messages in thread
From: Jonathan Wakely @ 2022-10-25 11:50 UTC (permalink / raw)
To: Marek Polacek; +Cc: Jason Merrill, GCC Patches
[-- Attachment #1: Type: text/plain, Size: 809 bytes --]
On Sat, 22 Oct 2022 at 00:28, Marek Polacek wrote:
> 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
These two are XFAIL tests, they're checking for a required static
assert due to misusing std::forward, so I don't think we want to
change them to remove a "bug" (they're meant to be buggy).
I think they should use -Wno-dangling-reference
> * experimental/any/misc/any_cast.cc
> all of these look like genuine bugs. A bootstrap with the warning
> enabled by default passed.
I've attached a patch for the two any_cast.cc tests that avoids
creating dangling references, without changing the intended behaviour
of the tests.
[-- Attachment #2: patch.txt --]
[-- Type: text/plain, Size: 1755 bytes --]
diff --git a/libstdc++-v3/testsuite/20_util/any/misc/any_cast.cc b/libstdc++-v3/testsuite/20_util/any/misc/any_cast.cc
index 8d63dfbba9b..43f90c336c9 100644
--- a/libstdc++-v3/testsuite/20_util/any/misc/any_cast.cc
+++ b/libstdc++-v3/testsuite/20_util/any/misc/any_cast.cc
@@ -93,7 +93,8 @@ void test03()
MoveEnabled m;
MoveEnabled m2 = any_cast<MoveEnabled>(any(m));
VERIFY(move_count == 1);
- MoveEnabled&& m3 = any_cast<MoveEnabled&&>(any(m));
+ auto rvalue = [](MoveEnabled&&) { };
+ rvalue(any_cast<MoveEnabled&&>(any(m))); // No move construction, just a cast.
VERIFY(move_count == 1);
}
diff --git a/libstdc++-v3/testsuite/experimental/any/misc/any_cast.cc b/libstdc++-v3/testsuite/experimental/any/misc/any_cast.cc
index f3ce83ad702..98347f6d37d 100644
--- a/libstdc++-v3/testsuite/experimental/any/misc/any_cast.cc
+++ b/libstdc++-v3/testsuite/experimental/any/misc/any_cast.cc
@@ -92,7 +92,8 @@ void test03()
MoveEnabled m;
MoveEnabled m2 = any_cast<MoveEnabled>(any(m));
VERIFY(move_count == 1);
- MoveEnabled&& m3 = any_cast<MoveEnabled&&>(any(m));
+ auto rvalue = [](MoveEnabled&&) { }; // Can only be called with an rvalue.
+ rvalue(any_cast<MoveEnabled&&>(any(m))); // Doesn't move, just casts to rval.
VERIFY(move_count == 1);
struct MoveDeleted
{
@@ -101,8 +102,9 @@ void test03()
MoveDeleted(const MoveDeleted&) = default;
};
MoveDeleted md;
- MoveDeleted&& md2 = any_cast<MoveDeleted>(any(std::move(md)));
- MoveDeleted&& md3 = any_cast<MoveDeleted&&>(any(std::move(md)));
+ auto rvalue2 = [](MoveDeleted&&) { }; // Can only be called with an rvalue.
+ rvalue2(any_cast<MoveDeleted>(any(std::move(md))));
+ rvalue2(any_cast<MoveDeleted&&>(any(std::move(md))));
}
void test05()
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] c++: Implement -Wdangling-reference [PR106393]
2022-10-25 11:34 ` Jonathan Wakely
@ 2022-10-25 13:14 ` Marek Polacek
2022-10-25 15:39 ` Jason Merrill
0 siblings, 1 reply; 14+ messages in thread
From: Marek Polacek @ 2022-10-25 13:14 UTC (permalink / raw)
To: Jonathan Wakely; +Cc: Jason Merrill, GCC Patches
On Tue, Oct 25, 2022 at 12:34:50PM +0100, Jonathan Wakely wrote:
> On Mon, 24 Oct 2022 at 18:30, Jason Merrill wrote:
> >
> > On 10/21/22 19:28, Marek Polacek wrote:
> > > 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).
>
> I assume you mean using #pragma here. If we disable it around the
> definition of use_facet, will that disable it for callers of
> use_facet, or only within the definition of use_facet itself?
Right, a #pragma will not help, it would only disable the warning
within the definition of use_facet itself.
Another way would be to use some kind of attribute on use_facet but
I didn't find any that would be relevant in this scenario (so I guess
we'd have to add one).
Marek
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH v2] c++: Implement -Wdangling-reference [PR106393]
2022-10-24 17:30 ` Jason Merrill
2022-10-25 11:34 ` Jonathan Wakely
@ 2022-10-25 15:21 ` Marek Polacek
2022-10-25 15:53 ` Jason Merrill
1 sibling, 1 reply; 14+ messages in thread
From: Marek Polacek @ 2022-10-25 15:21 UTC (permalink / raw)
To: Jason Merrill; +Cc: GCC Patches, Jonathan Wakely
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
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] c++: Implement -Wdangling-reference [PR106393]
2022-10-25 11:50 ` [PATCH] " Jonathan Wakely
@ 2022-10-25 15:24 ` Marek Polacek
0 siblings, 0 replies; 14+ messages in thread
From: Marek Polacek @ 2022-10-25 15:24 UTC (permalink / raw)
To: Jonathan Wakely; +Cc: Jason Merrill, GCC Patches
On Tue, Oct 25, 2022 at 12:50:36PM +0100, Jonathan Wakely wrote:
> On Sat, 22 Oct 2022 at 00:28, Marek Polacek wrote:
> > 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
>
> These two are XFAIL tests, they're checking for a required static
> assert due to misusing std::forward, so I don't think we want to
> change them to remove a "bug" (they're meant to be buggy).
> I think they should use -Wno-dangling-reference
I think we don't have to do anything: -Wdangling-reference is only
enabled by -Wextra and that isn't used in these tests. I was just
curious about the warning's impact.
> > * experimental/any/misc/any_cast.cc
> > all of these look like genuine bugs. A bootstrap with the warning
> > enabled by default passed.
>
>
> I've attached a patch for the two any_cast.cc tests that avoids
> creating dangling references, without changing the intended behaviour
> of the tests.
Thanks a lot!
Marek
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] c++: Implement -Wdangling-reference [PR106393]
2022-10-25 13:14 ` Marek Polacek
@ 2022-10-25 15:39 ` Jason Merrill
2022-10-25 16:35 ` Marek Polacek
0 siblings, 1 reply; 14+ messages in thread
From: Jason Merrill @ 2022-10-25 15:39 UTC (permalink / raw)
To: Marek Polacek, Jonathan Wakely; +Cc: GCC Patches
On 10/25/22 09:14, Marek Polacek wrote:
> On Tue, Oct 25, 2022 at 12:34:50PM +0100, Jonathan Wakely wrote:
>> On Mon, 24 Oct 2022 at 18:30, Jason Merrill wrote:
>>>
>>> On 10/21/22 19:28, Marek Polacek wrote:
>>>> 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).
>>
>> I assume you mean using #pragma here. If we disable it around the
>> definition of use_facet, will that disable it for callers of
>> use_facet, or only within the definition of use_facet itself?
>
> Right, a #pragma will not help, it would only disable the warning
> within the definition of use_facet itself.
That's why I mentioned adjusting the compiler, i.e. check
warning_enabled_at (DECL_SOURCE_LOCATION (fn)
Jason
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v2] c++: Implement -Wdangling-reference [PR106393]
2022-10-25 15:21 ` [PATCH v2] " Marek Polacek
@ 2022-10-25 15:53 ` Jason Merrill
2022-10-26 16:10 ` [PATCH v3] " Marek Polacek
0 siblings, 1 reply; 14+ messages in thread
From: Jason Merrill @ 2022-10-25 15:53 UTC (permalink / raw)
To: Marek Polacek; +Cc: GCC Patches, Jonathan Wakely
On 10/25/22 11:21, Marek Polacek wrote:
> 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();
Yes. Or
struct S {
int ar[4];
int& operator[](int i) { return ar[i]; }
};
const int &r = S()[2];
> 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?
Like you limited the above check to TREE_STATIC, let's also forget about
checking for return *this.
>>> 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 (?).
Replied.
>> 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.
That looks like a true positive.
> I suppose this could be limited to << and >>? I'm not sure.
I'm not sure either. Another possibility would be to only consider the
LHS of binary operators, since returning a reference to the RHS is very
unusual.
>>> --- /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_;
Ah, right, the ?: is a prvalue.
>>> +// 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.
Indeed, but why limit it to checking for the same function rather than
also warning for
f(g(1))
or
A().f().g()
?
> 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
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] c++: Implement -Wdangling-reference [PR106393]
2022-10-25 15:39 ` Jason Merrill
@ 2022-10-25 16:35 ` Marek Polacek
0 siblings, 0 replies; 14+ messages in thread
From: Marek Polacek @ 2022-10-25 16:35 UTC (permalink / raw)
To: Jason Merrill; +Cc: Jonathan Wakely, GCC Patches
On Tue, Oct 25, 2022 at 11:39:31AM -0400, Jason Merrill wrote:
> On 10/25/22 09:14, Marek Polacek wrote:
> > On Tue, Oct 25, 2022 at 12:34:50PM +0100, Jonathan Wakely wrote:
> > > On Mon, 24 Oct 2022 at 18:30, Jason Merrill wrote:
> > > >
> > > > On 10/21/22 19:28, Marek Polacek wrote:
> > > > > 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).
> > >
> > > I assume you mean using #pragma here. If we disable it around the
> > > definition of use_facet, will that disable it for callers of
> > > use_facet, or only within the definition of use_facet itself?
> >
> > Right, a #pragma will not help, it would only disable the warning
> > within the definition of use_facet itself.
>
> That's why I mentioned adjusting the compiler, i.e. check warning_enabled_at
> (DECL_SOURCE_LOCATION (fn)
Ah, like with -Wdeprecated-copy. I think I fixed this:
--- a/libstdc++-v3/include/bits/locale_classes.tcc
+++ b/libstdc++-v3/include/bits/locale_classes.tcc
@@ -127,6 +127,8 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
* @return Reference to facet of type Facet.
* @throw std::bad_cast if @p __loc doesn't contain a facet of type _Facet.
*/
+#pragma GCC diagnostic push
+#pragma GCC diagnostic ignored "-Wdangling-reference"
template<typename _Facet>
const _Facet&
use_facet(const locale& __loc)
@@ -141,6 +143,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
return static_cast<const _Facet&>(*__facets[__i]);
#endif
}
+#pragma GCC diagnostic pop
// Generic version does nothing.
and then just check warning_enabled_at. Nice!
Marek
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH v3] c++: Implement -Wdangling-reference [PR106393]
2022-10-25 15:53 ` Jason Merrill
@ 2022-10-26 16:10 ` Marek Polacek
2022-10-26 16:42 ` Jason Merrill
0 siblings, 1 reply; 14+ messages in thread
From: Marek Polacek @ 2022-10-26 16:10 UTC (permalink / raw)
To: Jason Merrill; +Cc: Jonathan Wakely, GCC Patches
On Tue, Oct 25, 2022 at 11:53:51AM -0400, Jason Merrill via Gcc-patches wrote:
> On 10/25/22 11:21, Marek Polacek wrote:
> > On Mon, Oct 24, 2022 at 01:30:42PM -0400, Jason Merrill wrote:
> > > On 10/21/22 19:28, Marek Polacek wrote:
> > > > 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();
>
> Yes. Or
>
> struct S {
> int ar[4];
> int& operator[](int i) { return ar[i]; }
> };
> const int &r = S()[2];
Ok, I've extended the warning to check if the object argument is a temporary
as well, so we get a warning here now.
> > 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?
>
> Like you limited the above check to TREE_STATIC, let's also forget about
> checking for return *this.
I don't follow, but it looks like I don't need to change anything and just
keep the TREE_STATIC check?
> > > > 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 (?).
>
> Replied.
Fixed.
> > > > + 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.
>
> That looks like a true positive.
Ok, then...
> > I suppose this could be limited to << and >>? I'm not sure.
>
> I'm not sure either. Another possibility would be to only consider the LHS
> of binary operators, since returning a reference to the RHS is very unusual.
...I've removed the DECL_OVERLOADED_OPERATOR_P check altogether and don't
really see any false positives. It's probably better to start with a more
general warning and then tweak it based on empirical evidence.
> > > > +// 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.
>
> Indeed, but why limit it to checking for the same function rather than also
> warning for
>
> f(g(1))
>
> or
>
> A().f().g()
>
> ?
OK, I've made the warning even more recursive so that now we detect
both of these, see Wdangling-reference3.C. While at it, I've renamed
the functions.
As before, I've tested this patch twice, once with -Wdangling-reference
enabled by default, once with -Wdangling-reference enabled by -Wextra.
The couple of FAILs I saw were true positives (e.g., rv-conv1.C, rv-func2.C).
Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk?
-- >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 warns when the function in question is a member function, but only if
the function is invoked on a temporary object, otherwise the warning
would emit loads of warnings for valid code like obj.emplace<T>({0}, 0).
It does detect the dangling reference in:
struct S {
const S& self () { return *this; }
};
const S& s = S().self();
It warns in member initializer lists as well:
const int& f(const int& i) { return i; }
struct S {
const int &r;
S() : r(f(10)) { }
};
I've run the testsuite/bootstrap with the warning enabled by default.
There were just a few FAILs, all of which look like genuine bugs.
A bootstrap with the warning enabled by default passed as well.
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 a #pragma and code to suppress the
warning.
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): ...here. Don't return false just
because a ck_base is missing. Use expr_represents_temporary_p.
(do_warn_dangling_reference): New.
(maybe_warn_dangling_reference): New.
(extend_ref_init_temps): Call maybe_warn_dangling_reference.
* typeck.cc (check_return_expr): Suppress -Wdangling-reference
warnings.
gcc/ChangeLog:
* doc/invoke.texi: Document -Wdangling-reference.
libstdc++-v3/ChangeLog:
* include/bits/locale_classes.tcc: Add #pragma to disable
-Wdangling-reference with std::use_facet.
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.
* g++.dg/warn/Wdangling-reference3.C: New test.
---
gcc/c-family/c.opt | 4 +
gcc/cp/call.cc | 148 ++++++++++++++++--
gcc/cp/cp-tree.h | 4 +-
gcc/cp/typeck.cc | 10 ++
gcc/doc/invoke.texi | 40 ++++-
gcc/testsuite/g++.dg/cpp23/elision4.C | 5 +-
gcc/testsuite/g++.dg/cpp23/elision7.C | 3 +-
.../g++.dg/warn/Wdangling-reference1.C | 144 +++++++++++++++++
.../g++.dg/warn/Wdangling-reference2.C | 28 ++++
.../g++.dg/warn/Wdangling-reference3.C | 24 +++
libstdc++-v3/include/bits/locale_classes.tcc | 3 +
11 files changed, 396 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
create mode 100644 gcc/testsuite/g++.dg/warn/Wdangling-reference3.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..dae7a33fb89 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,121 @@ initialize_reference (tree type, tree expr,
return expr;
}
+/* 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
+ if none found. For instance:
+
+ const S& s = S().self(); // S::self (&TARGET_EXPR <...>)
+ 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
+do_warn_dangling_reference (tree expr)
+{
+ STRIP_NOPS (expr);
+ switch (TREE_CODE (expr))
+ {
+ case CALL_EXPR:
+ {
+ tree fndecl = cp_get_callee_fndecl_nofold (expr);
+ if (!fndecl
+ || 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_P (TREE_TYPE (TREE_TYPE (fndecl))))
+ 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)
+ {
+ tree arg = CALL_EXPR_ARG (expr, i);
+ /* Check that this argument initializes a reference, except for
+ the argument initializing the object of a member function. */
+ if (!DECL_NONSTATIC_MEMBER_FUNCTION_P (fndecl)
+ && !TYPE_REF_P (TREE_TYPE (arg)))
+ continue;
+ /* It could also be another call taking a temporary and returning
+ it and initializing this reference parameter. */
+ if (do_warn_dangling_reference (arg))
+ return expr;
+ STRIP_NOPS (arg);
+ if (TREE_CODE (arg) == ADDR_EXPR)
+ arg = TREE_OPERAND (arg, 0);
+ if (expr_represents_temporary_p (arg))
+ return expr;
+ /* Don't warn about member function like:
+ std::any a(...);
+ S& s = a.emplace<S>({0}, 0);
+ which constructs a new object and returns a reference to it, but
+ we still want to detect:
+ struct S { const S& self () { return *this; } };
+ const S& s = S().self();
+ where 's' dangles. If we've gotten here, the object this function
+ is invoked on is not a temporary. */
+ if (DECL_NONSTATIC_MEMBER_FUNCTION_P (fndecl))
+ break;
+ }
+ return NULL_TREE;
+ }
+ case COMPOUND_EXPR:
+ return do_warn_dangling_reference (TREE_OPERAND (expr, 1));
+ case COND_EXPR:
+ if (tree t = do_warn_dangling_reference (TREE_OPERAND (expr, 1)))
+ return t;
+ return do_warn_dangling_reference (TREE_OPERAND (expr, 2));
+ case PAREN_EXPR:
+ return do_warn_dangling_reference (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.
+
+ DECL is the reference being initialized, INIT is the initializer. */
+
+static void
+maybe_warn_dangling_reference (const_tree decl, tree init)
+{
+ if (!warn_dangling_reference)
+ return;
+ if (!TYPE_REF_P (TREE_TYPE (decl)))
+ return;
+ if (tree call = do_warn_dangling_reference (init))
+ {
+ 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 +13646,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;
+
+ maybe_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 867096b08c6..40f5bf802c3 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..0a3a174cdc8 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,42 @@ 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 checked as well, but only their object argument:
+
+@smallexample
+struct S @{
+ const S& self () @{ return *this; @}
+@};
+const S& s = S().self(); // s is dangling
+@end smallexample
+
+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 +5973,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..97c81ee716c
--- /dev/null
+++ b/gcc/testsuite/g++.dg/warn/Wdangling-reference1.C
@@ -0,0 +1,144 @@
+// PR c++/106393
+// { dg-do compile { target c++11 } }
+// { dg-options "-Wdangling-reference" }
+
+const int& f(const int& i) { return i; }
+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. No dangling reference.
+const int& r11 = g ? f(10) : 9;
+const int& r12 = g ? 9 : f(10);
+// r12 = f (f ((const int &) &TARGET_EXPR <D.2459, 1>))
+const int& r13 = f(f(1)); // { dg-warning "dangling reference" }
+const int& r14 = f(f_(1)); // { dg-warning "dangling reference" }
+const int& r15 = f(g ? f(1) : f(2)); // { dg-warning "dangling reference" }
+const int& r16 = f(*&f(1)); // { dg-warning "dangling reference" }
+const int& r17 = rp(&f(1));
+const int& r18 = rp(&f(g));
+const int& r19 = h(f(1));
+// Other forms of initializers.
+const int& r20(f(10)); // { dg-warning "dangling reference" }
+const int& r21(f(10)); // { dg-warning "dangling reference" }
+// Returns a ref, but doesn't have a parameter of reference type.
+const int& r22 = h(10);
+const int& r23 = g ? h(10) : f(10); // { dg-warning "dangling reference" }
+const int& r24 = g ? f(10) : h(10); // { dg-warning "dangling reference" }
+const int& r25 = g ? h(10) : (1, f(10)); // { dg-warning "dangling reference" }
+const int& r26 = g ? (1, f(10)) : h(10); // { dg-warning "dangling reference" }
+const int& r29 = f((f_(1), 1)); // { dg-warning "dangling reference" }
+const int& r30 = f((f_(1), g));
+
+struct Z {
+ operator int() { return 42; }
+};
+
+const int& r27 = f(Z()); // { dg-warning "dangling reference" }
+const int& r28 = f(true ? Z() : Z()); // { dg-warning "dangling reference" }
+
+const int& operator|(const int &, Z);
+const int& r31 = 1 | Z(); // { 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); // { dg-warning "dangling reference" }
+
+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" }
+
+struct Y {
+ operator int&();
+ operator int&&();
+ const int& foo(const int&);
+};
+
+// 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" }
+const int& t1 = Y().foo(10); // { dg-warning "dangling reference" }
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));
+}
diff --git a/gcc/testsuite/g++.dg/warn/Wdangling-reference3.C b/gcc/testsuite/g++.dg/warn/Wdangling-reference3.C
new file mode 100644
index 00000000000..4bc20c13b3f
--- /dev/null
+++ b/gcc/testsuite/g++.dg/warn/Wdangling-reference3.C
@@ -0,0 +1,24 @@
+// PR c++/106393
+// { dg-do compile { target c++11 } }
+// { dg-options "-Wdangling-reference" }
+
+struct A {
+ int ar[4];
+ int& operator[](int i) { return ar[i]; }
+};
+const int &r = A()[2]; // { dg-warning "dangling reference" }
+
+struct S {
+ const S& self () { return *this; }
+};
+const S& s = S().self(); // { dg-warning "dangling reference" }
+
+struct G {
+ const G& g() { return *this; }
+};
+
+struct F {
+ G& f();
+};
+
+const G& g = F().f().g(); // { dg-warning "dangling reference" }
diff --git a/libstdc++-v3/include/bits/locale_classes.tcc b/libstdc++-v3/include/bits/locale_classes.tcc
index 64cd7534dc6..9cc4f238ee7 100644
--- a/libstdc++-v3/include/bits/locale_classes.tcc
+++ b/libstdc++-v3/include/bits/locale_classes.tcc
@@ -127,6 +127,8 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
* @return Reference to facet of type Facet.
* @throw std::bad_cast if @p __loc doesn't contain a facet of type _Facet.
*/
+#pragma GCC diagnostic push
+#pragma GCC diagnostic ignored "-Wdangling-reference"
template<typename _Facet>
const _Facet&
use_facet(const locale& __loc)
@@ -141,6 +143,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
return static_cast<const _Facet&>(*__facets[__i]);
#endif
}
+#pragma GCC diagnostic pop
// Generic version does nothing.
base-commit: a87819b8f1b890d36a3f05bd9de80be20e9525dd
--
2.37.3
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v3] c++: Implement -Wdangling-reference [PR106393]
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
0 siblings, 1 reply; 14+ messages in thread
From: Jason Merrill @ 2022-10-26 16:42 UTC (permalink / raw)
To: Marek Polacek; +Cc: Jonathan Wakely, GCC Patches
On 10/26/22 12:10, Marek Polacek wrote:
> On Tue, Oct 25, 2022 at 11:53:51AM -0400, Jason Merrill via Gcc-patches wrote:
>> On 10/25/22 11:21, Marek Polacek wrote:
>>> On Mon, Oct 24, 2022 at 01:30:42PM -0400, Jason Merrill wrote:
>>>> On 10/21/22 19:28, Marek Polacek wrote:
>>>>> 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();
>>
>> Yes. Or
>>
>> struct S {
>> int ar[4];
>> int& operator[](int i) { return ar[i]; }
>> };
>> const int &r = S()[2];
>
> Ok, I've extended the warning to check if the object argument is a temporary
> as well, so we get a warning here now.
>
>>> 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?
>>
>> Like you limited the above check to TREE_STATIC, let's also forget about
>> checking for return *this.
>
> I don't follow, but it looks like I don't need to change anything and just
> keep the TREE_STATIC check?
>
>>>>> 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 (?).
>>
>> Replied.
>
> Fixed.
>
>>>>> + 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.
>>
>> That looks like a true positive.
>
> Ok, then...
>
>>> I suppose this could be limited to << and >>? I'm not sure.
>>
>> I'm not sure either. Another possibility would be to only consider the LHS
>> of binary operators, since returning a reference to the RHS is very unusual.
>
> ...I've removed the DECL_OVERLOADED_OPERATOR_P check altogether and don't
> really see any false positives. It's probably better to start with a more
> general warning and then tweak it based on empirical evidence.
>
>>>>> +// 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.
>>
>> Indeed, but why limit it to checking for the same function rather than also
>> warning for
>>
>> f(g(1))
>>
>> or
>>
>> A().f().g()
>>
>> ?
>
> OK, I've made the warning even more recursive so that now we detect
> both of these, see Wdangling-reference3.C. While at it, I've renamed
> the functions.
>
> As before, I've tested this patch twice, once with -Wdangling-reference
> enabled by default, once with -Wdangling-reference enabled by -Wextra.
> The couple of FAILs I saw were true positives (e.g., rv-conv1.C, rv-func2.C).
I might actually add it to -Wall, the false positive rate sounds pretty
low at this point.
> Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk?
>
> -- >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 warns when the function in question is a member function, but only if
> the function is invoked on a temporary object, otherwise the warning
> would emit loads of warnings for valid code like obj.emplace<T>({0}, 0).
> It does detect the dangling reference in:
>
> struct S {
> const S& self () { return *this; }
> };
> const S& s = S().self();
>
> It warns in member initializer lists as well:
>
> const int& f(const int& i) { return i; }
> struct S {
> const int &r;
> S() : r(f(10)) { }
> };
>
> I've run the testsuite/bootstrap with the warning enabled by default.
> There were just a few FAILs, all of which look like genuine bugs.
> A bootstrap with the warning enabled by default passed as well.
>
> 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 a #pragma and code to suppress the
> warning.
>
> 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): ...here. Don't return false just
> because a ck_base is missing. Use expr_represents_temporary_p.
> (do_warn_dangling_reference): New.
> (maybe_warn_dangling_reference): New.
> (extend_ref_init_temps): Call maybe_warn_dangling_reference.
> * typeck.cc (check_return_expr): Suppress -Wdangling-reference
> warnings.
>
> gcc/ChangeLog:
>
> * doc/invoke.texi: Document -Wdangling-reference.
>
> libstdc++-v3/ChangeLog:
>
> * include/bits/locale_classes.tcc: Add #pragma to disable
> -Wdangling-reference with std::use_facet.
>
> 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.
> * g++.dg/warn/Wdangling-reference3.C: New test.
> ---
> gcc/c-family/c.opt | 4 +
> gcc/cp/call.cc | 148 ++++++++++++++++--
> gcc/cp/cp-tree.h | 4 +-
> gcc/cp/typeck.cc | 10 ++
> gcc/doc/invoke.texi | 40 ++++-
> gcc/testsuite/g++.dg/cpp23/elision4.C | 5 +-
> gcc/testsuite/g++.dg/cpp23/elision7.C | 3 +-
> .../g++.dg/warn/Wdangling-reference1.C | 144 +++++++++++++++++
> .../g++.dg/warn/Wdangling-reference2.C | 28 ++++
> .../g++.dg/warn/Wdangling-reference3.C | 24 +++
> libstdc++-v3/include/bits/locale_classes.tcc | 3 +
> 11 files changed, 396 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
> create mode 100644 gcc/testsuite/g++.dg/warn/Wdangling-reference3.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..dae7a33fb89 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,121 @@ initialize_reference (tree type, tree expr,
> return expr;
> }
>
> +/* 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
> + if none found. For instance:
> +
> + const S& s = S().self(); // S::self (&TARGET_EXPR <...>)
> + 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
> +do_warn_dangling_reference (tree expr)
> +{
> + STRIP_NOPS (expr);
> + switch (TREE_CODE (expr))
> + {
> + case CALL_EXPR:
> + {
> + tree fndecl = cp_get_callee_fndecl_nofold (expr);
> + if (!fndecl
> + || 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_P (TREE_TYPE (TREE_TYPE (fndecl))))
Maybe TYPE_REF_OBJ_P? Either way is fine.
> + 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)
> + {
> + tree arg = CALL_EXPR_ARG (expr, i);
> + /* Check that this argument initializes a reference, except for
> + the argument initializing the object of a member function. */
> + if (!DECL_NONSTATIC_MEMBER_FUNCTION_P (fndecl)
> + && !TYPE_REF_P (TREE_TYPE (arg)))
> + continue;
> + /* It could also be another call taking a temporary and returning
> + it and initializing this reference parameter. */
> + if (do_warn_dangling_reference (arg))
> + return expr;
> + STRIP_NOPS (arg);
> + if (TREE_CODE (arg) == ADDR_EXPR)
> + arg = TREE_OPERAND (arg, 0);
> + if (expr_represents_temporary_p (arg))
> + return expr;
> + /* Don't warn about member function like:
> + std::any a(...);
> + S& s = a.emplace<S>({0}, 0);
> + which constructs a new object and returns a reference to it, but
> + we still want to detect:
> + struct S { const S& self () { return *this; } };
> + const S& s = S().self();
> + where 's' dangles. If we've gotten here, the object this function
> + is invoked on is not a temporary. */
> + if (DECL_NONSTATIC_MEMBER_FUNCTION_P (fndecl))
> + break;
> + }
> + return NULL_TREE;
> + }
> + case COMPOUND_EXPR:
> + return do_warn_dangling_reference (TREE_OPERAND (expr, 1));
> + case COND_EXPR:
> + if (tree t = do_warn_dangling_reference (TREE_OPERAND (expr, 1)))
> + return t;
> + return do_warn_dangling_reference (TREE_OPERAND (expr, 2));
> + case PAREN_EXPR:
> + return do_warn_dangling_reference (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.
> +
> + DECL is the reference being initialized, INIT is the initializer. */
> +
> +static void
> +maybe_warn_dangling_reference (const_tree decl, tree init)
> +{
> + if (!warn_dangling_reference)
> + return;
> + if (!TYPE_REF_P (TREE_TYPE (decl)))
> + return;
> + if (tree call = do_warn_dangling_reference (init))
> + {
> + 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 +13646,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;
> +
> + maybe_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 867096b08c6..40f5bf802c3 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
You probably need to check VAR_P (bare_retval) here, static_flag is used
for various other things in other tree codes.
> + && 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..0a3a174cdc8 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,42 @@ 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 checked as well, but only their object argument:
> +
> +@smallexample
> +struct S @{
> + const S& self () @{ return *this; @}
> +@};
> +const S& s = S().self(); // s is dangling
> +@end smallexample
> +
> +This warning is enabled by @option{-Wextra}.
It would be good to mention using #pragma to disable the warning around
safe functions.
> @item -Wdelete-non-virtual-dtor @r{(C++ and Objective-C++ only)}
> @opindex Wdelete-non-virtual-dtor
> @opindex Wno-delete-non-virtual-dtor
> @@ -5936,6 +5973,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..97c81ee716c
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/warn/Wdangling-reference1.C
> @@ -0,0 +1,144 @@
> +// PR c++/106393
> +// { dg-do compile { target c++11 } }
> +// { dg-options "-Wdangling-reference" }
> +
> +const int& f(const int& i) { return i; }
> +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. No dangling reference.
> +const int& r11 = g ? f(10) : 9;
> +const int& r12 = g ? 9 : f(10);
> +// r12 = f (f ((const int &) &TARGET_EXPR <D.2459, 1>))
> +const int& r13 = f(f(1)); // { dg-warning "dangling reference" }
> +const int& r14 = f(f_(1)); // { dg-warning "dangling reference" }
> +const int& r15 = f(g ? f(1) : f(2)); // { dg-warning "dangling reference" }
> +const int& r16 = f(*&f(1)); // { dg-warning "dangling reference" }
> +const int& r17 = rp(&f(1));
> +const int& r18 = rp(&f(g));
> +const int& r19 = h(f(1));
> +// Other forms of initializers.
> +const int& r20(f(10)); // { dg-warning "dangling reference" }
> +const int& r21(f(10)); // { dg-warning "dangling reference" }
> +// Returns a ref, but doesn't have a parameter of reference type.
> +const int& r22 = h(10);
> +const int& r23 = g ? h(10) : f(10); // { dg-warning "dangling reference" }
> +const int& r24 = g ? f(10) : h(10); // { dg-warning "dangling reference" }
> +const int& r25 = g ? h(10) : (1, f(10)); // { dg-warning "dangling reference" }
> +const int& r26 = g ? (1, f(10)) : h(10); // { dg-warning "dangling reference" }
> +const int& r29 = f((f_(1), 1)); // { dg-warning "dangling reference" }
> +const int& r30 = f((f_(1), g));
> +
> +struct Z {
> + operator int() { return 42; }
> +};
> +
> +const int& r27 = f(Z()); // { dg-warning "dangling reference" }
> +const int& r28 = f(true ? Z() : Z()); // { dg-warning "dangling reference" }
> +
> +const int& operator|(const int &, Z);
> +const int& r31 = 1 | Z(); // { 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); // { dg-warning "dangling reference" }
> +
> +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" }
> +
> +struct Y {
> + operator int&();
> + operator int&&();
> + const int& foo(const int&);
> +};
> +
> +// 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" }
> +const int& t1 = Y().foo(10); // { dg-warning "dangling reference" }
> 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));
> +}
> diff --git a/gcc/testsuite/g++.dg/warn/Wdangling-reference3.C b/gcc/testsuite/g++.dg/warn/Wdangling-reference3.C
> new file mode 100644
> index 00000000000..4bc20c13b3f
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/warn/Wdangling-reference3.C
> @@ -0,0 +1,24 @@
> +// PR c++/106393
> +// { dg-do compile { target c++11 } }
> +// { dg-options "-Wdangling-reference" }
> +
> +struct A {
> + int ar[4];
> + int& operator[](int i) { return ar[i]; }
> +};
> +const int &r = A()[2]; // { dg-warning "dangling reference" }
> +
> +struct S {
> + const S& self () { return *this; }
> +};
> +const S& s = S().self(); // { dg-warning "dangling reference" }
> +
> +struct G {
> + const G& g() { return *this; }
> +};
> +
> +struct F {
> + G& f();
> +};
> +
> +const G& g = F().f().g(); // { dg-warning "dangling reference" }
> diff --git a/libstdc++-v3/include/bits/locale_classes.tcc b/libstdc++-v3/include/bits/locale_classes.tcc
> index 64cd7534dc6..9cc4f238ee7 100644
> --- a/libstdc++-v3/include/bits/locale_classes.tcc
> +++ b/libstdc++-v3/include/bits/locale_classes.tcc
> @@ -127,6 +127,8 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
> * @return Reference to facet of type Facet.
> * @throw std::bad_cast if @p __loc doesn't contain a facet of type _Facet.
> */
> +#pragma GCC diagnostic push
> +#pragma GCC diagnostic ignored "-Wdangling-reference"
> template<typename _Facet>
> const _Facet&
> use_facet(const locale& __loc)
> @@ -141,6 +143,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
> return static_cast<const _Facet&>(*__facets[__i]);
> #endif
> }
> +#pragma GCC diagnostic pop
>
>
> // Generic version does nothing.
>
> base-commit: a87819b8f1b890d36a3f05bd9de80be20e9525dd
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH v4] c++: Implement -Wdangling-reference [PR106393]
2022-10-26 16:42 ` Jason Merrill
@ 2022-10-26 18:26 ` Marek Polacek
2022-10-26 18:42 ` Jason Merrill
0 siblings, 1 reply; 14+ messages in thread
From: Marek Polacek @ 2022-10-26 18:26 UTC (permalink / raw)
To: Jason Merrill; +Cc: Jonathan Wakely, GCC Patches
On Wed, Oct 26, 2022 at 12:42:27PM -0400, Jason Merrill wrote:
> On 10/26/22 12:10, Marek Polacek wrote:
> > As before, I've tested this patch twice, once with -Wdangling-reference
> > enabled by default, once with -Wdangling-reference enabled by -Wextra.
> > The couple of FAILs I saw were true positives (e.g., rv-conv1.C, rv-func2.C).
>
> I might actually add it to -Wall, the false positive rate sounds pretty low
> at this point.
Done (and invoke.texi adjusted).
> > +static tree
> > +do_warn_dangling_reference (tree expr)
> > +{
> > + STRIP_NOPS (expr);
> > + switch (TREE_CODE (expr))
> > + {
> > + case CALL_EXPR:
> > + {
> > + tree fndecl = cp_get_callee_fndecl_nofold (expr);
> > + if (!fndecl
> > + || 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_P (TREE_TYPE (TREE_TYPE (fndecl))))
>
> Maybe TYPE_REF_OBJ_P? Either way is fine.
Adjusted.
> > --- 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
>
> You probably need to check VAR_P (bare_retval) here, static_flag is used for
> various other things in other tree codes.
Added VAR_P.
> > + && 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..0a3a174cdc8 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,42 @@ 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 checked as well, but only their object argument:
> > +
> > +@smallexample
> > +struct S @{
> > + const S& self () @{ return *this; @}
> > +@};
> > +const S& s = S().self(); // s is dangling
> > +@end smallexample
> > +
> > +This warning is enabled by @option{-Wextra}.
>
> It would be good to mention using #pragma to disable the warning around safe
> functions.
Yes, text added.
Now that the warning is in -Wall, I've added -Wno-dangling-reference to
g++.dg/warn/Wdangling-pointer-2.C.
Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk?
-- >8 --
This patch implements a new experimental warning (enabled by -Wall) 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 warns when the function in question is a member function, but only if
the function is invoked on a temporary object, otherwise the warning
would emit loads of warnings for valid code like obj.emplace<T>({0}, 0).
It does detect the dangling reference in:
struct S {
const S& self () { return *this; }
};
const S& s = S().self();
It warns in member initializer lists as well:
const int& f(const int& i) { return i; }
struct S {
const int &r;
S() : r(f(10)) { }
};
I've run the testsuite/bootstrap with the warning enabled by default.
There were just a few FAILs, all of which look like genuine bugs.
A bootstrap with the warning enabled by default passed as well.
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 a #pragma and code to suppress the
warning.
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): ...here. Don't return false just
because a ck_base is missing. Use expr_represents_temporary_p.
(do_warn_dangling_reference): New.
(maybe_warn_dangling_reference): New.
(extend_ref_init_temps): Call maybe_warn_dangling_reference.
* typeck.cc (check_return_expr): Suppress -Wdangling-reference
warnings.
gcc/ChangeLog:
* doc/invoke.texi: Document -Wdangling-reference.
libstdc++-v3/ChangeLog:
* include/bits/locale_classes.tcc: Add #pragma to disable
-Wdangling-reference with std::use_facet.
gcc/testsuite/ChangeLog:
* g++.dg/cpp23/elision4.C: Use -Wdangling-reference, add dg-warning.
* g++.dg/cpp23/elision7.C: Likewise.
* g++.dg/warn/Wdangling-pointer-2.C: Use -Wno-dangling-reference.
* g++.dg/warn/Wdangling-reference1.C: New test.
* g++.dg/warn/Wdangling-reference2.C: New test.
* g++.dg/warn/Wdangling-reference3.C: New test.
---
gcc/c-family/c.opt | 4 +
gcc/cp/call.cc | 148 ++++++++++++++++--
gcc/cp/cp-tree.h | 4 +-
gcc/cp/typeck.cc | 11 ++
gcc/doc/invoke.texi | 51 +++++-
gcc/testsuite/g++.dg/cpp23/elision4.C | 5 +-
gcc/testsuite/g++.dg/cpp23/elision7.C | 3 +-
.../g++.dg/warn/Wdangling-pointer-2.C | 2 +-
.../g++.dg/warn/Wdangling-reference1.C | 144 +++++++++++++++++
.../g++.dg/warn/Wdangling-reference2.C | 28 ++++
.../g++.dg/warn/Wdangling-reference3.C | 24 +++
libstdc++-v3/include/bits/locale_classes.tcc | 3 +
12 files changed, 409 insertions(+), 18 deletions(-)
create mode 100644 gcc/testsuite/g++.dg/warn/Wdangling-reference1.C
create mode 100644 gcc/testsuite/g++.dg/warn/Wdangling-reference2.C
create mode 100644 gcc/testsuite/g++.dg/warn/Wdangling-reference3.C
diff --git a/gcc/c-family/c.opt b/gcc/c-family/c.opt
index 01d480759ae..070f85c81d2 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++, Wall)
+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..951b9fd2a88 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,121 @@ initialize_reference (tree type, tree expr,
return expr;
}
+/* 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
+ if none found. For instance:
+
+ const S& s = S().self(); // S::self (&TARGET_EXPR <...>)
+ 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
+do_warn_dangling_reference (tree expr)
+{
+ STRIP_NOPS (expr);
+ switch (TREE_CODE (expr))
+ {
+ case CALL_EXPR:
+ {
+ tree fndecl = cp_get_callee_fndecl_nofold (expr);
+ if (!fndecl
+ || 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))))
+ 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)
+ {
+ tree arg = CALL_EXPR_ARG (expr, i);
+ /* Check that this argument initializes a reference, except for
+ the argument initializing the object of a member function. */
+ if (!DECL_NONSTATIC_MEMBER_FUNCTION_P (fndecl)
+ && !TYPE_REF_P (TREE_TYPE (arg)))
+ continue;
+ /* It could also be another call taking a temporary and returning
+ it and initializing this reference parameter. */
+ if (do_warn_dangling_reference (arg))
+ return expr;
+ STRIP_NOPS (arg);
+ if (TREE_CODE (arg) == ADDR_EXPR)
+ arg = TREE_OPERAND (arg, 0);
+ if (expr_represents_temporary_p (arg))
+ return expr;
+ /* Don't warn about member function like:
+ std::any a(...);
+ S& s = a.emplace<S>({0}, 0);
+ which constructs a new object and returns a reference to it, but
+ we still want to detect:
+ struct S { const S& self () { return *this; } };
+ const S& s = S().self();
+ where 's' dangles. If we've gotten here, the object this function
+ is invoked on is not a temporary. */
+ if (DECL_NONSTATIC_MEMBER_FUNCTION_P (fndecl))
+ break;
+ }
+ return NULL_TREE;
+ }
+ case COMPOUND_EXPR:
+ return do_warn_dangling_reference (TREE_OPERAND (expr, 1));
+ case COND_EXPR:
+ if (tree t = do_warn_dangling_reference (TREE_OPERAND (expr, 1)))
+ return t;
+ return do_warn_dangling_reference (TREE_OPERAND (expr, 2));
+ case PAREN_EXPR:
+ return do_warn_dangling_reference (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.
+
+ DECL is the reference being initialized, INIT is the initializer. */
+
+static void
+maybe_warn_dangling_reference (const_tree decl, tree init)
+{
+ if (!warn_dangling_reference)
+ return;
+ if (!TYPE_REF_P (TREE_TYPE (decl)))
+ return;
+ if (tree call = do_warn_dangling_reference (init))
+ {
+ 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 +13646,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;
+
+ maybe_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 867096b08c6..40f5bf802c3 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..4605f734e2d 100644
--- a/gcc/cp/typeck.cc
+++ b/gcc/cp/typeck.cc
@@ -11246,6 +11246,17 @@ 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
+ && VAR_P (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..9f0e5460861 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,54 @@ 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 checked as well, but only their object argument:
+
+@smallexample
+struct S @{
+ const S& self () @{ return *this; @}
+@};
+const S& s = S().self(); // s is dangling
+@end smallexample
+
+Certain functions are safe in this respect, for example @code{std::use_facet}:
+they take and return a reference, but they don't return one of its arguments,
+which can fool the warning. Such functions can be excluded from the warning
+by wrapping them in a @code{#pragma}:
+
+@smallexample
+#pragma GCC diagnostic push
+#pragma GCC diagnostic ignored "-Wdangling-reference"
+const T& foo (const T&) @{ @dots{} @}
+#pragma GCC diagnostic pop
+@end smallexample
+
+This warning is enabled by @option{-Wall}.
+
@item -Wdelete-non-virtual-dtor @r{(C++ and Objective-C++ only)}
@opindex Wdelete-non-virtual-dtor
@opindex Wno-delete-non-virtual-dtor
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-pointer-2.C b/gcc/testsuite/g++.dg/warn/Wdangling-pointer-2.C
index 151418f3c32..802ac9cd954 100644
--- a/gcc/testsuite/g++.dg/warn/Wdangling-pointer-2.C
+++ b/gcc/testsuite/g++.dg/warn/Wdangling-pointer-2.C
@@ -1,5 +1,5 @@
/* { dg-do compile }
- { dg-options "-O1 -Wall -Wno-class-memaccess" } */
+ { dg-options "-O1 -Wall -Wno-class-memaccess -Wno-dangling-reference" } */
struct A { A (); };
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..97c81ee716c
--- /dev/null
+++ b/gcc/testsuite/g++.dg/warn/Wdangling-reference1.C
@@ -0,0 +1,144 @@
+// PR c++/106393
+// { dg-do compile { target c++11 } }
+// { dg-options "-Wdangling-reference" }
+
+const int& f(const int& i) { return i; }
+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. No dangling reference.
+const int& r11 = g ? f(10) : 9;
+const int& r12 = g ? 9 : f(10);
+// r12 = f (f ((const int &) &TARGET_EXPR <D.2459, 1>))
+const int& r13 = f(f(1)); // { dg-warning "dangling reference" }
+const int& r14 = f(f_(1)); // { dg-warning "dangling reference" }
+const int& r15 = f(g ? f(1) : f(2)); // { dg-warning "dangling reference" }
+const int& r16 = f(*&f(1)); // { dg-warning "dangling reference" }
+const int& r17 = rp(&f(1));
+const int& r18 = rp(&f(g));
+const int& r19 = h(f(1));
+// Other forms of initializers.
+const int& r20(f(10)); // { dg-warning "dangling reference" }
+const int& r21(f(10)); // { dg-warning "dangling reference" }
+// Returns a ref, but doesn't have a parameter of reference type.
+const int& r22 = h(10);
+const int& r23 = g ? h(10) : f(10); // { dg-warning "dangling reference" }
+const int& r24 = g ? f(10) : h(10); // { dg-warning "dangling reference" }
+const int& r25 = g ? h(10) : (1, f(10)); // { dg-warning "dangling reference" }
+const int& r26 = g ? (1, f(10)) : h(10); // { dg-warning "dangling reference" }
+const int& r29 = f((f_(1), 1)); // { dg-warning "dangling reference" }
+const int& r30 = f((f_(1), g));
+
+struct Z {
+ operator int() { return 42; }
+};
+
+const int& r27 = f(Z()); // { dg-warning "dangling reference" }
+const int& r28 = f(true ? Z() : Z()); // { dg-warning "dangling reference" }
+
+const int& operator|(const int &, Z);
+const int& r31 = 1 | Z(); // { 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); // { dg-warning "dangling reference" }
+
+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" }
+
+struct Y {
+ operator int&();
+ operator int&&();
+ const int& foo(const int&);
+};
+
+// 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" }
+const int& t1 = Y().foo(10); // { dg-warning "dangling reference" }
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));
+}
diff --git a/gcc/testsuite/g++.dg/warn/Wdangling-reference3.C b/gcc/testsuite/g++.dg/warn/Wdangling-reference3.C
new file mode 100644
index 00000000000..4bc20c13b3f
--- /dev/null
+++ b/gcc/testsuite/g++.dg/warn/Wdangling-reference3.C
@@ -0,0 +1,24 @@
+// PR c++/106393
+// { dg-do compile { target c++11 } }
+// { dg-options "-Wdangling-reference" }
+
+struct A {
+ int ar[4];
+ int& operator[](int i) { return ar[i]; }
+};
+const int &r = A()[2]; // { dg-warning "dangling reference" }
+
+struct S {
+ const S& self () { return *this; }
+};
+const S& s = S().self(); // { dg-warning "dangling reference" }
+
+struct G {
+ const G& g() { return *this; }
+};
+
+struct F {
+ G& f();
+};
+
+const G& g = F().f().g(); // { dg-warning "dangling reference" }
diff --git a/libstdc++-v3/include/bits/locale_classes.tcc b/libstdc++-v3/include/bits/locale_classes.tcc
index 64cd7534dc6..9cc4f238ee7 100644
--- a/libstdc++-v3/include/bits/locale_classes.tcc
+++ b/libstdc++-v3/include/bits/locale_classes.tcc
@@ -127,6 +127,8 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
* @return Reference to facet of type Facet.
* @throw std::bad_cast if @p __loc doesn't contain a facet of type _Facet.
*/
+#pragma GCC diagnostic push
+#pragma GCC diagnostic ignored "-Wdangling-reference"
template<typename _Facet>
const _Facet&
use_facet(const locale& __loc)
@@ -141,6 +143,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
return static_cast<const _Facet&>(*__facets[__i]);
#endif
}
+#pragma GCC diagnostic pop
// Generic version does nothing.
base-commit: f896c13489d22b30d01257bc8316ab97b3359d1c
--
2.37.3
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v4] c++: Implement -Wdangling-reference [PR106393]
2022-10-26 18:26 ` [PATCH v4] " Marek Polacek
@ 2022-10-26 18:42 ` Jason Merrill
0 siblings, 0 replies; 14+ messages in thread
From: Jason Merrill @ 2022-10-26 18:42 UTC (permalink / raw)
To: Marek Polacek; +Cc: Jonathan Wakely, GCC Patches
On 10/26/22 14:26, Marek Polacek wrote:
> On Wed, Oct 26, 2022 at 12:42:27PM -0400, Jason Merrill wrote:
>> On 10/26/22 12:10, Marek Polacek wrote:
>>> As before, I've tested this patch twice, once with -Wdangling-reference
>>> enabled by default, once with -Wdangling-reference enabled by -Wextra.
>>> The couple of FAILs I saw were true positives (e.g., rv-conv1.C, rv-func2.C).
>>
>> I might actually add it to -Wall, the false positive rate sounds pretty low
>> at this point.
>
> Done (and invoke.texi adjusted).
>
>>> +static tree
>>> +do_warn_dangling_reference (tree expr)
>>> +{
>>> + STRIP_NOPS (expr);
>>> + switch (TREE_CODE (expr))
>>> + {
>>> + case CALL_EXPR:
>>> + {
>>> + tree fndecl = cp_get_callee_fndecl_nofold (expr);
>>> + if (!fndecl
>>> + || 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_P (TREE_TYPE (TREE_TYPE (fndecl))))
>>
>> Maybe TYPE_REF_OBJ_P? Either way is fine.
>
> Adjusted.
>
>>> --- 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
>>
>> You probably need to check VAR_P (bare_retval) here, static_flag is used for
>> various other things in other tree codes.
>
> Added VAR_P.
>
>>> + && 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..0a3a174cdc8 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,42 @@ 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 checked as well, but only their object argument:
>>> +
>>> +@smallexample
>>> +struct S @{
>>> + const S& self () @{ return *this; @}
>>> +@};
>>> +const S& s = S().self(); // s is dangling
>>> +@end smallexample
>>> +
>>> +This warning is enabled by @option{-Wextra}.
>>
>> It would be good to mention using #pragma to disable the warning around safe
>> functions.
>
> Yes, text added.
>
> Now that the warning is in -Wall, I've added -Wno-dangling-reference to
> g++.dg/warn/Wdangling-pointer-2.C.
>
> Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk?
OK, thanks.
> -- >8 --
> This patch implements a new experimental warning (enabled by -Wall) 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 warns when the function in question is a member function, but only if
> the function is invoked on a temporary object, otherwise the warning
> would emit loads of warnings for valid code like obj.emplace<T>({0}, 0).
> It does detect the dangling reference in:
>
> struct S {
> const S& self () { return *this; }
> };
> const S& s = S().self();
>
> It warns in member initializer lists as well:
>
> const int& f(const int& i) { return i; }
> struct S {
> const int &r;
> S() : r(f(10)) { }
> };
>
> I've run the testsuite/bootstrap with the warning enabled by default.
> There were just a few FAILs, all of which look like genuine bugs.
> A bootstrap with the warning enabled by default passed as well.
>
> 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 a #pragma and code to suppress the
> warning.
>
> 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): ...here. Don't return false just
> because a ck_base is missing. Use expr_represents_temporary_p.
> (do_warn_dangling_reference): New.
> (maybe_warn_dangling_reference): New.
> (extend_ref_init_temps): Call maybe_warn_dangling_reference.
> * typeck.cc (check_return_expr): Suppress -Wdangling-reference
> warnings.
>
> gcc/ChangeLog:
>
> * doc/invoke.texi: Document -Wdangling-reference.
>
> libstdc++-v3/ChangeLog:
>
> * include/bits/locale_classes.tcc: Add #pragma to disable
> -Wdangling-reference with std::use_facet.
>
> gcc/testsuite/ChangeLog:
>
> * g++.dg/cpp23/elision4.C: Use -Wdangling-reference, add dg-warning.
> * g++.dg/cpp23/elision7.C: Likewise.
> * g++.dg/warn/Wdangling-pointer-2.C: Use -Wno-dangling-reference.
> * g++.dg/warn/Wdangling-reference1.C: New test.
> * g++.dg/warn/Wdangling-reference2.C: New test.
> * g++.dg/warn/Wdangling-reference3.C: New test.
> ---
> gcc/c-family/c.opt | 4 +
> gcc/cp/call.cc | 148 ++++++++++++++++--
> gcc/cp/cp-tree.h | 4 +-
> gcc/cp/typeck.cc | 11 ++
> gcc/doc/invoke.texi | 51 +++++-
> gcc/testsuite/g++.dg/cpp23/elision4.C | 5 +-
> gcc/testsuite/g++.dg/cpp23/elision7.C | 3 +-
> .../g++.dg/warn/Wdangling-pointer-2.C | 2 +-
> .../g++.dg/warn/Wdangling-reference1.C | 144 +++++++++++++++++
> .../g++.dg/warn/Wdangling-reference2.C | 28 ++++
> .../g++.dg/warn/Wdangling-reference3.C | 24 +++
> libstdc++-v3/include/bits/locale_classes.tcc | 3 +
> 12 files changed, 409 insertions(+), 18 deletions(-)
> create mode 100644 gcc/testsuite/g++.dg/warn/Wdangling-reference1.C
> create mode 100644 gcc/testsuite/g++.dg/warn/Wdangling-reference2.C
> create mode 100644 gcc/testsuite/g++.dg/warn/Wdangling-reference3.C
>
> diff --git a/gcc/c-family/c.opt b/gcc/c-family/c.opt
> index 01d480759ae..070f85c81d2 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++, Wall)
> +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..951b9fd2a88 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,121 @@ initialize_reference (tree type, tree expr,
> return expr;
> }
>
> +/* 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
> + if none found. For instance:
> +
> + const S& s = S().self(); // S::self (&TARGET_EXPR <...>)
> + 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
> +do_warn_dangling_reference (tree expr)
> +{
> + STRIP_NOPS (expr);
> + switch (TREE_CODE (expr))
> + {
> + case CALL_EXPR:
> + {
> + tree fndecl = cp_get_callee_fndecl_nofold (expr);
> + if (!fndecl
> + || 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))))
> + 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)
> + {
> + tree arg = CALL_EXPR_ARG (expr, i);
> + /* Check that this argument initializes a reference, except for
> + the argument initializing the object of a member function. */
> + if (!DECL_NONSTATIC_MEMBER_FUNCTION_P (fndecl)
> + && !TYPE_REF_P (TREE_TYPE (arg)))
> + continue;
> + /* It could also be another call taking a temporary and returning
> + it and initializing this reference parameter. */
> + if (do_warn_dangling_reference (arg))
> + return expr;
> + STRIP_NOPS (arg);
> + if (TREE_CODE (arg) == ADDR_EXPR)
> + arg = TREE_OPERAND (arg, 0);
> + if (expr_represents_temporary_p (arg))
> + return expr;
> + /* Don't warn about member function like:
> + std::any a(...);
> + S& s = a.emplace<S>({0}, 0);
> + which constructs a new object and returns a reference to it, but
> + we still want to detect:
> + struct S { const S& self () { return *this; } };
> + const S& s = S().self();
> + where 's' dangles. If we've gotten here, the object this function
> + is invoked on is not a temporary. */
> + if (DECL_NONSTATIC_MEMBER_FUNCTION_P (fndecl))
> + break;
> + }
> + return NULL_TREE;
> + }
> + case COMPOUND_EXPR:
> + return do_warn_dangling_reference (TREE_OPERAND (expr, 1));
> + case COND_EXPR:
> + if (tree t = do_warn_dangling_reference (TREE_OPERAND (expr, 1)))
> + return t;
> + return do_warn_dangling_reference (TREE_OPERAND (expr, 2));
> + case PAREN_EXPR:
> + return do_warn_dangling_reference (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.
> +
> + DECL is the reference being initialized, INIT is the initializer. */
> +
> +static void
> +maybe_warn_dangling_reference (const_tree decl, tree init)
> +{
> + if (!warn_dangling_reference)
> + return;
> + if (!TYPE_REF_P (TREE_TYPE (decl)))
> + return;
> + if (tree call = do_warn_dangling_reference (init))
> + {
> + 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 +13646,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;
> +
> + maybe_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 867096b08c6..40f5bf802c3 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..4605f734e2d 100644
> --- a/gcc/cp/typeck.cc
> +++ b/gcc/cp/typeck.cc
> @@ -11246,6 +11246,17 @@ 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
> + && VAR_P (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..9f0e5460861 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,54 @@ 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 checked as well, but only their object argument:
> +
> +@smallexample
> +struct S @{
> + const S& self () @{ return *this; @}
> +@};
> +const S& s = S().self(); // s is dangling
> +@end smallexample
> +
> +Certain functions are safe in this respect, for example @code{std::use_facet}:
> +they take and return a reference, but they don't return one of its arguments,
> +which can fool the warning. Such functions can be excluded from the warning
> +by wrapping them in a @code{#pragma}:
> +
> +@smallexample
> +#pragma GCC diagnostic push
> +#pragma GCC diagnostic ignored "-Wdangling-reference"
> +const T& foo (const T&) @{ @dots{} @}
> +#pragma GCC diagnostic pop
> +@end smallexample
> +
> +This warning is enabled by @option{-Wall}.
> +
> @item -Wdelete-non-virtual-dtor @r{(C++ and Objective-C++ only)}
> @opindex Wdelete-non-virtual-dtor
> @opindex Wno-delete-non-virtual-dtor
> 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-pointer-2.C b/gcc/testsuite/g++.dg/warn/Wdangling-pointer-2.C
> index 151418f3c32..802ac9cd954 100644
> --- a/gcc/testsuite/g++.dg/warn/Wdangling-pointer-2.C
> +++ b/gcc/testsuite/g++.dg/warn/Wdangling-pointer-2.C
> @@ -1,5 +1,5 @@
> /* { dg-do compile }
> - { dg-options "-O1 -Wall -Wno-class-memaccess" } */
> + { dg-options "-O1 -Wall -Wno-class-memaccess -Wno-dangling-reference" } */
>
> struct A { A (); };
>
> 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..97c81ee716c
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/warn/Wdangling-reference1.C
> @@ -0,0 +1,144 @@
> +// PR c++/106393
> +// { dg-do compile { target c++11 } }
> +// { dg-options "-Wdangling-reference" }
> +
> +const int& f(const int& i) { return i; }
> +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. No dangling reference.
> +const int& r11 = g ? f(10) : 9;
> +const int& r12 = g ? 9 : f(10);
> +// r12 = f (f ((const int &) &TARGET_EXPR <D.2459, 1>))
> +const int& r13 = f(f(1)); // { dg-warning "dangling reference" }
> +const int& r14 = f(f_(1)); // { dg-warning "dangling reference" }
> +const int& r15 = f(g ? f(1) : f(2)); // { dg-warning "dangling reference" }
> +const int& r16 = f(*&f(1)); // { dg-warning "dangling reference" }
> +const int& r17 = rp(&f(1));
> +const int& r18 = rp(&f(g));
> +const int& r19 = h(f(1));
> +// Other forms of initializers.
> +const int& r20(f(10)); // { dg-warning "dangling reference" }
> +const int& r21(f(10)); // { dg-warning "dangling reference" }
> +// Returns a ref, but doesn't have a parameter of reference type.
> +const int& r22 = h(10);
> +const int& r23 = g ? h(10) : f(10); // { dg-warning "dangling reference" }
> +const int& r24 = g ? f(10) : h(10); // { dg-warning "dangling reference" }
> +const int& r25 = g ? h(10) : (1, f(10)); // { dg-warning "dangling reference" }
> +const int& r26 = g ? (1, f(10)) : h(10); // { dg-warning "dangling reference" }
> +const int& r29 = f((f_(1), 1)); // { dg-warning "dangling reference" }
> +const int& r30 = f((f_(1), g));
> +
> +struct Z {
> + operator int() { return 42; }
> +};
> +
> +const int& r27 = f(Z()); // { dg-warning "dangling reference" }
> +const int& r28 = f(true ? Z() : Z()); // { dg-warning "dangling reference" }
> +
> +const int& operator|(const int &, Z);
> +const int& r31 = 1 | Z(); // { 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); // { dg-warning "dangling reference" }
> +
> +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" }
> +
> +struct Y {
> + operator int&();
> + operator int&&();
> + const int& foo(const int&);
> +};
> +
> +// 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" }
> +const int& t1 = Y().foo(10); // { dg-warning "dangling reference" }
> 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));
> +}
> diff --git a/gcc/testsuite/g++.dg/warn/Wdangling-reference3.C b/gcc/testsuite/g++.dg/warn/Wdangling-reference3.C
> new file mode 100644
> index 00000000000..4bc20c13b3f
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/warn/Wdangling-reference3.C
> @@ -0,0 +1,24 @@
> +// PR c++/106393
> +// { dg-do compile { target c++11 } }
> +// { dg-options "-Wdangling-reference" }
> +
> +struct A {
> + int ar[4];
> + int& operator[](int i) { return ar[i]; }
> +};
> +const int &r = A()[2]; // { dg-warning "dangling reference" }
> +
> +struct S {
> + const S& self () { return *this; }
> +};
> +const S& s = S().self(); // { dg-warning "dangling reference" }
> +
> +struct G {
> + const G& g() { return *this; }
> +};
> +
> +struct F {
> + G& f();
> +};
> +
> +const G& g = F().f().g(); // { dg-warning "dangling reference" }
> diff --git a/libstdc++-v3/include/bits/locale_classes.tcc b/libstdc++-v3/include/bits/locale_classes.tcc
> index 64cd7534dc6..9cc4f238ee7 100644
> --- a/libstdc++-v3/include/bits/locale_classes.tcc
> +++ b/libstdc++-v3/include/bits/locale_classes.tcc
> @@ -127,6 +127,8 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
> * @return Reference to facet of type Facet.
> * @throw std::bad_cast if @p __loc doesn't contain a facet of type _Facet.
> */
> +#pragma GCC diagnostic push
> +#pragma GCC diagnostic ignored "-Wdangling-reference"
> template<typename _Facet>
> const _Facet&
> use_facet(const locale& __loc)
> @@ -141,6 +143,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
> return static_cast<const _Facet&>(*__facets[__i]);
> #endif
> }
> +#pragma GCC diagnostic pop
>
>
> // Generic version does nothing.
>
> base-commit: f896c13489d22b30d01257bc8316ab97b3359d1c
^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2022-10-26 18:42 UTC | newest]
Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-10-21 23:28 [PATCH] c++: Implement -Wdangling-reference [PR106393] 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 ` [PATCH v2] " Marek Polacek
2022-10-25 15:53 ` 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
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).