[PATCH] fix-it hints for warn_uninit

[PATCH] fix-it hints for warn_uninit

[David Malcolm]

We warn about uses of uninitialized variables from the middle end, via
warn_uninit.

This patch adds the ability for such warnings to contain fix-it hints,
showing the user how to zero-initialize the relevant variables.
Naturally this is highly frontend-dependent, so the patch adds a langhook:
LANG_HOOKS_GET_UNINITIALIZED_DECL_FIX to allow the frontend to optionally
provide a fragment of text to be inserted after the decl.

This is implemented for the C and C++ frontends, for a subset of types,
falling back to not emitting a hint if it's not clearly correct to do so.
The precise text varies with the type e.g. "0" vs "0.0f" vs "0.0" vs "false"
vs "NULL".

In combination with -fdiagnostics-generate-patch this can generate output
like this:

  --- ../../src/gcc/testsuite/c-c++-common/fix-missing-initializer-1.c
  +++ ../../src/gcc/testsuite/c-c++-common/fix-missing-initializer-1.c
  @@ -2,7 +2,7 @@

   int test_int (void)
   {
  -  int ivar;
  +  int ivar = 0;
     return ivar;  /* { dg-warning "used uninitialized" } */
     /* { dg-begin-multiline-output "" }
      return ivar;
  @@ -17,7 +17,7 @@

   char test_char (void)
   {
  -  char cvar;
  +  char cvar = 0;
     return cvar;  /* { dg-warning "used uninitialized" } */
     /* { dg-begin-multiline-output "" }
      return cvar;
  @@ -32,7 +32,7 @@

   float test_float (void)
   {
  -  float fvar;
  +  float fvar = 0.0f;
     return fvar;  /* { dg-warning "used uninitialized" } */
     /* { dg-begin-multiline-output "" }
      return fvar;
  @@ -47,7 +47,7 @@

   double test_double (void)
   {
  -  double dvar;
  +  double dvar = 0.0;
     return dvar;  /* { dg-warning "used uninitialized" } */
     /* { dg-begin-multiline-output "" }
      return dvar;
  @@ -73,7 +73,7 @@

   int test_multiple_on_one_line (void)
   {
  -  int ivar_a, ivar_b;
  +  int ivar_a = 0, ivar_b = 0;
     int tot = 0;
     tot += ivar_a; /* { dg-warning "used uninitialized" } */
     /* { dg-begin-multiline-output "" }

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

Adds 93 PASS results to g++.sum.
Adds 33 PASS results to gcc.sum, converting
  gcc.dg/uninit-15.c  (test for warnings, line 23)
from an XFAIL to a PASS, due to the fix-it hint being emitted for the:
  int j;
decl, and thus forcing the "note: 'j' was declared here"
to be printed.

OK for trunk?

gcc/c-family/ChangeLog:
	* c-common.c (c_common_get_uninitialized_decl_fix): New function.
	* c-common.h (c_common_get_uninitialized_decl_fix): New decl.

gcc/c/ChangeLog:
	* c-lang.c (LANG_HOOKS_GET_UNINITIALIZED_DECL_FIX): Redefine as
	c_common_get_uninitialized_decl_fix.

gcc/cp/ChangeLog:
	* cp-lang.c (LANG_HOOKS_GET_UNINITIALIZED_DECL_FIX): Redefine as
	c_common_get_uninitialized_decl_fix.

gcc/ChangeLog:
	* langhooks-def.h (lhd_get_uninitialized_decl_fix): New decl.
	(LANG_HOOKS_GET_UNINITIALIZED_DECL_FIX): Define.
	(LANG_HOOKS_INITIALIZER): Add
	LANG_HOOKS_GET_UNINITIALIZED_DECL_FIX.
	* langhooks.c (lhd_get_uninitialized_decl_fix): New function.
	* langhooks.h (struct lang_hooks_for_decls): Add field
	"get_uninitialized_decl_fix".

gcc/testsuite/ChangeLog:
	* c-c++-common/fix-missing-initializer-1.c: New test case.
	* c-c++-common/fix-missing-initializer-2.c: New test case.
	* g++.dg/fix-missing-initializer-1.C: New test case.
	* gcc.dg/diagnostic-tree-expr-ranges-2.c: Add fix-it hints to
	expected output.
	* gcc.dg/fix-missing-initializer-1.c: New test case.
	* gcc.dg/uninit-15.c: Remove xfail from "int j;".

gcc/ChangeLog:
	* tree-ssa-uninit.c (warn_uninit): When issuing note about
	location of declaration, potentially provide a fix-it hint.
---
 gcc/c-family/c-common.c                            |  53 +++++++++++
 gcc/c-family/c-common.h                            |   2 +
 gcc/c/c-lang.c                                     |   3 +
 gcc/cp/cp-lang.c                                   |   3 +
 gcc/langhooks-def.h                                |   3 +
 gcc/langhooks.c                                    |   9 ++
 gcc/langhooks.h                                    |   7 ++
 .../c-c++-common/fix-missing-initializer-1.c       | 101 +++++++++++++++++++++
 .../c-c++-common/fix-missing-initializer-2.c       |  36 ++++++++
 gcc/testsuite/g++.dg/fix-missing-initializer-1.C   |  18 ++++
 .../gcc.dg/diagnostic-tree-expr-ranges-2.c         |  10 ++
 gcc/testsuite/gcc.dg/fix-missing-initializer-1.c   |  18 ++++
 gcc/testsuite/gcc.dg/uninit-15.c                   |   2 +-
 gcc/tree-ssa-uninit.c                              |  13 ++-
 14 files changed, 275 insertions(+), 3 deletions(-)
 create mode 100644 gcc/testsuite/c-c++-common/fix-missing-initializer-1.c
 create mode 100644 gcc/testsuite/c-c++-common/fix-missing-initializer-2.c
 create mode 100644 gcc/testsuite/g++.dg/fix-missing-initializer-1.C
 create mode 100644 gcc/testsuite/gcc.dg/fix-missing-initializer-1.c

diff --git a/gcc/c-family/c-common.c b/gcc/c-family/c-common.c
index 328e5f6..f6325311 100644
--- a/gcc/c-family/c-common.c
+++ b/gcc/c-family/c-common.c
@@ -13060,4 +13060,57 @@ diagnose_mismatched_attributes (tree olddecl, tree newdecl)
   return warned;
 }
 
+/* Implementation of LANG_HOOKS_GET_UNINITIALIZED_DECL_FIX.
+   Given an uninitialized warning about a usage of DECL, return
+   a fragment of code suitable for insertion immediately after
+   DECL for initializing it, for use as a fix-it hint to
+   suppress the warning.
+   Return NULL if it's not possible to emit such a hint.  */
+
+const char *
+c_common_get_uninitialized_decl_fix (const_tree decl)
+{
+  gcc_assert (decl);
+  tree type = TREE_TYPE (decl);
+  switch (TREE_CODE (type))
+    {
+    case INTEGER_TYPE:
+      return " = 0";
+
+    case REAL_TYPE:
+      {
+	if (type == float_type_node)
+	  return " = 0.0f";
+	else if (type == double_type_node)
+	  return " = 0.0";
+	else
+	  return NULL;
+      }
+
+    case POINTER_TYPE:
+      {
+	tree pointed_to = TREE_TYPE (type);
+	if (TREE_CODE (pointed_to) == FUNCTION_TYPE)
+	  /* We can't easily insert a fixit for a function pointer,
+	     unless it's a typedef.  For now don't attempt to.  */
+	  return NULL;
+
+	/* If the macro "NULL" has been defined, then suggest it.  */
+	if (cpp_defined (parse_in, (const unsigned char *)"NULL", 4))
+	  return " = NULL";
+	else
+	  return NULL;
+      }
+
+    case BOOLEAN_TYPE:
+      /* If we have a BOOLEAN_TYPE, then presumably we have either
+	 C++, or C99 onwards.  */
+      return " = false";
+
+    default:
+      return NULL;
+    }
+}
+
+
 #include "gt-c-family-c-common.h"
diff --git a/gcc/c-family/c-common.h b/gcc/c-family/c-common.h
index 42ce969..a3c6f54 100644
--- a/gcc/c-family/c-common.h
+++ b/gcc/c-family/c-common.h
@@ -1162,6 +1162,8 @@ class substring_loc
   int m_end_idx;
 };
 
