public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [PATCH] Improve -Wmaybe-uninitialized documentation
@ 2017-11-15 15:00 Jonathan Wakely
  2017-11-16  5:32 ` Martin Sebor
  0 siblings, 1 reply; 8+ messages in thread
From: Jonathan Wakely @ 2017-11-15 15:00 UTC (permalink / raw)
  To: gcc-patches

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

The docs for -Wmaybe-uninitialized have some issues:

- That first sentence is looooooong.
- Apparently some C++ programmers think "automatic variable" means one
  declared with C++11 `auto`, rather than simply a local variable.
- The sentence about only warning when optimizing is stuck in between
  two chunks talking about longjmp, which could be inferred to mean
  only the setjmp/longjmp part of the warning depends on optimization.

This attempts to make it easier to parse and understand.

OK for trunk?


[-- Attachment #2: patch.txt --]
[-- Type: text/plain, Size: 3060 bytes --]

commit a923e297acfd7c0ca3d3820463450f38230ab4ea
Author: Jonathan Wakely <jwakely@redhat.com>
Date:   Wed Nov 15 14:25:09 2017 +0000

    Improve -Wmaybe-uninitialized documentation
    
            * doc/invoke.texi (-Wmaybe-uninitialized): Rephrase more accurately.

diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
index 44273284483..fac4122fe3e 100644
--- a/gcc/doc/invoke.texi
+++ b/gcc/doc/invoke.texi
@@ -4970,14 +4970,17 @@ void store (int *i)
 @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 for which the variable is not initialized, the compiler
-emits a warning if it cannot prove the uninitialized paths are not
-executed at run time. These warnings are made optional because GCC is
-not smart enough to see all the reasons why the code might be correct
-in spite of appearing to have an error.  Here is one example of how
-this can happen:
+Warn if there exists a path from entry to a function to a use of an automatic
+(i.e.@ local) variable, for which the variable is not initialized, and the
+compiler cannot prove that the uninitialized path will not be executed at run
+time.
+
+These warnings are only possible in optimizing compilation, because otherwise
+GCC does not keep track of the state of variables.
+
+These warnings are optional because GCC is not smart enough to see all the
+reasons why the code might be correct in spite of appearing to have an error.
+Here is one example of how this can happen:
 
 @smallexample
 @group
@@ -5003,19 +5006,15 @@ warning, you need 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
-changed by a call to @code{longjmp}.  These warnings as well are possible
-only in optimizing compilation.
-
-The compiler sees only the calls to @code{setjmp}.  It cannot know
-where @code{longjmp} will be called; in fact, a signal handler could
-call it at any point in the code.  As a result, you may get a warning
-even when there is in fact no problem because @code{longjmp} cannot
-in fact be called at the place that would cause a problem.
+This option also warns when a non-volatile automatic variable might be changed
+by a call to @code{longjmp}.  The compiler sees only the calls to
+@code{setjmp}.  It cannot know where @code{longjmp} will be called; in fact, a
+signal handler could call it at any point in the code.  As a result, you may
+get a warning even when there is in fact no problem because @code{longjmp}
+cannot in fact be called at the place that would cause a problem.
 
 Some spurious warnings can be avoided if you declare all the functions
-you use that never return as @code{noreturn}.  @xref{Function
-Attributes}.
+you use that never return as @code{noreturn}.  @xref{Function Attributes}.
 
 This warning is enabled by @option{-Wall} or @option{-Wextra}.
 

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

* Re: [PATCH] Improve -Wmaybe-uninitialized documentation
  2017-11-15 15:00 [PATCH] Improve -Wmaybe-uninitialized documentation Jonathan Wakely
@ 2017-11-16  5:32 ` Martin Sebor
  2017-11-16 10:57   ` Jonathan Wakely
  0 siblings, 1 reply; 8+ messages in thread
From: Martin Sebor @ 2017-11-16  5:32 UTC (permalink / raw)
  To: Jonathan Wakely, gcc-patches

On 11/15/2017 07:31 AM, Jonathan Wakely wrote:
> The docs for -Wmaybe-uninitialized have some issues:
>
> - That first sentence is looooooong.
> - Apparently some C++ programmers think "automatic variable" means one
>  declared with C++11 `auto`, rather than simply a local variable.
> - The sentence about only warning when optimizing is stuck in between
>  two chunks talking about longjmp, which could be inferred to mean
>  only the setjmp/longjmp part of the warning depends on optimization.
>
> This attempts to make it easier to parse and understand.

I've always found the description remarkably precise.  Particularly
the bit where it talks about the two paths, one initialized and the
other not.  Your rewording loses that distinction so I don't think
it's as accurate, or even correct.

To use an example, this would satisfy the new description:

   int f (void)
   {
     int i;
     return i;
   }

but it doesn't match GCC behavior (it triggers -Wuninitialized,
not -Wmaybe-uninitialized).  Unless the distinction is more
subtle than I ascribe to it I think it needs to be preserved
in the rewording.

Martin

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

* Re: [PATCH] Improve -Wmaybe-uninitialized documentation
  2017-11-16  5:32 ` Martin Sebor
