diff mbox series

C++ PATCH to enhance -Wreturn-local-addr (c++/86982)

Message ID 20180906000153.GM12638@redhat.com
State New
Headers show
Series C++ PATCH to enhance -Wreturn-local-addr (c++/86982) | expand

Commit Message

Marek Polacek Sept. 6, 2018, 12:01 a.m. UTC 
Jonathan suggested to handle std::move and std::forward specially for the
purpose of -Wreturn-local-addr, because it's a fairly common mistake to
return a local var by reference.

Now that we have is_std_move_p it's fairly trivial to extend the warning.

Bootstrapped/regtested on x86_64-linux, ok for trunk?

2018-09-05  Marek Polacek  <polacek@redhat.com>

	PR c++/86982, -Wreturn-local-addr and std::move and std::forward.
	* typeck.c (maybe_warn_about_returning_address_of_local): Handle calls
	to std::move or std::forward.
	(is_std_forward_p): New function.

	* g++.dg/warn/Wreturn-local-addr-5.C: New test.

Comments

Marek Polacek Sept. 6, 2018, 12:13 a.m. UTC | #1 
On Wed, Sep 05, 2018 at 08:01:53PM -0400, Marek Polacek wrote:
> Jonathan suggested to handle std::move and std::forward specially for the

And for the record I meant "suggested handling"!
Jason Merrill Sept. 6, 2018, 12:21 a.m. UTC | #2 
OK.

On Wed, Sep 5, 2018 at 8:13 PM, Marek Polacek <polacek@redhat.com> wrote:
> On Wed, Sep 05, 2018 at 08:01:53PM -0400, Marek Polacek wrote:
>> Jonathan suggested to handle std::move and std::forward specially for the
>
> And for the record I meant "suggested handling"!
diff mbox series

Patch

diff --git gcc/cp/typeck.c gcc/cp/typeck.c
index 84cf4c478aa..9fa4c16bf14 100644
--- gcc/cp/typeck.c
+++ gcc/cp/typeck.c
@@ -63,6 +63,8 @@  static tree lookup_destructor (tree, tree, tree, tsubst_flags_t);
 static void error_args_num (location_t, tree, bool);
 static int convert_arguments (tree, vec<tree, va_gc> **, tree, int,
                               tsubst_flags_t);
+static bool is_std_move_p (tree);
+static bool is_std_forward_p (tree);
 
 /* Do `exp = require_complete_type (exp);' to make sure exp
    does not have an incomplete type.  (That includes void types.)
@@ -9071,6 +9073,15 @@  maybe_warn_about_returning_address_of_local (tree retval)
       STRIP_NOPS (whats_returned);
     }
 
+  /* As a special case, we handle a call to std::move or std::forward.  */
+  if (TREE_CODE (whats_returned) == CALL_EXPR
+      && (is_std_move_p (whats_returned)
+	  || is_std_forward_p (whats_returned)))
+    {
+      tree arg = CALL_EXPR_ARG (whats_returned, 0);
+      return maybe_warn_about_returning_address_of_local (arg);
+    }
+
   if (TREE_CODE (whats_returned) != ADDR_EXPR)
     return false;
   whats_returned = TREE_OPERAND (whats_returned, 0);
@@ -9136,6 +9147,23 @@  decl_in_std_namespace_p (tree decl)
 	  && DECL_NAMESPACE_STD_P (decl_namespace_context (decl)));
 }
 
+/* Returns true if FN, a CALL_EXPR, is a call to std::forward.  */
+
+static bool
+is_std_forward_p (tree fn)
+{
+  /* std::forward only takes one argument.  */
+  if (call_expr_nargs (fn) != 1)
+    return false;
+
+  tree fndecl = cp_get_callee_fndecl_nofold (fn);
+  if (!decl_in_std_namespace_p (fndecl))
+    return false;
+
+  tree name = DECL_NAME (fndecl);
+  return name && id_equal (name, "forward");
+}
+
 /* Returns true if FN, a CALL_EXPR, is a call to std::move.  */
 
 static bool
diff --git gcc/testsuite/g++.dg/warn/Wreturn-local-addr-5.C gcc/testsuite/g++.dg/warn/Wreturn-local-addr-5.C
index e69de29bb2d..76096279a64 100644
--- gcc/testsuite/g++.dg/warn/Wreturn-local-addr-5.C
+++ gcc/testsuite/g++.dg/warn/Wreturn-local-addr-5.C
@@ -0,0 +1,8 @@ 
+// PR c++/86982
+// { dg-do compile { target c++11 } }
+
+#include <utility>
+
+int&& f() { int i = 0; return std::move(i); } // { dg-warning "reference to local variable" }
+int&& g() { int i = 0; return std::forward<int>(i); } // { dg-warning "reference to local variable" }
+int&& h() { long l = 0; return std::forward<int>(l); } // { dg-warning "reference to temporary" }