+extern const char *c_common_get_uninitialized_decl_fix (const_tree);
+
 /* In c-gimplify.c  */
 extern void c_genericize (tree);
 extern int c_gimplify_expr (tree *, gimple_seq *, gimple_seq *);
diff --git a/gcc/c/c-lang.c b/gcc/c/c-lang.c
index b26be6a..866d351 100644
--- a/gcc/c/c-lang.c
+++ b/gcc/c/c-lang.c
@@ -37,6 +37,9 @@ enum c_language_kind c_language = clk_c;
 #define LANG_HOOKS_INIT c_objc_common_init
 #undef LANG_HOOKS_INIT_TS
 #define LANG_HOOKS_INIT_TS c_common_init_ts
+#undef LANG_HOOKS_GET_UNINITIALIZED_DECL_FIX
+#define LANG_HOOKS_GET_UNINITIALIZED_DECL_FIX \
+  c_common_get_uninitialized_decl_fix
 
 #if CHECKING_P
 #undef LANG_HOOKS_RUN_LANG_SELFTESTS
diff --git a/gcc/cp/cp-lang.c b/gcc/cp/cp-lang.c
index 8cfee4f..75bab92 100644
--- a/gcc/cp/cp-lang.c
+++ b/gcc/cp/cp-lang.c
@@ -78,6 +78,9 @@ static tree cxx_enum_underlying_base_type (const_tree);
 #define LANG_HOOKS_EH_RUNTIME_TYPE build_eh_type_type
 #undef LANG_HOOKS_ENUM_UNDERLYING_BASE_TYPE
 #define LANG_HOOKS_ENUM_UNDERLYING_BASE_TYPE cxx_enum_underlying_base_type