@ 2017-11-16 10:57   ` Jonathan Wakely
  2017-11-16 16:22     ` Martin Sebor
  2017-11-16 18:12     ` Jeff Law
  0 siblings, 2 replies; 8+ messages in thread
From: Jonathan Wakely @ 2017-11-16 10:57 UTC (permalink / raw)
  To: Martin Sebor; +Cc: gcc-patches

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

On 15/11/17 20:28 -0700, Martin Sebor wrote:
>On 11/15/2017 07:31 AM, Jonathan Wakely wrote:
>>The docs for -Wmaybe-uninitialized have some issues:
>>
>>- That first sentence is looooooong.
>>- Apparently some C++ programmers think "automatic variable" means one
>> declared with C++11 `auto`, rather than simply a local variable.
>>- The sentence about only warning when optimizing is stuck in between
>> two chunks talking about longjmp, which could be inferred to mean
>> only the setjmp/longjmp part of the warning depends on optimization.
>>
>>This attempts to make it easier to parse and understand.
>
>I've always found the description remarkably precise.  Particularly
>the bit where it talks about the two paths, one initialized and the
>other not.  Your rewording loses that distinction so I don't think
>it's as accurate, or even correct.
>
>To use an example, this would satisfy the new description:
>
>  int f (void)
>  {
>    int i;
>    return i;
>  }
>
>but it doesn't match GCC behavior (it triggers -Wuninitialized,
>not -Wmaybe-uninitialized).  Unless the distinction is more
>subtle than I ascribe to it I think it needs to be preserved
>in the rewording.

Ah, I tested a similar case and missed that the warning I got was from
-Wuninitialized not -Wmaybe-uninitialized, which made me think that
"a use of the variable that is initialized" was wrong.

OK, so then here's an alternative patch which doesn't touch that first
sentence except to add "(i.e. local)". That makes the first sentence
even longer, but if it's accurate maybe that's OK. This still adds
"These warnings are only possible in optimizing compilation, because
otherwise GCC does not keep track of the state of variables." And
removes the similar text from the middle of the setjmp/longjmp
discussion.



[-- Attachment #2: patch.txt --]
[-- Type: text/x-patch, Size: 1915 bytes --]

commit 3ebe2a74817bbbbb63e27f961e91e6c044d00245
Author: Jonathan Wakely <jwakely@redhat.com>
Date:   Thu Nov 16 10:43:51 2017 +0000

    Improve -Wmaybe-uninitialized documentation
    
            * doc/invoke.texi (-Wmaybe-uninitialized): Rephrase for clarity.

diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
index 85c980bdfc9..bb68c308166 100644
--- a/gcc/doc/invoke.texi
+++ b/gcc/doc/invoke.texi
@@ -4970,11 +4970,16 @@ void store (int *i)
 @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
+For an automatic (i.e.@ local) 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 for which the variable is not initialized, the compiler
 emits a warning if it cannot prove the uninitialized paths are not
-executed at run time. These warnings are made optional because GCC is
+executed at run time.
+
+These warnings are only possible in optimizing compilation, because otherwise
+GCC does not keep track of the state of variables.
+
+These warnings are made optional because GCC is
 not smart enough to see all the reasons why the code might be correct
 in spite of appearing to have an error.  Here is one example of how
 this can happen:
@@ -5004,9 +5009,7 @@ similar code.
 
 @cindex @code{longjmp} warnings
 This option also warns when a non-volatile automatic variable might be
-changed by a call to @code{longjmp}.  These warnings as well are possible
-only in optimizing compilation.
-
+changed by a call to @code{longjmp}.
 The compiler sees only the calls to @code{setjmp}.  It cannot know
 where @code{longjmp} will be called; in fact, a signal handler could
 call it at any point in the code.  As a result, you may get a warning

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

* Re: [PATCH] Improve -Wmaybe-uninitialized documentation
  2017-11-16 10:57   ` Jonathan Wakely
