public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* new option -Wno-maybe-uninitialized
@ 2011-04-08  0:24 Xinliang David Li
  2011-04-21 17:40 ` Jeff Law
  0 siblings, 1 reply; 8+ messages in thread
From: Xinliang David Li @ 2011-04-08  0:24 UTC (permalink / raw)
  To: GCC Patches

[-- Attachment #1: Type: text/plain, Size: 1002 bytes --]

Hi,

the following patch implements the option to fine control the emitted
warnings --
1) allow suppressing warnings for use of values that may be
uninitialized.  Definitely uninitialized values that may be used
warning is not affected
2) allow fine grain control on promotion of warnings to errors:
-Wno-error=maybe-uninitialized

This is useful for users who only care about definite uninitialized
variable warnings.

Ok for trunk?

thanks,

David


2011-04-07  Xinliang David Li  <davidxl@google.com>

	* tree-ssa-uninit.c (warn_uninitialized_phi): Pass
	warning code.
	* c-family/c-opts.c (c_common_handle_option): Set
	warn_maybe_uninitialized.
	* opts.c (common_handle_option): Ditto.
	* common.opt:  New option.
	* tree-ssa.c (warn_uninit): Add one more parameter.
	(warn_uninitialized_var): Pass warning code.
	* tree-flow.h: Interface change.



2011-04-07  Xinliang David Li  <davidxl@google.com>

        * gcc.dg/uninit-suppress.c: New test.
        * gcc.dg/uninit-suppress_2.c: New test.

[-- Attachment #2: maybe.p --]
[-- Type: text/x-pascal, Size: 8613 bytes --]

Index: doc/invoke.texi
===================================================================
--- doc/invoke.texi	(revision 171959)
+++ doc/invoke.texi	(working copy)
@@ -246,11 +246,11 @@ Objective-C and Objective-C++ Dialects}.
 -Wformat-security  -Wformat-y2k @gol
 -Wframe-larger-than=@var{len} -Wjump-misses-init -Wignored-qualifiers @gol
 -Wimplicit  -Wimplicit-function-declaration  -Wimplicit-int @gol
--Winit-self  -Winline @gol
+-Winit-self  -Winline -Wmaybe-uninitialized @gol
 -Wno-int-to-pointer-cast -Wno-invalid-offsetof @gol
 -Winvalid-pch -Wlarger-than=@var{len}  -Wunsafe-loop-optimizations @gol
 -Wlogical-op -Wlong-long @gol
--Wmain  -Wmissing-braces  -Wmissing-field-initializers @gol
+-Wmain -Wmaybe-uninitialized -Wmissing-braces  -Wmissing-field-initializers @gol
 -Wmissing-format-attribute  -Wmissing-include-dirs @gol
 -Wno-mudflap @gol
 -Wno-multichar  -Wnonnull  -Wno-overflow @gol
@@ -2945,6 +2945,7 @@ Options} and @ref{Objective-C and Object
 -Wcomment  @gol
 -Wformat   @gol
 -Wmain @r{(only for C/ObjC and unless} @option{-ffreestanding}@r{)}  @gol
+-Wmaybe-uninitialized @gol
 -Wmissing-braces  @gol
 -Wnonnull  @gol
 -Wparentheses  @gol
@@ -3529,8 +3530,15 @@ to compute a value that itself is never 
 computations may be deleted by data flow analysis before the warnings
 are printed.
 
-These warnings are made optional because GCC is not smart
-enough to see all the reasons why the code might be correct
+@item -Wmaybe-uninitialized
+@opindex Wmaybe-uninitialized
+@opindex Wno-maybe-uninitialized
+For an automatic variable, if there exists a path from the function
+entry to a use of the variable that is initialized, but there exist
+some other paths the variable is not initialized, the compiler will
+emit a warning if it can not prove the uninitialized paths do not
+happen at runtime. These warnings are made optional because GCC is 
+not smart enough to see all the reasons why the code might be correct
 despite appearing to have an error.  Here is one example of how
 this can happen:
 
@@ -3553,20 +3561,9 @@ this can happen:
 
 @noindent
 If the value of @code{y} is always 1, 2 or 3, then @code{x} is
-always initialized, but GCC doesn't know this.  Here is
-another common case:
-
-@smallexample
-@{
-  int save_y;
-  if (change_y) save_y = y, y = new_y;
-  @dots{}
-  if (change_y) y = save_y;
-@}
-@end smallexample
-
-@noindent
-This has no bug because @code{save_y} is used only if it is set.
+always initialized, but GCC doesn't know this. To suppress the
+warning, the user needs to provide a default case with assert(0) or
+similar code.
 
 @cindex @code{longjmp} warnings
 This option also warns when a non-volatile automatic variable might be
Index: tree-ssa-uninit.c
===================================================================
--- tree-ssa-uninit.c	(revision 171959)
+++ tree-ssa-uninit.c	(working copy)
@@ -1955,7 +1955,7 @@ warn_uninitialized_phi (gimple phi, VEC(
     return;
 
   uninit_op = gimple_phi_arg_def (phi, MASK_FIRST_SET_BIT (uninit_opnds));
-  warn_uninit (uninit_op,
+  warn_uninit (OPT_Wmaybe_uninitialized, uninit_op,
                "%qD may be used uninitialized in this function",
                uninit_use_stmt);
 
Index: c-family/c-opts.c
===================================================================
--- c-family/c-opts.c	(revision 171959)
+++ c-family/c-opts.c	(working copy)
@@ -379,6 +379,7 @@ c_common_handle_option (size_t scode, co
       warn_unknown_pragmas = value;
 
       warn_uninitialized = value;
+      warn_maybe_uninitialized = value;
 
       if (!c_dialect_cxx ())
 	{
Index: testsuite/gcc.dg/uninit-suppress.c
===================================================================
--- testsuite/gcc.dg/uninit-suppress.c	(revision 0)
+++ testsuite/gcc.dg/uninit-suppress.c	(revision 0)
@@ -0,0 +1,16 @@
+/* { dg-do compile } */
+/* { dg-options "-fno-tree-ccp -O2 -Wuninitialized -Wno-maybe-uninitialized" } */
+void blah();
+int gflag;
+
+void foo()
+{
+   int v;
+   if (gflag)
+     v = 10;
+
+   blah(); /* *gflag may be killed, but compiler won't know */
+
+   if (gflag)
+    bar(v);   /* { dg-bogus "uninitialized" "should be suppressed" } */
+}
Index: testsuite/gcc.dg/uninit-suppress_2.c
===================================================================
--- testsuite/gcc.dg/uninit-suppress_2.c	(revision 0)
+++ testsuite/gcc.dg/uninit-suppress_2.c	(revision 0)
@@ -0,0 +1,16 @@
+/* { dg-do compile } */
+/* { dg-options "-fno-tree-ccp -O2 -Wuninitialized -Werror=uninitialized -Wno-error=maybe-uninitialized" } */
+void blah();
+int gflag;
+
+void foo()
+{
+   int v;
+   if (gflag)
+     v = 10;
+
+   blah(); /* *gflag may be killed, but compiler won't know */
+
+   if (gflag)
+    bar(v);   /* { dg-warning "uninitialized" "should not be promoted to error" } */
+}
Index: opts.c
===================================================================
--- opts.c	(revision 171959)
+++ opts.c	(working copy)
@@ -1680,6 +1680,11 @@ common_handle_option (struct gcc_options
       /* No-op. Used by the driver and passed to us because it starts with f.*/
       break;
 
+    case OPT_Wuninitialized:
+      /* Also turn on maybe uninitialized warning.  */
+      warn_maybe_uninitialized = value;
+      break;
+
     default:
       /* If the flag was handled in a standard way, assume the lack of
 	 processing here is intentional.  */
@@ -1958,6 +1963,9 @@ enable_warning_as_error (const char *arg
       control_warning_option (option_index, (int) kind, value,
 			      loc, lang_mask,
 			      handlers, opts, opts_set, dc);
+      if (option_index == OPT_Wuninitialized)
+        enable_warning_as_error ("maybe-uninitialized", value, lang_mask,
+	                         handlers, opts, opts_set, loc, dc);
     }
   free (new_option);
 }
Index: common.opt
===================================================================
--- common.opt	(revision 171959)
+++ common.opt	(working copy)
@@ -622,6 +622,10 @@ Wuninitialized
 Common Var(warn_uninitialized) Init(-1) Warning
 Warn about uninitialized automatic variables
 
+Wmaybe-uninitialized
+Common Var(warn_maybe_uninitialized) Warning
+Warn about maybe uninitialized automatic variables
+
 Wunreachable-code
 Common Ignore
 Does nothing. Preserved for backward compatibility.
Index: tree-ssa.c
===================================================================
--- tree-ssa.c	(revision 171959)
+++ tree-ssa.c	(working copy)
@@ -1622,10 +1622,11 @@ walk_use_def_chains (tree var, walk_use_
    changed conditionally uninitialized to unconditionally uninitialized.  */
 
 /* Emit a warning for T, an SSA_NAME, being uninitialized.  The exact
-   warning text is in MSGID and LOCUS may contain a location or be null.  */
+   warning text is in MSGID and LOCUS may contain a location or be null.
+   WC is the warning code.  */
 
 void
-warn_uninit (tree t, const char *gmsgid, void *data)
+warn_uninit (enum opt_code wc, tree t, const char *gmsgid, void *data)
 {
   tree var = SSA_NAME_VAR (t);
   gimple context = (gimple) data;
@@ -1649,7 +1650,7 @@ warn_uninit (tree t, const char *gmsgid,
 	     : DECL_SOURCE_LOCATION (var);
   xloc = expand_location (location);
   floc = expand_location (DECL_SOURCE_LOCATION (cfun->decl));
-  if (warning_at (location, OPT_Wuninitialized, gmsgid, var))
+  if (warning_at (location, wc, gmsgid, var))
     {
       TREE_NO_WARNING (var) = 1;
 
@@ -1731,10 +1732,12 @@ warn_uninitialized_var (tree *tp, int *w
       /* We only do data flow with SSA_NAMEs, so that's all we
 	 can warn about.  */
       if (data->always_executed)
-        warn_uninit (t, "%qD is used uninitialized in this function",
+        warn_uninit (OPT_Wuninitialized,
+	             t, "%qD is used uninitialized in this function",
 		     data->stmt);
       else if (data->warn_possibly_uninitialized)
-        warn_uninit (t, "%qD may be used uninitialized in this function",
+        warn_uninit (OPT_Wuninitialized,
+	             t, "%qD may be used uninitialized in this function",
 		     data->stmt);
       *walk_subtrees = 0;
       break;
Index: tree-flow.h
===================================================================
--- tree-flow.h	(revision 171959)
+++ tree-flow.h	(working copy)
@@ -546,7 +546,7 @@ extern void flush_pending_stmts (edge);
 extern void verify_ssa (bool);
 extern void delete_tree_ssa (void);
 extern bool ssa_undefined_value_p (tree);
-extern void warn_uninit (tree, const char *, void *);
+extern void warn_uninit (enum opt_code, tree, const char *, void *);
 extern unsigned int warn_uninitialized_vars (bool);
 extern void execute_update_addresses_taken (void);
 

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: new option -Wno-maybe-uninitialized
  2011-04-08  0:24 new option -Wno-maybe-uninitialized Xinliang David Li
@ 2011-04-21 17:40 ` Jeff Law
  2011-04-21 19:15   ` Xinliang David Li
  0 siblings, 1 reply; 8+ messages in thread
From: Jeff Law @ 2011-04-21 17:40 UTC (permalink / raw)
  To: Xinliang David Li; +Cc: GCC Patches

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 04/07/11 18:24, Xinliang David Li wrote:
> Hi,
> 
> the following patch implements the option to fine control the emitted
> warnings --
> 1) allow suppressing warnings for use of values that may be
> uninitialized.  Definitely uninitialized values that may be used
> warning is not affected
> 2) allow fine grain control on promotion of warnings to errors:
> -Wno-error=maybe-uninitialized
> 
> This is useful for users who only care about definite uninitialized
> variable warnings.
I'm definitely in favor of having the ability for the user to be able to
fine tune the warnings they want.  However, I have some concerns.

First, I'm not sure how reconcile the may vs must inconsistency.  We can
have an object which is used in a real statement in the IL, which we
currently would classify as must-be-uninitialized, even if the path
leading to the statement is unexecutable.    I guess I want to find
better ways to describe this kind of stuff in our documentation and
switch names

Second, I'm concerned about changing the default behaviour of
- -Wuninitialized.  I don't think we ever reached a consensus on that
issue last time we tried to hash through this stuff.  Maybe I missed
something, but it appears to me your patch makes -Wuninitialized only
warn about real uses and ignore uses in PHIs.

Given the lack of consensus (and I believe there will never be a
consensus), I believe we should keep -Wuninitialized behavior as-is and
use -Wno-maybe-uninitialized to override -Wuninitialized.

If we can come up with better switch name than -Wno-maybe-uninitialized,
that would be a good step as well.  However, I'm not offhand sure what
name to use given the issues surrounding may vs must be used uninitialized.

jeff

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.11 (GNU/Linux)
Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org/

iQEcBAEBAgAGBQJNsGecAAoJEBRtltQi2kC7BugH/01X2ppdVUONq2fBzztxF+YR
Qn85yccwI45iizNG1dlmyZlYpATYcfTGwDkBYTjiVllhOJa64Ri6bIP3ErySPllk
OrSAbwFT96TK9/h1eGfVj45mZS3MO0Pa+sxp93noGnxIMWWOt+7LiyrfJEHHLaUK
rPhREXodzLnN0KpXTD8+RS2uzTV+YODAuiWQDkiyQ0XpjCw5w2ccc7dWnrzgvEn5
6x/baTqQxunV8t3/ezUlHiMcUNyMigXccFgIeRRsvMengY/reLxy7eg1i+vdeets
DyWR/Hg3HewLAQidL3SAJLLGuqTrNUZ0wswgqghqh9VllAFoUdKn0xk8GFt0wbk=
=4aA4
-----END PGP SIGNATURE-----

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: new option -Wno-maybe-uninitialized
  2011-04-21 17:40 ` Jeff Law