+#undef LANG_HOOKS_GET_UNINITIALIZED_DECL_FIX
+#define LANG_HOOKS_GET_UNINITIALIZED_DECL_FIX \
+  c_common_get_uninitialized_decl_fix
 
 /* Each front end provides its own lang hook initializer.  */
 struct lang_hooks lang_hooks = LANG_HOOKS_INITIALIZER;
diff --git a/gcc/langhooks-def.h b/gcc/langhooks-def.h
index 10d910c..962a708 100644
--- a/gcc/langhooks-def.h
+++ b/gcc/langhooks-def.h
@@ -56,6 +56,7 @@ extern void lhd_incomplete_type_error (location_t, const_tree, const_tree);
 extern tree lhd_type_promotes_to (tree);
 extern void lhd_register_builtin_type (tree, const char *);
 extern bool lhd_decl_ok_for_sibcall (const_tree);
+extern const char *lhd_get_uninitialized_decl_fix (const_tree);
 extern size_t lhd_tree_size (enum tree_code);
 extern HOST_WIDE_INT lhd_to_target_charset (HOST_WIDE_INT);
 extern tree lhd_expr_to_decl (tree, bool *, bool *);
@@ -215,6 +216,7 @@ extern tree lhd_make_node (enum tree_code);
 #define LANG_HOOKS_WARN_UNUSED_GLOBAL_DECL lhd_warn_unused_global_decl
 #define LANG_HOOKS_POST_COMPILATION_PARSING_CLEANUPS NULL
 #define LANG_HOOKS_DECL_OK_FOR_SIBCALL	lhd_decl_ok_for_sibcall
+#define LANG_HOOKS_GET_UNINITIALIZED_DECL_FIX lhd_get_uninitialized_decl_fix
 #define LANG_HOOKS_OMP_PRIVATIZE_BY_REFERENCE hook_bool_const_tree_false
 #define LANG_HOOKS_OMP_PREDETERMINED_SHARING lhd_omp_predetermined_sharing
 #define LANG_HOOKS_OMP_REPORT_DECL lhd_pass_through_t