@ 2017-11-16 16:22     ` Martin Sebor
  2017-11-17 12:56       ` Jonathan Wakely
  2017-11-16 18:12     ` Jeff Law
  1 sibling, 1 reply; 8+ messages in thread
From: Martin Sebor @ 2017-11-16 16:22 UTC (permalink / raw)
  To: Jonathan Wakely; +Cc: gcc-patches

On 11/16/2017 03:49 AM, Jonathan Wakely wrote:
> On 15/11/17 20:28 -0700, Martin Sebor wrote:
>> On 11/15/2017 07:31 AM, Jonathan Wakely wrote:
>>> The docs for -Wmaybe-uninitialized have some issues:
>>>
>>> - That first sentence is looooooong.
>>> - Apparently some C++ programmers think "automatic variable" means one
>>> declared with C++11 `auto`, rather than simply a local variable.
>>> - The sentence about only warning when optimizing is stuck in between
>>> two chunks talking about longjmp, which could be inferred to mean
>>> only the setjmp/longjmp part of the warning depends on optimization.
>>>
>>> This attempts to make it easier to parse and understand.
>>
>> I've always found the description remarkably precise.  Particularly
>> the bit where it talks about the two paths, one initialized and the
>> other not.  Your rewording loses that distinction so I don't think
>> it's as accurate, or even correct.
>>
>> To use an example, this would satisfy the new description:
>>
>>  int f (void)
>>  {
>>    int i;
>>    return i;
>>  }
>>
>> but it doesn't match GCC behavior (it triggers -Wuninitialized,
>> not -Wmaybe-uninitialized).  Unless the distinction is more
>> subtle than I ascribe to it I think it needs to be preserved
>> in the rewording.
>
> Ah, I tested a similar case and missed that the warning I got was from
> -Wuninitialized not -Wmaybe-uninitialized, which made me think that
> "a use of the variable that is initialized" was wrong.
>
> OK, so then here's an alternative patch which doesn't touch that first
> sentence except to add "(i.e. local)". That makes the first sentence
> even longer, but if it's accurate maybe that's OK. This still adds
> "These warnings are only possible in optimizing compilation, because
> otherwise GCC does not keep track of the state of variables." And
> removes the similar text from the middle of the setjmp/longjmp
> discussion.

Thanks, this looks fine to me.

As an aside, I wonder if you think that rewording the part about
GCC not being smart enough might be worthwhile:

  These warnings are made optional because GCC is not smart enough
  to see all the reasons why the code might be correct in spite of
  appearing to have an error.

It sounds just a little pejorative (or maybe just colloquial) to
me for the manual.  Perhaps:

  These warnings are made optional because GCC may not be able to
  determine when the code is correct in spite of appearing to have
  an error.

Martin

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

* Re: [PATCH] Improve -Wmaybe-uninitialized documentation
  2017-11-16 10:57   ` Jonathan Wakely
  2017-11-16 16:22     ` Martin Sebor
@ 2017-11-16 18:12     ` Jeff Law
  2017-11-17 12:44       ` Jonathan Wakely
  1 sibling, 1 reply; 8+ messages in thread
From: Jeff Law @ 2017-11-16 18:12 UTC (permalink / raw)
  To: Jonathan Wakely, Martin Sebor; +Cc: gcc-patches

On 11/16/2017 03:49 AM, Jonathan Wakely wrote:
> On 15/11/17 20:28 -0700, Martin Sebor wrote:
>> On 11/15/2017 07:31 AM, Jonathan Wakely wrote:
>>> The docs for -Wmaybe-uninitialized have some issues:
>>>
>>> - That first sentence is looooooong.
>>> - Apparently some C++ programmers think "automatic variable" means one
>>> declared with C++11 `auto`, rather than simply a local variable.
>>> - The sentence about only warning when optimizing is stuck in between
>>> two chunks talking about longjmp, which could be inferred to mean
>>> only the setjmp/longjmp part of the warning depends on optimization.
>>>
>>> This attempts to make it easier to parse and understand.
>>
>> I've always found the description remarkably precise.  Particularly
>> the bit where it talks about the two paths, one initialized and the
>> other not.  Your rewording loses that distinction so I don't think
>> it's as accurate, or even correct.
>>
>> To use an example, this would satisfy the new description:
>>
>>  int f (void)
>>  {
>>    int i;
>>    return i;
>>  }
>>
>> but it doesn't match GCC behavior (it triggers -Wuninitialized,
>> not -Wmaybe-uninitialized).  Unless the distinction is more
>> subtle than I ascribe to it I think it needs to be preserved
>> in the rewording.
> 
> Ah, I tested a similar case and missed that the warning I got was from
> -Wuninitialized not -Wmaybe-uninitialized, which made me think that
> "a use of the variable that is initialized" was wrong.
> 
> OK, so then here's an alternative patch which doesn't touch that first
> sentence except to add "(i.e. local)". That makes the first sentence
> even longer, but if it's accurate maybe that's OK. This still adds
> "These warnings are only possible in optimizing compilation, because
> otherwise GCC does not keep track of the state of variables." And
> removes the similar text from the middle of the setjmp/longjmp
> discussion.
> 
> 
> 
> patch.txt
> 
> 
> commit 3ebe2a74817bbbbb63e27f961e91e6c044d00245
> Author: Jonathan Wakely <jwakely@redhat.com>
> Date:   Thu Nov 16 10:43:51 2017 +0000
> 
>     Improve -Wmaybe-uninitialized documentation
>     
>             * doc/invoke.texi (-Wmaybe-uninitialized): Rephrase for clarity.
> 
> diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
> index 85c980bdfc9..bb68c308166 100644
> --- a/gcc/doc/invoke.texi
> +++ b/gcc/doc/invoke.texi
> @@ -4970,11 +4970,16 @@ void store (int *i)
>  @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
> +For an automatic (i.e.@ local) variable, if there exists a path from the
> +function entry to a use of the variable that is initialized, but there exist
s/exist/exists/

?

I think with that nit it's ok.

jeff

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

* Re: [PATCH] Improve -Wmaybe-uninitialized documentation
  2017-11-16 18:12     ` Jeff Law