@ 2011-04-21 19:15   ` Xinliang David Li
  2011-04-22 10:34     ` Richard Guenther
  2011-04-25 22:24     ` Jeff Law
  0 siblings, 2 replies; 8+ messages in thread
From: Xinliang David Li @ 2011-04-21 19:15 UTC (permalink / raw)
  To: Jeff Law; +Cc: GCC Patches

On Thu, Apr 21, 2011 at 10:21 AM, Jeff Law <law@redhat.com> wrote:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
>
> On 04/07/11 18:24, Xinliang David Li wrote:
>> Hi,
>>
>> the following patch implements the option to fine control the emitted
>> warnings --
>> 1) allow suppressing warnings for use of values that may be
>> uninitialized.  Definitely uninitialized values that may be used
>> warning is not affected
>> 2) allow fine grain control on promotion of warnings to errors:
>> -Wno-error=maybe-uninitialized
>>
>> This is useful for users who only care about definite uninitialized
>> variable warnings.
> I'm definitely in favor of having the ability for the user to be able to
> fine tune the warnings they want.  However, I have some concerns.
>
> First, I'm not sure how reconcile the may vs must inconsistency.  We can
> have an object which is used in a real statement in the IL, which we
> currently would classify as must-be-uninitialized, even if the path
> leading to the statement is unexecutable.    I guess I want to find
> better ways to describe this kind of stuff in our documentation and
> switch names

There are three different kind of uninit warnings:

1) definitely uninitialized, and the use the variable dominates the
exit -- this is the must be used uninitialized case
2) definitely uninitialized, but the use may not be executed at runtime;
3) Only initialized in some paths from entry to the use (the use may
or may not be executed).

Currently 2) and 3) uses the same warning message, which may be misleading.

The proposed new option only controls  3).

>
> Second, I'm concerned about changing the default behaviour of
> - -Wuninitialized.  I don't think we ever reached a consensus on that
> issue last time we tried to hash through this stuff.  Maybe I missed
> something, but it appears to me your patch makes -Wuninitialized only
> warn about real uses and ignore uses in PHIs.

No, the patch does not change the default behavior of -Wuninitialized
wihch will turn on -Wmaybe-uninitialized as well. The only useful
scenarios for the new option are:

1) -Wuninitialized -Wno-maybe-uninitialized
2) -Wuninitialized -Werror=uninitialized -Wno-error=maybe-uninitialized

>
> Given the lack of consensus (and I believe there will never be a
> consensus), I believe we should keep -Wuninitialized behavior as-is and
> use -Wno-maybe-uninitialized to override -Wuninitialized.

Yes, that is this patch is supposed to do -- the attached test case is
testing it.

>
> If we can come up with better switch name than -Wno-maybe-uninitialized,
> that would be a good step as well.  However, I'm not offhand sure what
> name to use given the issues surrounding may vs must be used uninitialized.

Maybe-uninitialized matches case 3) -- different from case 2 which is
actually 'maybe-used-uninitialized'.

Thanks,

David

>
> jeff
>
> -----BEGIN PGP SIGNATURE-----
> Version: GnuPG v1.4.11 (GNU/Linux)
> Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org/
>
> iQEcBAEBAgAGBQJNsGecAAoJEBRtltQi2kC7BugH/01X2ppdVUONq2fBzztxF+YR
> Qn85yccwI45iizNG1dlmyZlYpATYcfTGwDkBYTjiVllhOJa64Ri6bIP3ErySPllk
> OrSAbwFT96TK9/h1eGfVj45mZS3MO0Pa+sxp93noGnxIMWWOt+7LiyrfJEHHLaUK
> rPhREXodzLnN0KpXTD8+RS2uzTV+YODAuiWQDkiyQ0XpjCw5w2ccc7dWnrzgvEn5
> 6x/baTqQxunV8t3/ezUlHiMcUNyMigXccFgIeRRsvMengY/reLxy7eg1i+vdeets
> DyWR/Hg3HewLAQidL3SAJLLGuqTrNUZ0wswgqghqh9VllAFoUdKn0xk8GFt0wbk=
> =4aA4
> -----END PGP SIGNATURE-----
>

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: new option -Wno-maybe-uninitialized
  2011-04-21 19:15   ` Xinliang David Li