@@ -241,6 +243,7 @@ extern tree lhd_make_node (enum tree_code);
   LANG_HOOKS_WARN_UNUSED_GLOBAL_DECL, \
   LANG_HOOKS_POST_COMPILATION_PARSING_CLEANUPS, \
   LANG_HOOKS_DECL_OK_FOR_SIBCALL, \
+  LANG_HOOKS_GET_UNINITIALIZED_DECL_FIX, \
   LANG_HOOKS_OMP_PRIVATIZE_BY_REFERENCE, \
   LANG_HOOKS_OMP_PREDETERMINED_SHARING, \
   LANG_HOOKS_OMP_REPORT_DECL, \
diff --git a/gcc/langhooks.c b/gcc/langhooks.c
index 3256a9d..667e39e 100644
--- a/gcc/langhooks.c
+++ b/gcc/langhooks.c
@@ -291,6 +291,15 @@ lhd_decl_ok_for_sibcall (const_tree decl ATTRIBUTE_UNUSED)
   return true;
 }
 
+/* Default implementation of LANG_HOOKS_GET_UNINITIALIZED_DECL_FIX.
+   Return NULL, meaning to not issue a fix-it hint.  */
+
+const char *
+lhd_get_uninitialized_decl_fix (const_tree decl ATTRIBUTE_UNUSED)
+{
+  return NULL;
+}
+
 /* Generic global declaration processing.  This is meant to be called
    by the front-ends at the end of parsing.  C/C++ do their own thing,
    but other front-ends may call this.  */
diff --git a/gcc/langhooks.h b/gcc/langhooks.h
index 44c258e..521e984 100644
--- a/gcc/langhooks.h
+++ b/gcc/langhooks.h
@@ -215,6 +215,13 @@ struct lang_hooks_for_decls
   /* True if this decl may be called via a sibcall.  */
   bool (*ok_for_sibcall) (const_tree);
 
+  /* Given an uninitialized warning about a usage of DECL, return
+     a fragment of code suitable for insertion immediately after
+     DECL for initializing it, for use as a fix-it hint to
+     suppress the warning, such as " = 0".
+     Return NULL if it's not possible to emit such a hint.  */
+  const char * (*get_uninitialized_decl_fix) (const_tree);
+
   /* True if OpenMP should privatize what this DECL points to rather
      than the DECL itself.  */
   bool (*omp_privatize_by_reference) (const_tree);