@ 2017-11-17 12:44       ` Jonathan Wakely
  0 siblings, 0 replies; 8+ messages in thread
From: Jonathan Wakely @ 2017-11-17 12:44 UTC (permalink / raw)
  To: Jeff Law; +Cc: Martin Sebor, gcc-patches

On 16/11/17 10:59 -0700, Jeff Law wrote:
>On 11/16/2017 03:49 AM, Jonathan Wakely wrote:
>> On 15/11/17 20:28 -0700, Martin Sebor wrote:
>>> On 11/15/2017 07:31 AM, Jonathan Wakely wrote:
>>>> The docs for -Wmaybe-uninitialized have some issues:
>>>>
>>>> - That first sentence is looooooong.
>>>> - Apparently some C++ programmers think "automatic variable" means one
>>>> declared with C++11 `auto`, rather than simply a local variable.
>>>> - The sentence about only warning when optimizing is stuck in between
>>>> two chunks talking about longjmp, which could be inferred to mean
>>>> only the setjmp/longjmp part of the warning depends on optimization.
>>>>
>>>> This attempts to make it easier to parse and understand.
>>>
>>> I've always found the description remarkably precise.  Particularly
>>> the bit where it talks about the two paths, one initialized and the
>>> other not.  Your rewording loses that distinction so I don't think
>>> it's as accurate, or even correct.
>>>
>>> To use an example, this would satisfy the new description:
>>>
>>>  int f (void)
>>>  {
>>>    int i;
>>>    return i;
>>>  }
>>>
>>> but it doesn't match GCC behavior (it triggers -Wuninitialized,
>>> not -Wmaybe-uninitialized).  Unless the distinction is more
>>> subtle than I ascribe to it I think it needs to be preserved
>>> in the rewording.
>>
>> Ah, I tested a similar case and missed that the warning I got was from
>> -Wuninitialized not -Wmaybe-uninitialized, which made me think that
>> "a use of the variable that is initialized" was wrong.
>>
>> OK, so then here's an alternative patch which doesn't touch that first
>> sentence except to add "(i.e. local)". That makes the first sentence
>> even longer, but if it's accurate maybe that's OK. This still adds
>> "These warnings are only possible in optimizing compilation, because
>> otherwise GCC does not keep track of the state of variables." And
>> removes the similar text from the middle of the setjmp/longjmp
>> discussion.
>>
>>
>>
>> patch.txt
>>
>>
>> commit 3ebe2a74817bbbbb63e27f961e91e6c044d00245
>> Author: Jonathan Wakely <jwakely@redhat.com>
>> Date:   Thu Nov 16 10:43:51 2017 +0000
>>
>>     Improve -Wmaybe-uninitialized documentation
>>
>>             * doc/invoke.texi (-Wmaybe-uninitialized): Rephrase for clarity.
>>
>> diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
>> index 85c980bdfc9..bb68c308166 100644
>> --- a/gcc/doc/invoke.texi
>> +++ b/gcc/doc/invoke.texi
>> @@ -4970,11 +4970,16 @@ void store (int *i)
>>  @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
>> +For an automatic (i.e.@ local) variable, if there exists a path from the
>> +function entry to a use of the variable that is initialized, but there exist
>s/exist/exists/
>
>?

I think it's correct as written, because it says "there exist some
other paths" i.e. paths plural.

I did consider changing it to "there exists some other path" but I
didn't think that was any better or worse, so didn't change it.

>I think with that nit it's ok.
>
>jeff

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

* Re: [PATCH] Improve -Wmaybe-uninitialized documentation
  2017-11-16 16:22     ` Martin Sebor