@ 2011-04-22 10:34     ` Richard Guenther
  2011-04-22 17:11       ` Xinliang David Li
  2011-04-22 17:19       ` Gabriel Dos Reis
  2011-04-25 22:24     ` Jeff Law
  1 sibling, 2 replies; 8+ messages in thread
From: Richard Guenther @ 2011-04-22 10:34 UTC (permalink / raw)
  To: Xinliang David Li; +Cc: Jeff Law, GCC Patches

On Thu, Apr 21, 2011 at 7:43 PM, Xinliang David Li <davidxl@google.com> wrote:
> On Thu, Apr 21, 2011 at 10:21 AM, Jeff Law <law@redhat.com> wrote:
>> -----BEGIN PGP SIGNED MESSAGE-----
>> Hash: SHA1
>>
>> On 04/07/11 18:24, Xinliang David Li wrote:
>>> Hi,
>>>
>>> the following patch implements the option to fine control the emitted
>>> warnings --
>>> 1) allow suppressing warnings for use of values that may be
>>> uninitialized.  Definitely uninitialized values that may be used
>>> warning is not affected
>>> 2) allow fine grain control on promotion of warnings to errors:
>>> -Wno-error=maybe-uninitialized
>>>
>>> This is useful for users who only care about definite uninitialized
>>> variable warnings.
>> I'm definitely in favor of having the ability for the user to be able to
>> fine tune the warnings they want.  However, I have some concerns.
>>
>> First, I'm not sure how reconcile the may vs must inconsistency.  We can
>> have an object which is used in a real statement in the IL, which we
>> currently would classify as must-be-uninitialized, even if the path
>> leading to the statement is unexecutable.    I guess I want to find
>> better ways to describe this kind of stuff in our documentation and
>> switch names
>
> There are three different kind of uninit warnings:
>
> 1) definitely uninitialized, and the use the variable dominates the
> exit -- this is the must be used uninitialized case
> 2) definitely uninitialized, but the use may not be executed at runtime;
> 3) Only initialized in some paths from entry to the use (the use may
> or may not be executed).
>
> Currently 2) and 3) uses the same warning message, which may be misleading.
>
> The proposed new option only controls  3).
>
>>
>> Second, I'm concerned about changing the default behaviour of
>> - -Wuninitialized.  I don't think we ever reached a consensus on that
>> issue last time we tried to hash through this stuff.  Maybe I missed
>> something, but it appears to me your patch makes -Wuninitialized only
>> warn about real uses and ignore uses in PHIs.
>
> No, the patch does not change the default behavior of -Wuninitialized
> wihch will turn on -Wmaybe-uninitialized as well. The only useful
> scenarios for the new option are:
>
> 1) -Wuninitialized -Wno-maybe-uninitialized
> 2) -Wuninitialized -Werror=uninitialized -Wno-error=maybe-uninitialized
>
>>
>> Given the lack of consensus (and I believe there will never be a
>> consensus), I believe we should keep -Wuninitialized behavior as-is and
>> use -Wno-maybe-uninitialized to override -Wuninitialized.
>
> Yes, that is this patch is supposed to do -- the attached test case is
> testing it.
>
>>
>> If we can come up with better switch name than -Wno-maybe-uninitialized,
>> that would be a good step as well.  However, I'm not offhand sure what
>> name to use given the issues surrounding may vs must be used uninitialized.
>
> Maybe-uninitialized matches case 3) -- different from case 2 which is
> actually 'maybe-used-uninitialized'.