diff --git a/gcc/testsuite/c-c++-common/fix-missing-initializer-1.c b/gcc/testsuite/c-c++-common/fix-missing-initializer-1.c
new file mode 100644
index 0000000..f035195
--- /dev/null
+++ b/gcc/testsuite/c-c++-common/fix-missing-initializer-1.c
@@ -0,0 +1,101 @@
+/* { dg-options "-fdiagnostics-show-caret -Wuninitialized" } */
+
+int test_int (void)
+{
+  int ivar;
+  return ivar;  /* { dg-warning "used uninitialized" } */
+  /* { dg-begin-multiline-output "" }
+   return ivar;
+          ^~~~
+     { dg-end-multiline-output "" } */
+  /* { dg-begin-multiline-output "" }
+   int ivar;
+       ^~~~
+            = 0
+     { dg-end-multiline-output "" } */
+}
+
+char test_char (void)
+{
+  char cvar;
+  return cvar;  /* { dg-warning "used uninitialized" } */
+  /* { dg-begin-multiline-output "" }
+   return cvar;
+          ^~~~
+     { dg-end-multiline-output "" } */
+  /* { dg-begin-multiline-output "" }
+   char cvar;
+        ^~~~
+             = 0
+     { dg-end-multiline-output "" } */
+}
+
+float test_float (void)
+{
+  float fvar;
+  return fvar;  /* { dg-warning "used uninitialized" } */
+  /* { dg-begin-multiline-output "" }
+   return fvar;
+          ^~~~
+     { dg-end-multiline-output "" } */
+  /* { dg-begin-multiline-output "" }
+   float fvar;
+         ^~~~
+              = 0.0f
+     { dg-end-multiline-output "" } */
+}
+
+double test_double (void)
+{
+  double dvar;
+  return dvar;  /* { dg-warning "used uninitialized" } */
+  /* { dg-begin-multiline-output "" }
+   return dvar;
+          ^~~~
+     { dg-end-multiline-output "" } */
+  /* { dg-begin-multiline-output "" }
+   double dvar;
+          ^~~~
+               = 0.0
+     { dg-end-multiline-output "" } */
+}
+
+void *test_ptr (void)
+{
+  void *ptr;
+  return ptr; /* { dg-warning "used uninitialized" } */
+  /* { dg-begin-multiline-output "" }
+   return ptr;
+          ^~~
+     { dg-end-multiline-output "" } */
+  /* NULL is not defined in this file, so no fix-it hint for pointers.  */
+}
+
+int test_multiple_on_one_line (void)
+{
+  int ivar_a, ivar_b;
+  int tot = 0;
+  tot += ivar_a; /* { dg-warning "used uninitialized" } */
+  /* { dg-begin-multiline-output "" }
+   tot += ivar_a;
+   ~~~~^~~~~~~~~
+     { dg-end-multiline-output "" } */
+  /* { dg-begin-multiline-output "" }
+   int ivar_a, ivar_b;
+       ^~~~~~
+              = 0
+     { dg-end-multiline-output "" } */
+
+  tot += ivar_b; /* { dg-warning "used uninitialized" } */
+  /* { dg-begin-multiline-output "" }
+   tot += ivar_b;
+   ~~~~^~~~~~~~~
+     { dg-end-multiline-output "" } */
+  /* { dg-begin-multiline-output "" }
+   int ivar_a, ivar_b;
+               ^~~~~~
+                      = 0
+     { dg-end-multiline-output "" } */
+
+  return tot;
+}
diff --git a/gcc/testsuite/c-c++-common/fix-missing-initializer-2.c b/gcc/testsuite/c-c++-common/fix-missing-initializer-2.c
new file mode 100644
index 0000000..5e16b98
--- /dev/null
+++ b/gcc/testsuite/c-c++-common/fix-missing-initializer-2.c
@@ -0,0 +1,36 @@
+/* { dg-options "-fdiagnostics-show-caret -Wuninitialized" } */
+
+#define NULL ((void)0)
+
+/* NULL is defined in this file, so pointers should get fix-it hints.  */
+
+void *test_ptr (void)
+{
+  void *ptr;
+  return ptr; /* { dg-warning "used uninitialized" } */
+  /* { dg-begin-multiline-output "" }
+   return ptr;
+          ^~~
+     { dg-end-multiline-output "" } */
+  /* { dg-begin-multiline-output "" }
+   void *ptr;
+         ^~~
+             = NULL
+     { dg-end-multiline-output "" } */
+}
+
+/* ...apart from function pointers.  */
+
+typedef void (*fnptr_t) (void);
+
+fnptr_t test_fnptr (void)
+{
+  void (*fnptr) (void);
+  return fnptr;  /* { dg-warning "used uninitialized" } */
+  /* { dg-begin-multiline-output "" }
+   return fnptr;
+          ^~~~~
+     { dg-end-multiline-output "" } */
+  /* We don't have enough location information, so we can't provide an
+     initializer for the function pointer.  */
+}
diff --git a/gcc/testsuite/g++.dg/fix-missing-initializer-1.C b/gcc/testsuite/g++.dg/fix-missing-initializer-1.C
new file mode 100644
index 0000000..46bc7fc
--- /dev/null
+++ b/gcc/testsuite/g++.dg/fix-missing-initializer-1.C
@@ -0,0 +1,18 @@
+/* { dg-options "-fdiagnostics-show-caret -Wuninitialized" } */
+
+int test_bool (void)
+{
+  bool bvar;
+  return bvar; /* { dg-warning "used uninitialized" } */
+  /* { dg-begin-multiline-output "" }
+   return bvar;
+          ^~~~
+     { dg-end-multiline-output "" } */
+  /* { dg-begin-multiline-output "" }
+   bool bvar;
+        ^~~~
+             = false
+     { dg-end-multiline-output "" } */
+
+  return 0;
+}
diff --git a/gcc/testsuite/gcc.dg/diagnostic-tree-expr-ranges-2.c b/gcc/testsuite/gcc.dg/diagnostic-tree-expr-ranges-2.c
index 302e233..79233c3 100644
--- a/gcc/testsuite/gcc.dg/diagnostic-tree-expr-ranges-2.c
+++ b/gcc/testsuite/gcc.dg/diagnostic-tree-expr-ranges-2.c
@@ -9,6 +9,11 @@ int test_uninit_1 (void)
    return result;
           ^~~~~~
    { dg-end-multiline-output "" } */