@ 2017-11-17 12:56       ` Jonathan Wakely
  2017-11-17 17:30         ` Jeff Law
  0 siblings, 1 reply; 8+ messages in thread
From: Jonathan Wakely @ 2017-11-17 12:56 UTC (permalink / raw)
  To: Martin Sebor; +Cc: gcc-patches

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

On 16/11/17 09:18 -0700, Martin Sebor wrote:
>On 11/16/2017 03:49 AM, Jonathan Wakely wrote:
>>On 15/11/17 20:28 -0700, Martin Sebor wrote:
>>>On 11/15/2017 07:31 AM, Jonathan Wakely wrote:
>>>>The docs for -Wmaybe-uninitialized have some issues:
>>>>
>>>>- That first sentence is looooooong.
>>>>- Apparently some C++ programmers think "automatic variable" means one
>>>>declared with C++11 `auto`, rather than simply a local variable.
>>>>- The sentence about only warning when optimizing is stuck in between
>>>>two chunks talking about longjmp, which could be inferred to mean
>>>>only the setjmp/longjmp part of the warning depends on optimization.
>>>>
>>>>This attempts to make it easier to parse and understand.
>>>
>>>I've always found the description remarkably precise.  Particularly
>>>the bit where it talks about the two paths, one initialized and the
>>>other not.  Your rewording loses that distinction so I don't think
>>>it's as accurate, or even correct.
>>>
>>>To use an example, this would satisfy the new description:
>>>
>>> int f (void)
>>> {
>>>   int i;
>>>   return i;
>>> }
>>>
>>>but it doesn't match GCC behavior (it triggers -Wuninitialized,
>>>not -Wmaybe-uninitialized).  Unless the distinction is more
>>>subtle than I ascribe to it I think it needs to be preserved
>>>in the rewording.
>>
>>Ah, I tested a similar case and missed that the warning I got was from
>>-Wuninitialized not -Wmaybe-uninitialized, which made me think that
>>"a use of the variable that is initialized" was wrong.
>>
>>OK, so then here's an alternative patch which doesn't touch that first
>>sentence except to add "(i.e. local)". That makes the first sentence
>>even longer, but if it's accurate maybe that's OK. This still adds
>>"These warnings are only possible in optimizing compilation, because
>>otherwise GCC does not keep track of the state of variables." And
>>removes the similar text from the middle of the setjmp/longjmp
>>discussion.
>
>Thanks, this looks fine to me.
>
>As an aside, I wonder if you think that rewording the part about
>GCC not being smart enough might be worthwhile:
>
> These warnings are made optional because GCC is not smart enough
> to see all the reasons why the code might be correct in spite of
> appearing to have an error.
>
>It sounds just a little pejorative (or maybe just colloquial) to
>me for the manual.  Perhaps:
>
> These warnings are made optional because GCC may not be able to
> determine when the code is correct in spite of appearing to have
> an error.

