public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [PATCH] Add fix-it hint for missing return statement in assignment operators (PR c++/85523)
@ 2018-05-01  0:22 David Malcolm
  2018-05-01  0:25 ` David Malcolm
  2018-05-01 11:18 ` Nathan Sidwell
  0 siblings, 2 replies; 5+ messages in thread
From: David Malcolm @ 2018-05-01  0:22 UTC (permalink / raw)
  To: gcc-patches, Jonathan Wakely, Nathan Sidwell; +Cc: Andrew Haley, David Malcolm

Following on from the thread on the "gcc" list here:

  https://gcc.gnu.org/ml/gcc/2018-04/msg00172.html

here's an updated version of Jonathan's patch, which:

* eliminates the separate "note" in favor of simplying having
  the warning itself emit the "return *this;" fix-it hint

   pr85523.C: In member function 's1& s1::operator=(const s1&)':
   pr85523.C:7:30: warning: no return statement in function returning
   non-void [-Wreturn-type]
   s1& operator=(const s1&) { }
                              ^
                              return *this;

  In doing so I guarded it with a call to:
     global_dc->option_enabled (OPT_Wreturn_type
  as per the insides of diagnostic.s, since adding a fix-it hint to
  a rich_location is non-trivial (e.g. it requires an allocation).

* uses the new gcc_rich_location::add_fixit_insert_formatted to
  make the fix-it hint respect typical C++ code formatting
  conventions

* adds testcases

Successfully bootstrapped & regrtested on x86_64-pc-linux-gnu.

OK for trunk?


Note for ChangeLog: jwakely is co-author.

gcc/cp/ChangeLog:
	PR c++/85523
	* decl.c: Include "gcc-rich-location.h".
	(add_return_star_this_fixit): New function.
	(finish_function): When warning about missing return statements in
	functions returning non-void, add a "return *this;" fix-it hint for
	assignment operators.

gcc/testsuite/ChangeLog:
	PR c++/85523
	* g++.dg/pr85523.C: New test.
---
 gcc/cp/decl.c                  | 34 +++++++++++++++-
 gcc/testsuite/g++.dg/pr85523.C | 88 ++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 120 insertions(+), 2 deletions(-)
 create mode 100644 gcc/testsuite/g++.dg/pr85523.C

diff --git a/gcc/cp/decl.c b/gcc/cp/decl.c
index 07f3a61..7952106 100644
--- a/gcc/cp/decl.c
+++ b/gcc/cp/decl.c
@@ -51,6 +51,7 @@ along with GCC; see the file COPYING3.  If not see
 #include "builtins.h"
 #include "gimplify.h"
 #include "asan.h"
+#include "gcc-rich-location.h"
 
 /* Possible cases of bad specifiers type used by bad_specifiers. */
 enum bad_spec_place {
@@ -15677,6 +15678,22 @@ maybe_save_function_definition (tree fun)
     register_constexpr_fundef (fun, DECL_SAVED_TREE (fun));
 }
 
+/* Attempt to add a fix-it hint to RICHLOC suggesting the insertion
+   of "return *this;" immediately before its location, using FNDECL's
+   first statement (if any) to give the indentation, if appropriate.  */
+
+static void
+add_return_star_this_fixit (gcc_rich_location *richloc, tree fndecl)
+{
+  location_t indent = UNKNOWN_LOCATION;
+  tree stmts = expr_first (DECL_SAVED_TREE (fndecl));
+  if (stmts)
+    indent = EXPR_LOCATION (stmts);
+  richloc->add_fixit_insert_formatted ("return *this;",
+				       richloc->get_loc (),
+				       indent);
+}
+
 /* Finish up a function declaration and compile that function
    all the way to assembler language output.  The free the storage
    for the function definition. INLINE_P is TRUE if we just
@@ -15870,8 +15887,21 @@ finish_function (bool inline_p)
       && !DECL_DESTRUCTOR_P (fndecl)
       && targetm.warn_func_return (fndecl))
     {
-      warning (OPT_Wreturn_type,
- 	       "no return statement in function returning non-void");
+      gcc_rich_location richloc (input_location);
+      /* Potentially add a "return *this;" fix-it hint for
+	 assignment operators.  */
+      if (IDENTIFIER_ASSIGN_OP_P (DECL_NAME (fndecl)))
+	{
+	  tree valtype = TREE_TYPE (DECL_RESULT (fndecl));
+	  if (TREE_CODE (valtype) == REFERENCE_TYPE
+	      && same_type_ignoring_top_level_qualifiers_p
+		  (TREE_TYPE (valtype), TREE_TYPE (current_class_ref)))
+	    if (global_dc->option_enabled (OPT_Wreturn_type,
+					   global_dc->option_state))
+	      add_return_star_this_fixit (&richloc, fndecl);
+	}
+      warning_at (&richloc, OPT_Wreturn_type,
+		  "no return statement in function returning non-void");
       TREE_NO_WARNING (fndecl) = 1;
     }
 