+/* { dg-begin-multiline-output "" }
+   int result;
+       ^~~~~~
+              = 0
+   { dg-end-multiline-output "" } */
 }
 
 int test_uninit_2 (void)
@@ -19,5 +24,10 @@ int test_uninit_2 (void)
    result += 3;
    ~~~~~~~^~~~
    { dg-end-multiline-output "" } */
+/* { dg-begin-multiline-output "" }
+   int result;
+       ^~~~~~
+              = 0
+   { dg-end-multiline-output "" } */
   return result;
 }
diff --git a/gcc/testsuite/gcc.dg/fix-missing-initializer-1.c b/gcc/testsuite/gcc.dg/fix-missing-initializer-1.c
new file mode 100644
index 0000000..a1cc04e
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/fix-missing-initializer-1.c
@@ -0,0 +1,18 @@
+/* { dg-options "-fdiagnostics-show-caret -Wuninitialized" } */
+
+int test_bool (void)
+{
+  _Bool bvar;
+  return bvar; /* { dg-warning "used uninitialized" } */
+  /* { dg-begin-multiline-output "" }
+   return bvar;
+          ^~~~
+     { dg-end-multiline-output "" } */
+  /* { dg-begin-multiline-output "" }
+   _Bool bvar;
+         ^~~~
+              = false
+     { dg-end-multiline-output "" } */
+
+  return 0;
+}
diff --git a/gcc/testsuite/gcc.dg/uninit-15.c b/gcc/testsuite/gcc.dg/uninit-15.c
index 20bea95..02165ce 100644
--- a/gcc/testsuite/gcc.dg/uninit-15.c
+++ b/gcc/testsuite/gcc.dg/uninit-15.c
@@ -20,7 +20,7 @@ void baz (void);
 void
 bar (void)
 {
-  int j; /* { dg-message "note: 'j' was declared here" "" { xfail *-*-* } } */
+  int j; /* { dg-message "note: 'j' was declared here" } */
   for (; foo (j); ++j)  /* { dg-warning "'j' is used uninitialized" } */
     baz ();
 }
diff --git a/gcc/tree-ssa-uninit.c b/gcc/tree-ssa-uninit.c
index d5f0344..5725086 100644
--- a/gcc/tree-ssa-uninit.c
+++ b/gcc/tree-ssa-uninit.c
@@ -33,6 +33,7 @@ along with GCC; see the file COPYING3.  If not see
 #include "tree-ssa.h"
 #include "params.h"
 #include "tree-cfg.h"
