Re: [PATCH v4] c++: Implement -Wdangling-reference [PR106393]
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
:
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. Wit
[PATCH v4] c++: Implement -Wdangling-reference [PR106393]
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
:
Capturing the result of std::max by reference pro