I think that's an improvement. New patch attached.

Jeff, is this still OK?



[-- Attachment #2: patch.txt --]
[-- Type: text/x-patch, Size: 2078 bytes --]

commit 7e9e4b7e6ab555daa549921ea7f19ff4d1b22129
Author: Jonathan Wakely <jwakely@redhat.com>
Date:   Fri Nov 17 12:40:18 2017 +0000

    Improve -Wmaybe-uninitialized documentation
    
            * doc/invoke.texi (-Wmaybe-uninitialized): Rephrase for clarity.

diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
index 1e2b869885b..e15bf260be5 100644
--- a/gcc/doc/invoke.texi
+++ b/gcc/doc/invoke.texi
@@ -4974,14 +4974,18 @@ void store (int *i)
 @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
+For an automatic (i.e.@ local) 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 for which the variable is not initialized, the compiler
 emits a warning if it cannot prove the uninitialized paths are not
-executed at run time. These warnings are made optional because GCC is
-not smart enough to see all the reasons why the code might be correct
-in spite of appearing to have an error.  Here is one example of how
-this can happen:
+executed at run time.
+
+These warnings are only possible in optimizing compilation, because otherwise
+GCC does not keep track of the state of variables.
+
+These warnings are made optional because GCC may not be able to determine when
+the code is correct in spite of appearing to have an error.  Here is one
+example of how this can happen:
 
 @smallexample
 @group
@@ -5008,9 +5012,7 @@ similar code.
 
 @cindex @code{longjmp} warnings
 This option also warns when a non-volatile automatic variable might be
-changed by a call to @code{longjmp}.  These warnings as well are possible
-only in optimizing compilation.
-
+changed by a call to @code{longjmp}.
 The compiler sees only the calls to @code{setjmp}.  It cannot know
 where @code{longjmp} will be called; in fact, a signal handler could
 call it at any point in the code.  As a result, you may get a warning

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

* Re: [PATCH] Improve -Wmaybe-uninitialized documentation
  2017-11-17 12:56       ` Jonathan Wakely
@ 2017-11-17 17:30         ` Jeff Law
  0 siblings, 0 replies; 8+ messages in thread
From: Jeff Law @ 2017-11-17 17:30 UTC (permalink / raw)
  To: Jonathan Wakely, Martin Sebor; +Cc: gcc-patches

On 11/17/2017 05:40 AM, Jonathan Wakely wrote:
> On 16/11/17 09:18 -0700, Martin Sebor wrote:
>> On 11/16/2017 03:49 AM, Jonathan Wakely wrote:
>>> On 15/11/17 20:28 -0700, Martin Sebor wrote:
>>>> On 11/15/2017 07:31 AM, Jonathan Wakely wrote:
>>>>> The docs for -Wmaybe-uninitialized have some issues:
>>>>>
>>>>> - That first sentence is looooooong.
>>>>> - Apparently some C++ programmers think "automatic variable" means one
>>>>> declared with C++11 `auto`, rather than simply a local variable.
>>>>> - The sentence about only warning when optimizing is stuck in between
>>>>> two chunks talking about longjmp, which could be inferred to mean
>>>>> only the setjmp/longjmp part of the warning depends on optimization.
>>>>>
>>>>> This attempts to make it easier to parse and understand.
>>>>
>>>> I've always found the description remarkably precise.  Particularly
>>>> the bit where it talks about the two paths, one initialized and the
>>>> other not.  Your rewording loses that distinction so I don't think
>>>> it's as accurate, or even correct.
>>>>
>>>> To use an example, this would satisfy the new description:
>>>>
>>>> int f (void)
>>>> {
>>>>   int i;
>>>>   return i;
>>>> }
>>>>
>>>> but it doesn't match GCC behavior (it triggers -Wuninitialized,
>>>> not -Wmaybe-uninitialized).  Unless the distinction is more
>>>> subtle than I ascribe to it I think it needs to be preserved
>>>> in the rewording.
>>>
>>> Ah, I tested a similar case and missed that the warning I got was from
>>> -Wuninitialized not -Wmaybe-uninitialized, which made me think that
>>> "a use of the variable that is initialized" was wrong.
>>>
>>> OK, so then here's an alternative patch which doesn't touch that first
>>> sentence except to add "(i.e. local)". That makes the first sentence
>>> even longer, but if it's accurate maybe that's OK. This still adds
>>> "These warnings are only possible in optimizing compilation, because
>>> otherwise GCC does not keep track of the state of variables." And
>>> removes the similar text from the middle of the setjmp/longjmp
>>> discussion.
>>
>> Thanks, this looks fine to me.
>>
>> As an aside, I wonder if you think that rewording the part about
>> GCC not being smart enough might be worthwhile:
>>
>> These warnings are made optional because GCC is not smart enough
>> to see all the reasons why the code might be correct in spite of
>> appearing to have an error.
>>
>> It sounds just a little pejorative (or maybe just colloquial) to
>> me for the manual.  Perhaps:
>>
>> These warnings are made optional because GCC may not be able to
>> determine when the code is correct in spite of appearing to have
>> an error.
> 
> I think that's an improvement. New patch attached.
> 
> Jeff, is this still OK?
Yes.
jeff

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

end of thread, other threads:[~2017-11-17 17:14 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-11-15 15:00 [PATCH] Improve -Wmaybe-uninitialized documentation Jonathan Wakely
2017-11-16  5:32 ` Martin Sebor
2017-11-16 10:57   ` Jonathan Wakely
2017-11-16 16:22     ` Martin Sebor
2017-11-17 12:56       ` Jonathan Wakely
2017-11-17 17:30         ` Jeff Law
2017-11-16 18:12     ` Jeff Law
2017-11-17 12:44       ` Jonathan Wakely

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