+#include "langhooks.h"
 
 /* This implements the pass that does predicate aware warning on uses of
    possibly uninitialized variables.  The pass first collects the set of
@@ -180,11 +181,19 @@ warn_uninit (enum opt_code wc, tree t, tree expr, tree var,
 
       if (location == DECL_SOURCE_LOCATION (var))
 	return;
+      const char *initializer_fix
+	= lang_hooks.decls.get_uninitialized_decl_fix (var);
       if (xloc.file != floc.file
 	  || linemap_location_before_p (line_table, location, cfun_loc)
 	  || linemap_location_before_p (line_table, cfun->function_end_locus,
-					location))
-	inform (DECL_SOURCE_LOCATION (var), "%qD was declared here", var);
+					location)
+	  || initializer_fix)
+	{
+	  rich_location richloc (line_table, DECL_SOURCE_LOCATION (var));
+	  if (initializer_fix)
+	    richloc.add_fixit_insert_after (initializer_fix);
+	  inform_at_rich_loc (&richloc, "%qD was declared here", var);
+	}
     }
 }
 
-- 
1.8.5.3

Re: [PATCH] fix-it hints for warn_uninit

[Bernd Schmidt]

On 09/14/2016 02:45 AM, David Malcolm wrote:
> In combination with -fdiagnostics-generate-patch this can generate output
> like this:
>
>   --- ../../src/gcc/testsuite/c-c++-common/fix-missing-initializer-1.c
>   +++ ../../src/gcc/testsuite/c-c++-common/fix-missing-initializer-1.c
>   @@ -2,7 +2,7 @@
>
>    int test_int (void)
>    {
>   -  int ivar;
>   +  int ivar = 0;
>      return ivar;  /* { dg-warning "used uninitialized" } */
>      /* { dg-begin-multiline-output "" }
>       return ivar;

I have to admit I feel uneasy about this. Just initializing stuff to 
zero is the naive approach to shutting up the warning, but there's no 
reason to think it's the correct fix. I think this is a warning where a 
human who understands what's supposed to be going on needs to take a 
look. Automating the work of a bad programmer seems like it could lead 
to rather unfortuante outcomes.


Bernd

Re: [PATCH] fix-it hints for warn_uninit

[Trevor Saunders]

On Tue, Sep 13, 2016 at 08:45:25PM -0400, David Malcolm wrote:
> We warn about uses of uninitialized variables from the middle end, via
> warn_uninit.
> 
> This patch adds the ability for such warnings to contain fix-it hints,
> showing the user how to zero-initialize the relevant variables.
> Naturally this is highly frontend-dependent, so the patch adds a langhook:
> LANG_HOOKS_GET_UNINITIALIZED_DECL_FIX to allow the frontend to optionally
> provide a fragment of text to be inserted after the decl.
> 
> This is implemented for the C and C++ frontends, for a subset of types,
> falling back to not emitting a hint if it's not clearly correct to do so.
> The precise text varies with the type e.g. "0" vs "0.0f" vs "0.0" vs "false"
> vs "NULL".

Is this warning really a good candidate for a fixit? it doesn't seem at
all clear to me that zero initialize the variable is generally the right
way to fix the warning.  It may well be that it is uninitialized because
control flow is incorrect, or that 0 is a valid value and it would be
better to initialize with something else that is clearly bogus.

Trev

Re: [PATCH] fix-it hints for warn_uninit

[Jeff Law]

On 09/14/2016 05:34 AM, Bernd Schmidt wrote:
> On 09/14/2016 02:45 AM, David Malcolm wrote:
>> In combination with -fdiagnostics-generate-patch this can generate output
>> like this:
>>
>>   --- ../../src/gcc/testsuite/c-c++-common/fix-missing-initializer-1.c
>>   +++ ../../src/gcc/testsuite/c-c++-common/fix-missing-initializer-1.c
>>   @@ -2,7 +2,7 @@
>>
>>    int test_int (void)
>>    {
>>   -  int ivar;
>>   +  int ivar = 0;
>>      return ivar;  /* { dg-warning "used uninitialized" } */
>>      /* { dg-begin-multiline-output "" }
>>       return ivar;
>
> I have to admit I feel uneasy about this. Just initializing stuff to
> zero is the naive approach to shutting up the warning, but there's no
> reason to think it's the correct fix. I think this is a warning where a
> human who understands what's supposed to be going on needs to take a
> look. Automating the work of a bad programmer seems like it could lead
> to rather unfortuante outcomes.
I'd started writing something longer which said the same essentially the 
same thing.

For uninitialized variable uses the thing to do is to provide a human 
readable explanation of the path (conditions) which lead to the 
uninitialized use.  The developer really needs to decide what to do to 
fix their code.

jeff