* [PATCH] enable -fprintf-return-value by default
@ 2016-11-09 3:13 Martin Sebor
2016-11-09 17:05 ` Sandra Loosemore
` (2 more replies)
0 siblings, 3 replies; 10+ messages in thread
From: Martin Sebor @ 2016-11-09 3:13 UTC (permalink / raw)
To: Gcc Patch List
[-- Attachment #1: Type: text/plain, Size: 342 bytes --]
The -fprintf-return-value optimization has been disabled since
the last time it caused a bootstrap failure on powerpc64le. With
the underlying problems fixed GCC has bootstrapped fine on all of
powerpc64, powerpc64le and x86_64 and tested with no regressions.
I'd like to re-enable the option. The attached patch does that.
Thanks
Martin
[-- Attachment #2: gcc-fprintf-return-value.diff --]
[-- Type: text/x-patch, Size: 1282 bytes --]
gcc/c-family/ChangeLog:
* c.opt (-fprintf-return-value): Enable by default.
gcc/ChangeLog:
* doc/invoke.texi (-fprintf-return-value): Document that option
is enabled by default.
diff --git a/gcc/c-family/c.opt b/gcc/c-family/c.opt
index 7d8a726..9c9e83f 100644
--- a/gcc/c-family/c.opt
+++ b/gcc/c-family/c.opt
@@ -1534,7 +1534,7 @@ C++ ObjC++ Var(flag_pretty_templates) Init(1)
-fno-pretty-templates Do not pretty-print template specializations as the template signature followed by the arguments.
fprintf-return-value
-C ObjC C++ ObjC++ LTO Optimization Var(flag_printf_return_value) Init(0)
+C ObjC C++ ObjC++ LTO Optimization Var(flag_printf_return_value) Init(1)
Treat known sprintf return values as constants.
freplace-objc-classes
diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
index 17c5c22..adebeff 100644
--- a/gcc/doc/invoke.texi
+++ b/gcc/doc/invoke.texi
@@ -8301,7 +8301,7 @@ if (snprintf (buf, "%08x", i) >= sizeof buf)
The @option{-fprintf-return-value} option relies on other optimizations
and yields best results with @option{-O2}. It works in tandem with the
@option{-Wformat-length} option. The @option{-fprintf-return-value}
-option is disabled by default.
+option is enabled by default.
@item -fno-peephole
@itemx -fno-peephole2
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] enable -fprintf-return-value by default
2016-11-09 3:13 [PATCH] enable -fprintf-return-value by default Martin Sebor
@ 2016-11-09 17:05 ` Sandra Loosemore
2016-11-16 16:49 ` PING " Martin Sebor
2016-11-18 18:32 ` [PATCH] enable -fprintf-return-value by default Jeff Law
2 siblings, 0 replies; 10+ messages in thread
From: Sandra Loosemore @ 2016-11-09 17:05 UTC (permalink / raw)
To: Martin Sebor, Gcc Patch List
On 11/08/2016 08:13 PM, Martin Sebor wrote:
> The -fprintf-return-value optimization has been disabled since
> the last time it caused a bootstrap failure on powerpc64le. With
> the underlying problems fixed GCC has bootstrapped fine on all of
> powerpc64, powerpc64le and x86_64 and tested with no regressions.
> I'd like to re-enable the option. The attached patch does that.
>
> diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
> index 17c5c22..adebeff 100644
> --- a/gcc/doc/invoke.texi
> +++ b/gcc/doc/invoke.texi
> @@ -8301,7 +8301,7 @@ if (snprintf (buf, "%08x", i) >= sizeof buf)
> The @option{-fprintf-return-value} option relies on other optimizations
> and yields best results with @option{-O2}. It works in tandem with the
> @option{-Wformat-length} option. The @option{-fprintf-return-value}
> -option is disabled by default.
> +option is enabled by default.
>
> @item -fno-peephole
> @itemx -fno-peephole2
Near the beginning of this chapter, in @node Invoking GCC, it says:
Many options have long names starting with @samp{-f} or with
@samp{-W}---for example,
@option{-fmove-loop-invariants}, @option{-Wformat} and so on. Most of
these have both positive and negative forms; the negative form of
@option{-ffoo} is @option{-fno-foo}. This manual documents
only one of these two forms, whichever one is not the default.
So you should be documenting the non-default negative form
-fno-printf-return-value instead of the default positive form. The
corresponding entry in the list in @node Option Summary needs to be
adjusted, too.
-Sandra
^ permalink raw reply [flat|nested] 10+ messages in thread
* PING [PATCH] enable -fprintf-return-value by default
2016-11-09 3:13 [PATCH] enable -fprintf-return-value by default Martin Sebor
2016-11-09 17:05 ` Sandra Loosemore
@ 2016-11-16 16:49 ` Martin Sebor
2016-11-18 5:35 ` Sandra Loosemore
2016-11-18 18:32 ` [PATCH] enable -fprintf-return-value by default Jeff Law
2 siblings, 1 reply; 10+ messages in thread
From: Martin Sebor @ 2016-11-16 16:49 UTC (permalink / raw)
To: Gcc Patch List; +Cc: Sandra Loosemore
[-- Attachment #1: Type: text/plain, Size: 615 bytes --]
I'm looking for an approval of the attached patch.
I've adjusted the documentation based on Sandra's input (i.e.,
documented the negative of the option rather than the positive;
thank you for the review, btw.)
On 11/08/2016 08:13 PM, Martin Sebor wrote:
> The -fprintf-return-value optimization has been disabled since
> the last time it caused a bootstrap failure on powerpc64le. With
> the underlying problems fixed GCC has bootstrapped fine on all of
> powerpc64, powerpc64le and x86_64 and tested with no regressions.
> I'd like to re-enable the option. The attached patch does that.
>
> Thanks
> Martin
>
[-- Attachment #2: gcc-fprintf-return-value.diff --]
[-- Type: text/x-patch, Size: 3577 bytes --]
gcc/c-family/ChangeLog:
* c.opt (-fprintf-return-value): Enable by default.
gcc/ChangeLog:
* doc/invoke.texi (-fprintf-return-value): Document that option
is enabled by default.
Index: gcc/c-family/c.opt
===================================================================
--- gcc/c-family/c.opt (revision 242500)
+++ gcc/c-family/c.opt (working copy)
@@ -1550,7 +1550,7 @@ C++ ObjC++ Var(flag_pretty_templates) Init(1)
-fno-pretty-templates Do not pretty-print template specializations as the template signature followed by the arguments.
fprintf-return-value
-C ObjC C++ ObjC++ LTO Optimization Var(flag_printf_return_value) Init(0)
+C ObjC C++ ObjC++ LTO Optimization Var(flag_printf_return_value) Init(1)
Treat known sprintf return values as constants.
freplace-objc-classes
Index: gcc/doc/invoke.texi
===================================================================
--- gcc/doc/invoke.texi (revision 242500)
+++ gcc/doc/invoke.texi (working copy)
@@ -384,7 +384,7 @@ Objective-C and Objective-C++ Dialects}.
-fno-toplevel-reorder -fno-trapping-math -fno-zero-initialized-in-bss @gol
-fomit-frame-pointer -foptimize-sibling-calls @gol
-fpartial-inlining -fpeel-loops -fpredictive-commoning @gol
--fprefetch-loop-arrays -fprintf-return-value @gol
+-fprefetch-loop-arrays -fno-printf-return-value @gol
-fprofile-correction @gol
-fprofile-use -fprofile-use=@var{path} -fprofile-values @gol
-fprofile-reorder-functions @gol
@@ -8286,18 +8286,19 @@ dependent on the structure of loops within the sou
Disabled at level @option{-Os}.
-@item -fprintf-return-value
-@opindex fprintf-return-value
-Substitute constants for known return value of formatted output functions
-such as @code{sprintf}, @code{snprintf}, @code{vsprintf}, and @code{vsnprintf}
-(but not @code{printf} of @code{fprintf}). This transformation allows GCC
-to optimize or even eliminate branches based on the known return value of
-these functions called with arguments that are either constant, or whose
-values are known to be in a range that makes determining the exact return
-value possible. For example, both the branch and the body of the @code{if}
-statement (but not the call to @code{snprint}) can be optimized away when
-@code{i} is a 32-bit or smaller integer because the return value is guaranteed
-to be at most 8.
+@item -fno-printf-return-value
+@opindex fno-printf-return-value
+Do not substitute constants for known return value of formatted output
+functions such as @code{sprintf}, @code{snprintf}, @code{vsprintf}, and
+@code{vsnprintf} (but not @code{printf} or @code{fprintf}). This
+transformation allows GCC to optimize or even eliminate branches based
+on the known return value of these functions called with arguments that
+are either constant, or whose values are known to be in a range that
+makes determining the exact return value possible. For example, when
+@option{-fprintf-return-value} is in effect, both the branch and the
+body of the @code{if} statement (but not the call to @code{snprintf})
+can be optimized away when @code{i} is a 32-bit or smaller integer
+because the return value is guaranteed to be at most 8.
@smallexample
char buf[9];
@@ -8308,7 +8309,7 @@ if (snprintf (buf, "%08x", i) >= sizeof buf)
The @option{-fprintf-return-value} option relies on other optimizations
and yields best results with @option{-O2}. It works in tandem with the
@option{-Wformat-length} option. The @option{-fprintf-return-value}
-option is disabled by default.
+option is enabled by default.
@item -fno-peephole
@itemx -fno-peephole2
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: PING [PATCH] enable -fprintf-return-value by default
2016-11-16 16:49 ` PING " Martin Sebor
@ 2016-11-18 5:35 ` Sandra Loosemore
2016-11-18 16:01 ` Martin Sebor
0 siblings, 1 reply; 10+ messages in thread
From: Sandra Loosemore @ 2016-11-18 5:35 UTC (permalink / raw)
To: Martin Sebor, Gcc Patch List
On 11/16/2016 09:49 AM, Martin Sebor wrote:
> I'm looking for an approval of the attached patch.
>
> I've adjusted the documentation based on Sandra's input (i.e.,
> documented the negative of the option rather than the positive;
> thank you for the review, btw.)
>
> On 11/08/2016 08:13 PM, Martin Sebor wrote:
>> The -fprintf-return-value optimization has been disabled since
>> the last time it caused a bootstrap failure on powerpc64le. With
>> the underlying problems fixed GCC has bootstrapped fine on all of
>> powerpc64, powerpc64le and x86_64 and tested with no regressions.
>> I'd like to re-enable the option. The attached patch does that.
>
> [snip]
>
> Index: gcc/doc/invoke.texi
> ===================================================================
> --- gcc/doc/invoke.texi (revision 242500)
> +++ gcc/doc/invoke.texi (working copy)
> @@ -384,7 +384,7 @@ Objective-C and Objective-C++ Dialects}.
> -fno-toplevel-reorder -fno-trapping-math -fno-zero-initialized-in-bss @gol
> -fomit-frame-pointer -foptimize-sibling-calls @gol
> -fpartial-inlining -fpeel-loops -fpredictive-commoning @gol
> --fprefetch-loop-arrays -fprintf-return-value @gol
> +-fprefetch-loop-arrays -fno-printf-return-value @gol
> -fprofile-correction @gol
> -fprofile-use -fprofile-use=@var{path} -fprofile-values @gol
> -fprofile-reorder-functions @gol
Please keep this list alphabetized -- the other "-fno-*" options are
sorted as such. The documentation parts of the patch are OK with that
fixed, but I can't approve changing the default for the option.
-Sandra
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: PING [PATCH] enable -fprintf-return-value by default
2016-11-18 5:35 ` Sandra Loosemore
@ 2016-11-18 16:01 ` Martin Sebor
2016-11-18 17:39 ` Sandra Loosemore
0 siblings, 1 reply; 10+ messages in thread
From: Martin Sebor @ 2016-11-18 16:01 UTC (permalink / raw)
To: Sandra Loosemore, Gcc Patch List
On 11/17/2016 10:34 PM, Sandra Loosemore wrote:
> On 11/16/2016 09:49 AM, Martin Sebor wrote:
>> I'm looking for an approval of the attached patch.
>>
>> I've adjusted the documentation based on Sandra's input (i.e.,
>> documented the negative of the option rather than the positive;
>> thank you for the review, btw.)
>>
>> On 11/08/2016 08:13 PM, Martin Sebor wrote:
>>> The -fprintf-return-value optimization has been disabled since
>>> the last time it caused a bootstrap failure on powerpc64le. With
>>> the underlying problems fixed GCC has bootstrapped fine on all of
>>> powerpc64, powerpc64le and x86_64 and tested with no regressions.
>>> I'd like to re-enable the option. The attached patch does that.
>>
>> [snip]
>>
>> Index: gcc/doc/invoke.texi
>> ===================================================================
>> --- gcc/doc/invoke.texi (revision 242500)
>> +++ gcc/doc/invoke.texi (working copy)
>> @@ -384,7 +384,7 @@ Objective-C and Objective-C++ Dialects}.
>> -fno-toplevel-reorder -fno-trapping-math -fno-zero-initialized-in-bss
>> @gol
>> -fomit-frame-pointer -foptimize-sibling-calls @gol
>> -fpartial-inlining -fpeel-loops -fpredictive-commoning @gol
>> --fprefetch-loop-arrays -fprintf-return-value @gol
>> +-fprefetch-loop-arrays -fno-printf-return-value @gol
>> -fprofile-correction @gol
>> -fprofile-use -fprofile-use=@var{path} -fprofile-values @gol
>> -fprofile-reorder-functions @gol
>
> Please keep this list alphabetized -- the other "-fno-*" options are
> sorted as such. The documentation parts of the patch are OK with that
> fixed, but I can't approve changing the default for the option.
I kept the option in the same place on the assumption that it was
the "printf" radix of the name, not the "no-" prefix, that these
were alphabetized by.
But now that you point it out and I've looked more carefully at
some of the other options, I see that in some sections they are
listed strictly alphabetically (-fno-foo after -fmoo) while in
others it's the way you suggest. AFAICS, the former style looks
prevalent in the C++ Language Options and the in Warning Options,
for example. The latter seems to be more popular in the
Optimization Options section (though there are counterexamples).
I'm happy to follow either of these conventions as long as its
consistent. Otherwise, without both kinds of examples to choose
from, I don't think I can trust my brain to remember yet another
rule.
Martin
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: PING [PATCH] enable -fprintf-return-value by default
2016-11-18 16:01 ` Martin Sebor
@ 2016-11-18 17:39 ` Sandra Loosemore
2016-11-18 18:52 ` Martin Sebor
0 siblings, 1 reply; 10+ messages in thread
From: Sandra Loosemore @ 2016-11-18 17:39 UTC (permalink / raw)
To: Martin Sebor, Gcc Patch List
On 11/18/2016 09:01 AM, Martin Sebor wrote:
> On 11/17/2016 10:34 PM, Sandra Loosemore wrote:
>> On 11/16/2016 09:49 AM, Martin Sebor wrote:
>>> I'm looking for an approval of the attached patch.
>>>
>>> I've adjusted the documentation based on Sandra's input (i.e.,
>>> documented the negative of the option rather than the positive;
>>> thank you for the review, btw.)
>>>
>>> On 11/08/2016 08:13 PM, Martin Sebor wrote:
>>>> The -fprintf-return-value optimization has been disabled since
>>>> the last time it caused a bootstrap failure on powerpc64le. With
>>>> the underlying problems fixed GCC has bootstrapped fine on all of
>>>> powerpc64, powerpc64le and x86_64 and tested with no regressions.
>>>> I'd like to re-enable the option. The attached patch does that.
>>>
>>> [snip]
>>>
>>> Index: gcc/doc/invoke.texi
>>> ===================================================================
>>> --- gcc/doc/invoke.texi (revision 242500)
>>> +++ gcc/doc/invoke.texi (working copy)
>>> @@ -384,7 +384,7 @@ Objective-C and Objective-C++ Dialects}.
>>> -fno-toplevel-reorder -fno-trapping-math -fno-zero-initialized-in-bss
>>> @gol
>>> -fomit-frame-pointer -foptimize-sibling-calls @gol
>>> -fpartial-inlining -fpeel-loops -fpredictive-commoning @gol
>>> --fprefetch-loop-arrays -fprintf-return-value @gol
>>> +-fprefetch-loop-arrays -fno-printf-return-value @gol
>>> -fprofile-correction @gol
>>> -fprofile-use -fprofile-use=@var{path} -fprofile-values @gol
>>> -fprofile-reorder-functions @gol
>>
>> Please keep this list alphabetized -- the other "-fno-*" options are
>> sorted as such. The documentation parts of the patch are OK with that
>> fixed, but I can't approve changing the default for the option.
>
> I kept the option in the same place on the assumption that it was
> the "printf" radix of the name, not the "no-" prefix, that these
> were alphabetized by.
>
> But now that you point it out and I've looked more carefully at
> some of the other options, I see that in some sections they are
> listed strictly alphabetically (-fno-foo after -fmoo) while in
> others it's the way you suggest. AFAICS, the former style looks
> prevalent in the C++ Language Options and the in Warning Options,
> for example. The latter seems to be more popular in the
> Optimization Options section (though there are counterexamples).
>
> I'm happy to follow either of these conventions as long as its
> consistent. Otherwise, without both kinds of examples to choose
> from, I don't think I can trust my brain to remember yet another
> rule.
Well, how about the rule is that you look at the convention of the
specific list you are adding something to? At least that way we retain
local consistency and don't mess up those parts of the documentation
that we have already tried to organize in some way. There's so much
material in the command-line options chapter that it would be hard to
figure out how to present it even if the content were completely static,
much less when people are adding/renaming/modifying options all the time.
-Sandra
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] enable -fprintf-return-value by default
2016-11-09 3:13 [PATCH] enable -fprintf-return-value by default Martin Sebor
2016-11-09 17:05 ` Sandra Loosemore
2016-11-16 16:49 ` PING " Martin Sebor
@ 2016-11-18 18:32 ` Jeff Law
2 siblings, 0 replies; 10+ messages in thread
From: Jeff Law @ 2016-11-18 18:32 UTC (permalink / raw)
To: Martin Sebor, Gcc Patch List
On 11/08/2016 08:13 PM, Martin Sebor wrote:
> The -fprintf-return-value optimization has been disabled since
> the last time it caused a bootstrap failure on powerpc64le. With
> the underlying problems fixed GCC has bootstrapped fine on all of
> powerpc64, powerpc64le and x86_64 and tested with no regressions.
> I'd like to re-enable the option. The attached patch does that.
>
> Thanks
> Martin
>
>
> gcc-fprintf-return-value.diff
>
>
> gcc/c-family/ChangeLog:
>
> * c.opt (-fprintf-return-value): Enable by default.
>
> gcc/ChangeLog:
>
> * doc/invoke.texi (-fprintf-return-value): Document that option
> is enabled by default.
OK once you and Sandra are in-sync on the doc changes.
jeff
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: PING [PATCH] enable -fprintf-return-value by default
2016-11-18 17:39 ` Sandra Loosemore
@ 2016-11-18 18:52 ` Martin Sebor
2016-11-18 19:05 ` Jeff Law
2016-11-18 20:08 ` documenting command-line options (was Re: PING [PATCH] enable -fprintf-return-value by default) Sandra Loosemore
0 siblings, 2 replies; 10+ messages in thread
From: Martin Sebor @ 2016-11-18 18:52 UTC (permalink / raw)
To: Sandra Loosemore, Gcc Patch List
On 11/18/2016 10:38 AM, Sandra Loosemore wrote:
> On 11/18/2016 09:01 AM, Martin Sebor wrote:
>> On 11/17/2016 10:34 PM, Sandra Loosemore wrote:
>>> On 11/16/2016 09:49 AM, Martin Sebor wrote:
>>>> I'm looking for an approval of the attached patch.
>>>>
>>>> I've adjusted the documentation based on Sandra's input (i.e.,
>>>> documented the negative of the option rather than the positive;
>>>> thank you for the review, btw.)
>>>>
>>>> On 11/08/2016 08:13 PM, Martin Sebor wrote:
>>>>> The -fprintf-return-value optimization has been disabled since
>>>>> the last time it caused a bootstrap failure on powerpc64le. With
>>>>> the underlying problems fixed GCC has bootstrapped fine on all of
>>>>> powerpc64, powerpc64le and x86_64 and tested with no regressions.
>>>>> I'd like to re-enable the option. The attached patch does that.
>>>>
>>>> [snip]
>>>>
>>>> Index: gcc/doc/invoke.texi
>>>> ===================================================================
>>>> --- gcc/doc/invoke.texi (revision 242500)
>>>> +++ gcc/doc/invoke.texi (working copy)
>>>> @@ -384,7 +384,7 @@ Objective-C and Objective-C++ Dialects}.
>>>> -fno-toplevel-reorder -fno-trapping-math -fno-zero-initialized-in-bss
>>>> @gol
>>>> -fomit-frame-pointer -foptimize-sibling-calls @gol
>>>> -fpartial-inlining -fpeel-loops -fpredictive-commoning @gol
>>>> --fprefetch-loop-arrays -fprintf-return-value @gol
>>>> +-fprefetch-loop-arrays -fno-printf-return-value @gol
>>>> -fprofile-correction @gol
>>>> -fprofile-use -fprofile-use=@var{path} -fprofile-values @gol
>>>> -fprofile-reorder-functions @gol
>>>
>>> Please keep this list alphabetized -- the other "-fno-*" options are
>>> sorted as such. The documentation parts of the patch are OK with that
>>> fixed, but I can't approve changing the default for the option.
>>
>> I kept the option in the same place on the assumption that it was
>> the "printf" radix of the name, not the "no-" prefix, that these
>> were alphabetized by.
>>
>> But now that you point it out and I've looked more carefully at
>> some of the other options, I see that in some sections they are
>> listed strictly alphabetically (-fno-foo after -fmoo) while in
>> others it's the way you suggest. AFAICS, the former style looks
>> prevalent in the C++ Language Options and the in Warning Options,
>> for example. The latter seems to be more popular in the
>> Optimization Options section (though there are counterexamples).
>>
>> I'm happy to follow either of these conventions as long as its
>> consistent. Otherwise, without both kinds of examples to choose
>> from, I don't think I can trust my brain to remember yet another
>> rule.
>
> Well, how about the rule is that you look at the convention of the
> specific list you are adding something to? At least that way we retain
> local consistency and don't mess up those parts of the documentation
> that we have already tried to organize in some way. There's so much
> material in the command-line options chapter that it would be hard to
> figure out how to present it even if the content were completely static,
> much less when people are adding/renaming/modifying options all the time.
I think it would be be ideal if all the options were sorted the same
way in all sections. Is there some command to have texinfo sort them
for us? If not, can we write a script to sort them, either each time
just before generating the docs or manually? (I'm happy to help.)
Otherwise, consistency will continue to be an elusive goal.
There are at least two reasons why I don't think following the style
used by each section is likely to yield good results (and clearly
hasn't to date). First, the big sections already have examples of
both approaches (or simply options out of order). In some of them
it can also be hard to tell if the radix of the options you're
looking to for guidance starts with an 'n'. Second, when adding
more than one option to different sections (such as the
-Wformat-length and -fprintf-format-length options) having to
remember to apply a different sort order for each (warnings are
sorted by radix but optimization options, for the most parts,
strictly alphabetically), seems also likely to trip people up.
Martin
PS I don't mind moving the -fno-printf-return-value option up or
down and I will do it before committing the patch. I would just
prefer to be able not to have to remember and worry about all
these subtle rules. There are too many of them and they tend
to take time and energy away from things that matter more (like
fixing bugs).
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: PING [PATCH] enable -fprintf-return-value by default
2016-11-18 18:52 ` Martin Sebor
@ 2016-11-18 19:05 ` Jeff Law
2016-11-18 20:08 ` documenting command-line options (was Re: PING [PATCH] enable -fprintf-return-value by default) Sandra Loosemore
1 sibling, 0 replies; 10+ messages in thread
From: Jeff Law @ 2016-11-18 19:05 UTC (permalink / raw)
To: Martin Sebor, Sandra Loosemore, Gcc Patch List
On 11/18/2016 11:52 AM, Martin Sebor wrote:
>
> I think it would be be ideal if all the options were sorted the same
> way in all sections. Is there some command to have texinfo sort them
> for us? If not, can we write a script to sort them, either each time
> just before generating the docs or manually? (I'm happy to help.)
> Otherwise, consistency will continue to be an elusive goal.
I'm not aware of texinfo way to do this automatically.
>
> There are at least two reasons why I don't think following the style
> used by each section is likely to yield good results (and clearly
> hasn't to date). First, the big sections already have examples of
> both approaches (or simply options out of order). In some of them
> it can also be hard to tell if the radix of the options you're
> looking to for guidance starts with an 'n'. Second, when adding
> more than one option to different sections (such as the
> -Wformat-length and -fprintf-format-length options) having to
> remember to apply a different sort order for each (warnings are
> sorted by radix but optimization options, for the most parts,
> strictly alphabetically), seems also likely to trip people up.
Let's split this issue off by moving the option into the location Sandra
has asked so that we're at least kindof, sorta, locally consistent.
That allows your patch to go forward.
Then separately we can see if we can bring more sense to the larger
issue. Sandra has tried to work towards bring sanity to our
documentation (which has grown like field bindweed over time) and we can
include a discussion about this issue in that larger effort.
> PS I don't mind moving the -fno-printf-return-value option up or
> down and I will do it before committing the patch. I would just
> prefer to be able not to have to remember and worry about all
> these subtle rules. There are too many of them and they tend
> to take time and energy away from things that matter more (like
> fixing bugs).
Understood. But that's also part of the reason why we delegate things
-- there's a million little things to remember and nobody can remember
them all. So it's a balance between saying "we should clean this up and
bring consistency here now" and "the maintainer has asked for a change,
let's do that and address the consistency issues separately".
There's obviously pros and cons to each decision which I don't enumerate ;-)
Jeff
^ permalink raw reply [flat|nested] 10+ messages in thread
* documenting command-line options (was Re: PING [PATCH] enable -fprintf-return-value by default)
2016-11-18 18:52 ` Martin Sebor
2016-11-18 19:05 ` Jeff Law
@ 2016-11-18 20:08 ` Sandra Loosemore
1 sibling, 0 replies; 10+ messages in thread
From: Sandra Loosemore @ 2016-11-18 20:08 UTC (permalink / raw)
To: Martin Sebor, Gcc Patch List
On 11/18/2016 11:52 AM, Martin Sebor wrote:
>
> [snip]
> I think it would be be ideal if all the options were sorted the same
> way in all sections. Is there some command to have texinfo sort them
> for us? If not, can we write a script to sort them, either each time
> just before generating the docs or manually? (I'm happy to help.)
> Otherwise, consistency will continue to be an elusive goal.
>
> There are at least two reasons why I don't think following the style
> used by each section is likely to yield good results (and clearly
> hasn't to date). First, the big sections already have examples of
> both approaches (or simply options out of order). In some of them
> it can also be hard to tell if the radix of the options you're
> looking to for guidance starts with an 'n'. Second, when adding
> more than one option to different sections (such as the
> -Wformat-length and -fprintf-format-length options) having to
> remember to apply a different sort order for each (warnings are
> sorted by radix but optimization options, for the most parts,
> strictly alphabetically), seems also likely to trip people up.
This is wandering off into a general documentation maintenance
discussion that has little to do with the original patch review request.
GCC has way too many command-line options and most of them are
interesting to only some tiny fraction of users. The documentation
needs to be structured to focus on the options users are most likely to
need or find useful, and not bury the information about the most
important options in the middle of a huge pile of barely-useful
documentation about barely-useful options. For this reason, I think a
strict alphabetical sorting across the board is not helpful. E.g., for
the list of optimization options, the -O options are clearly more
important than the -f options controlling individual optimizations, so
-O should be the first thing users see when they look at that section.
Similarly for debug options, the vast majority of users won't care about
anything other than -g. For target-specific options, it may make sense
to list -march/-mcpu/-mtune together regardless of their alphabetization
with respect to other -m options for that target.
My last round of cleanup on invoke.texi was focused on refining the
categorization of options and moving some option documentation that was
clearly mis-categorized to more appropriate places or newly-created
categories. Having fewer items listed in each category makes the
ordering of things within the group less critical.
I think people looking for documentation for a specific option should be
looking in the index first. We should have @cindex entries for both the
-ffoo and -fno-foo variants so the alphabetization works either way.
Anyway.... long story short.... there's probably no perfect
organization for this chapter, and the sheer number of options being
documented makes it a pile of work even to implement small changes in
convention across the board. That shouldn't stop us from making
incremental improvements in organization, one section at a time, or
taking care not to make the current situation worse when adding
documentation for new options.
-Sandra
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2016-11-18 20:08 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-11-09 3:13 [PATCH] enable -fprintf-return-value by default Martin Sebor
2016-11-09 17:05 ` Sandra Loosemore
2016-11-16 16:49 ` PING " Martin Sebor
2016-11-18 5:35 ` Sandra Loosemore
2016-11-18 16:01 ` Martin Sebor
2016-11-18 17:39 ` Sandra Loosemore
2016-11-18 18:52 ` Martin Sebor
2016-11-18 19:05 ` Jeff Law
2016-11-18 20:08 ` documenting command-line options (was Re: PING [PATCH] enable -fprintf-return-value by default) Sandra Loosemore
2016-11-18 18:32 ` [PATCH] enable -fprintf-return-value by default 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).