Other diagnostics use a level to indicate their noise level.  So, why not
do something like -Wuninitialized=N, with N from 1 to 3, covering all
three cases.  Also note that 1 and 2 are not really different as the function
in question may be never executed, so 1 becomes 2 as well.

Richard.

> Thanks,
>
> David
>
>>
>> jeff
>>
>> -----BEGIN PGP SIGNATURE-----
>> Version: GnuPG v1.4.11 (GNU/Linux)
>> Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org/
>>
>> iQEcBAEBAgAGBQJNsGecAAoJEBRtltQi2kC7BugH/01X2ppdVUONq2fBzztxF+YR
>> Qn85yccwI45iizNG1dlmyZlYpATYcfTGwDkBYTjiVllhOJa64Ri6bIP3ErySPllk
>> OrSAbwFT96TK9/h1eGfVj45mZS3MO0Pa+sxp93noGnxIMWWOt+7LiyrfJEHHLaUK
>> rPhREXodzLnN0KpXTD8+RS2uzTV+YODAuiWQDkiyQ0XpjCw5w2ccc7dWnrzgvEn5
>> 6x/baTqQxunV8t3/ezUlHiMcUNyMigXccFgIeRRsvMengY/reLxy7eg1i+vdeets
>> DyWR/Hg3HewLAQidL3SAJLLGuqTrNUZ0wswgqghqh9VllAFoUdKn0xk8GFt0wbk=
>> =4aA4
>> -----END PGP SIGNATURE-----
>>
>

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: new option -Wno-maybe-uninitialized
  2011-04-22 10:34     ` Richard Guenther
@ 2011-04-22 17:11       ` Xinliang David Li
  2011-04-22 17:19       ` Gabriel Dos Reis
  1 sibling, 0 replies; 8+ messages in thread
From: Xinliang David Li @ 2011-04-22 17:11 UTC (permalink / raw)
  To: Richard Guenther; +Cc: Jeff Law, GCC Patches

Sounds like a good idea, but does -Werror=xxx work for this ?  I tried
cases with -Wstrict-aliasing, -Werror handling seems broken.

Thanks,

David

On Fri, Apr 22, 2011 at 1:52 AM, Richard Guenther
<richard.guenther@gmail.com> wrote:
> On Thu, Apr 21, 2011 at 7:43 PM, Xinliang David Li <davidxl@google.com> wrote:
>> On Thu, Apr 21, 2011 at 10:21 AM, Jeff Law <law@redhat.com> wrote:
>>> -----BEGIN PGP SIGNED MESSAGE-----
>>> Hash: SHA1
>>>
>>> On 04/07/11 18:24, Xinliang David Li wrote:
>>>> Hi,
>>>>
>>>> the following patch implements the option to fine control the emitted
>>>> warnings --
>>>> 1) allow suppressing warnings for use of values that may be
>>>> uninitialized.  Definitely uninitialized values that may be used
>>>> warning is not affected
>>>> 2) allow fine grain control on promotion of warnings to errors:
>>>> -Wno-error=maybe-uninitialized
>>>>
>>>> This is useful for users who only care about definite uninitialized
>>>> variable warnings.
>>> I'm definitely in favor of having the ability for the user to be able to
>>> fine tune the warnings they want.  However, I have some concerns.
>>>
>>> First, I'm not sure how reconcile the may vs must inconsistency.  We can
>>> have an object which is used in a real statement in the IL, which we
>>> currently would classify as must-be-uninitialized, even if the path
>>> leading to the statement is unexecutable.    I guess I want to find
>>> better ways to describe this kind of stuff in our documentation and
>>> switch names
>>
>> There are three different kind of uninit warnings:
>>
>> 1) definitely uninitialized, and the use the variable dominates the
>> exit -- this is the must be used uninitialized case
>> 2) definitely uninitialized, but the use may not be executed at runtime;
>> 3) Only initialized in some paths from entry to the use (the use may
>> or may not be executed).
>>
>> Currently 2) and 3) uses the same warning message, which may be misleading.
>>
>> The proposed new option only controls  3).
>>
>>>
>>> Second, I'm concerned about changing the default behaviour of
>>> - -Wuninitialized.  I don't think we ever reached a consensus on that
>>> issue last time we tried to hash through this stuff.  Maybe I missed
>>> something, but it appears to me your patch makes -Wuninitialized only
>>> warn about real uses and ignore uses in PHIs.
>>
>> No, the patch does not change the default behavior of -Wuninitialized
>> wihch will turn on -Wmaybe-uninitialized as well. The only useful
>> scenarios for the new option are:
>>
>> 1) -Wuninitialized -Wno-maybe-uninitialized
>> 2) -Wuninitialized -Werror=uninitialized -Wno-error=maybe-uninitialized
>>
>>>
>>> Given the lack of consensus (and I believe there will never be a
>>> consensus), I believe we should keep -Wuninitialized behavior as-is and
>>> use -Wno-maybe-uninitialized to override -Wuninitialized.
>>
>> Yes, that is this patch is supposed to do -- the attached test case is
>> testing it.
>>
>>>
>>> If we can come up with better switch name than -Wno-maybe-uninitialized,
>>> that would be a good step as well.  However, I'm not offhand sure what
>>> name to use given the issues surrounding may vs must be used uninitialized.
>>
>> Maybe-uninitialized matches case 3) -- different from case 2 which is
>> actually 'maybe-used-uninitialized'.
>
> Other diagnostics use a level to indicate their noise level.  So, why not
> do something like -Wuninitialized=N, with N from 1 to 3, covering all
> three cases.  Also note that 1 and 2 are not really different as the function
> in question may be never executed, so 1 becomes 2 as well.
>
> Richard.
>
>> Thanks,
>>
>> David
>>
>>>
>>> jeff
>>>
>>> -----BEGIN PGP SIGNATURE-----
>>> Version: GnuPG v1.4.11 (GNU/Linux)
>>> Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org/
>>>
>>> iQEcBAEBAgAGBQJNsGecAAoJEBRtltQi2kC7BugH/01X2ppdVUONq2fBzztxF+YR
>>> Qn85yccwI45iizNG1dlmyZlYpATYcfTGwDkBYTjiVllhOJa64Ri6bIP3ErySPllk
>>> OrSAbwFT96TK9/h1eGfVj45mZS3MO0Pa+sxp93noGnxIMWWOt+7LiyrfJEHHLaUK
>>> rPhREXodzLnN0KpXTD8+RS2uzTV+YODAuiWQDkiyQ0XpjCw5w2ccc7dWnrzgvEn5
>>> 6x/baTqQxunV8t3/ezUlHiMcUNyMigXccFgIeRRsvMengY/reLxy7eg1i+vdeets
>>> DyWR/Hg3HewLAQidL3SAJLLGuqTrNUZ0wswgqghqh9VllAFoUdKn0xk8GFt0wbk=
>>> =4aA4
>>> -----END PGP SIGNATURE-----
>>>
>>
>

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: new option -Wno-maybe-uninitialized
  2011-04-22 10:34     ` Richard Guenther
  2011-04-22 17:11       ` Xinliang David Li
@ 2011-04-22 17:19       ` Gabriel Dos Reis
  2011-04-22 18:12         ` Xinliang David Li
  1 sibling, 1 reply; 8+ messages in thread
From: Gabriel Dos Reis @ 2011-04-22 17:19 UTC (permalink / raw)
  To: Richard Guenther; +Cc: Xinliang David Li, Jeff Law, GCC Patches

On Fri, Apr 22, 2011 at 3:52 AM, Richard Guenther
<richard.guenther@gmail.com> wrote:


>> Maybe-uninitialized matches case 3) -- different from case 2 which is
>> actually 'maybe-used-uninitialized'.
>
> Other diagnostics use a level to indicate their noise level.  So, why not
> do something like -Wuninitialized=N, with N from 1 to 3, covering all
> three cases.

because numbers are good for machines, but terrible as user-interface :-)

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: new option -Wno-maybe-uninitialized
  2011-04-22 17:19       ` Gabriel Dos Reis