diff --git a/gcc/testsuite/g++.dg/pr85523.C b/gcc/testsuite/g++.dg/pr85523.C
new file mode 100644
index 0000000..9cd939b
--- /dev/null
+++ b/gcc/testsuite/g++.dg/pr85523.C
@@ -0,0 +1,88 @@
+/* { dg-options "-fdiagnostics-show-caret" } */
+
+/* Verify that we emit a "return *this;" fix-it hint for
+   a missing return in an assignment operator.  */
+
+struct s1 {
+  s1& operator=(const s1&) { } // { dg-warning "no return statement in function returning non-void" }
+  /* { dg-begin-multiline-output "" }
+   s1& operator=(const s1&) { }
+                              ^
+                              return *this;
+     { dg-end-multiline-output "" } */
+};
+
+/* Likewise for +=.  */
+
+struct s2 {
+  s2& operator+=(const s2&) {} // { dg-warning "no return statement in function returning non-void" }
+  /* { dg-begin-multiline-output "" }
+   s2& operator+=(const s2&) {}
+                              ^
+                              return *this;
+     { dg-end-multiline-output "" } */
+};
+
+/* No warning for "void" return.  */
+
+struct s3 {
+  void operator=(const s3&) { }
+};
+
+/* We shouldn't issue the fix-it hint if the return type isn't right.  */
+
+struct s4 {
+  int operator=(int) { } // { dg-warning "no return statement in function returning non-void" }
+  /* { dg-begin-multiline-output "" }
+   int operator=(int) { }
+                        ^
+     { dg-end-multiline-output "" } */
+};
+
+/* Example of a multi-line fix-it hint.  */
+
+struct s5 {
+  int i;
+  s5& operator=(const s5& z) {
+    i = z.i;
+  } // { dg-warning "no return statement in function returning non-void" }
+  /* { dg-begin-multiline-output "" }
++    return *this;
+   }
+   ^
+     { dg-end-multiline-output "" } */
+};
+
+/* Example of a multi-line fix-it hint with other statements.  */
+
+extern void log (const char *);
+struct s6 {
+  int i;
+  s6& operator=(const s6& z) {
+    log ("operator=\n");
+    i = z.i;
+  } // { dg-warning "no return statement in function returning non-void" }
+  /* { dg-begin-multiline-output "" }
++    return *this;
+   }
+   ^
+     { dg-end-multiline-output "" } */
+};
+
+/* Another example of a multi-line fix-it hint with other statements.  */
+
+struct s7 {
+  int i;
+  s7& operator=(const s6& z) {
+    if (z.i)
+      log ("operator=\n");
+    else
+      log ("operator=\n");
+    i = z.i;
+  } // { dg-warning "no return statement in function returning non-void" }
+  /* { dg-begin-multiline-output "" }
++    return *this;
+   }
+   ^
+     { dg-end-multiline-output "" } */
+};
-- 
1.8.5.3

^ permalink raw reply	[flat|nested] 5+ messages in thread
end of thread, other threads:[~2018-08-07  1:45 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-05-01  0:22 [PATCH] Add fix-it hint for missing return statement in assignment operators (PR c++/85523) David Malcolm
2018-05-01  0:25 ` David Malcolm
2018-05-01 11:18 ` Nathan Sidwell
2018-08-03 18:41   ` David Malcolm
2018-08-07  1:45     ` H.J. Lu

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).