@ 2011-04-22 18:12         ` Xinliang David Li
  0 siblings, 0 replies; 8+ messages in thread
From: Xinliang David Li @ 2011-04-22 18:12 UTC (permalink / raw)
  To: Gabriel Dos Reis; +Cc: Richard Guenther, Jeff Law, GCC Patches

Looks like we won't get consensus (as Jeff mentioned) on the naming
etc. Shall I just commit this one?

Thanks,

David

On Fri, Apr 22, 2011 at 9:23 AM, Gabriel Dos Reis
<gdr@integrable-solutions.net> wrote:
> On Fri, Apr 22, 2011 at 3:52 AM, Richard Guenther
> <richard.guenther@gmail.com> wrote:
>
>
>>> Maybe-uninitialized matches case 3) -- different from case 2 which is
>>> actually 'maybe-used-uninitialized'.
>>
>> Other diagnostics use a level to indicate their noise level.  So, why not
>> do something like -Wuninitialized=N, with N from 1 to 3, covering all
>> three cases.
>
> because numbers are good for machines, but terrible as user-interface :-)
>

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: new option -Wno-maybe-uninitialized
  2011-04-21 19:15   ` Xinliang David Li
  2011-04-22 10:34     ` Richard Guenther
@ 2011-04-25 22:24     ` Jeff Law
  1 sibling, 0 replies; 8+ messages in thread
From: Jeff Law @ 2011-04-25 22:24 UTC (permalink / raw)
  To: Xinliang David Li; +Cc: GCC Patches

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 04/21/11 11:43, Xinliang David Li wrote:

> 
> There are three different kind of uninit warnings:
> 
> 1) definitely uninitialized, and the use the variable dominates the
> exit -- this is the must be used uninitialized case
> 2) definitely uninitialized, but the use may not be executed at runtime;
> 3) Only initialized in some paths from entry to the use (the use may
> or may not be executed).
> 
> Currently 2) and 3) uses the same warning message, which may be misleading.
> 
> The proposed new option only controls  3).
It's certainly possible to have an uninitialized use dominating an exit
which can never be executed (think infinite loop which can't be detected
at compile time.

I bring these issues up primarily to get us thinking about how better to
describe the scenarios where we warn in our documentation.  I don't want
to hold up real improvements.


> 
>> No, the patch does not change the default behavior of -Wuninitialized
>> wihch will turn on -Wmaybe-uninitialized as well. The only useful
>> scenarios for the new option are:
OK.  I must have misread something.


Given the fact that your patch isn't changing the -Wuninitialized
behaviour, then let's go ahead with your patch.

Jeff
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.11 (GNU/Linux)
Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org/

iQEcBAEBAgAGBQJNtem+AAoJEBRtltQi2kC7f54IALq8+i2dtRw2z/r6rKYX6+JD
u0eraT4+U5q58ZQQZeU05eBKv4P3Soq14z4/RuuM7dTuU44fqOKz4IrvAxUmnm9f
pdQy4OWHs1/vMR/04JdYy2IjDNbx9tRADioD4VvgrvfBSYHhZvLTJHg5ggAmk2eS
S1Dd1tnKrfn/h0Bz3wqwNDblAQT2TzUvYRcBEq+iTzBQWbrkCCKLr+rWIxEeCDGn
679hkhiFeUYcoFSvYIXc54YCYWCjWsdj5n3Fg9ex7IWGblvfjPejUfq//Xr2wq0J
jlDfuo3mDoDEgLxQ94RRMeHYf7kwCRhwiYqul/U2Fg7UOHlci2aWKpCfHEdY5VI=
=no9V
-----END PGP SIGNATURE-----

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2011-04-25 21:38 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-04-08  0:24 new option -Wno-maybe-uninitialized Xinliang David Li
2011-04-21 17:40 ` Jeff Law
2011-04-21 19:15   ` Xinliang David Li
2011-04-22 10:34     ` Richard Guenther
2011-04-22 17:11       ` Xinliang David Li
2011-04-22 17:19       ` Gabriel Dos Reis
2011-04-22 18:12         ` Xinliang David Li
2011-04-25 22:24     ` Jeff Law

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