Hi Richard,
Thanks for the comments!
on 2020/8/13 上午12:10, Richard Sandiford wrote:
> "Kewen.Lin" <li...@linux.ibm.com> writes:
>> Hi Segher,
>>
>> on 2020/8/7 锟斤拷锟斤拷10:42, Segher Boessenkool wrote:
>>> Hi!
>>>
>>> On Fri, Aug 07, 2020 at 10:44:10AM +0800, Kewen.Lin wrote:
>>>>> I think this makes a lot of sense.
>>>>>
>>>>>> btw, not sure whether it's a good idea to move
>>>>>> target_option_override_hook
>>>>>> call into print_specific_help and use one function local static
>>>>>> variable to control it's called once for all kinds of help dumping
>>>>>> (possible combination), then can remove the calls in function
>>>>>> common_handle_option.
>>>>>
>>>>> I cannot easily imagine what that will look like... it could easily be
>>>>> worse than what you have here (callbacks aren't so nice, but there are
>>>>> worse things).
>>>>>
>>>>
>>>> I attached opts_alt2.diff to be more specific for this, both alt1 and alt2
>>>> follow the existing callback scheme, alt2 aims to avoid possible multiple
>>>> times target_option_override_hook calls when we have several --help= or
>>>> similar, but I guess alt1 is also fine since the hook should be allowed to
>>>> be called more than once.
>
> Yeah. I guess ideally (and independently of this patch) we'd have a
> flag_checking assert that override_options is idempotent, but that
> might be tricky to implement.
>
> It looks like there's a subtle (pre-existing) difference in what --help
> and --help= do. --help already calls target_option_override_hook,
> but does it at the point that the option occurs. --help= instead
> queues the help until we've finished processing other arguments,
> and would therefore take later options into account.
>
Yes, it is.
> I don't know that one is obviously better than the other though.
>
>> […]
>> *opts_alt1.diff*
>>
>> gcc/ChangeLog:
>>
>> * opts-global.c (decode_options): Adjust call to print_help.
>> * opts.c (print_help): Add one function point argument
>> target_option_override_hook and call it before print_specific_help.
>> * opts.h (print_help): Add one more argument to function declare.
>
> I think personally I'd prefer an option (3): call
> target_option_override_hook directly in decode_options,
> if help_option_arguments is nonempty. Like you say,
> decode_options appears to be the only caller of print_help.
>
Good idea! The related patch is attached, different from opts_alt{1,2}
it could still call target_option_override_hook even if we won't call
print_specific_help eventually for some special cases like lang_mask is
CL_DRIVER or include_flags is empty. But I think it's fine.
Also bootstrapped/regtested on powerpc64le-linux-gnu P8.
BR,
Kewen
-----
gcc/ChangeLog:
* opts-global.c (decode_options): Call target_option_override_hook
before it prints for --help=*.
diff --git a/gcc/opts-global.c b/gcc/opts-global.c
index b1a8429dc3c..fc332871cb8 100644
--- a/gcc/opts-global.c
+++ b/gcc/opts-global.c
@@ -327,8 +327,14 @@ decode_options (struct gcc_options *opts, struct
gcc_options *opts_set,
unsigned i;
const char *arg;
- FOR_EACH_VEC_ELT (help_option_arguments, i, arg)
- print_help (opts, lang_mask, arg);
+ if (!help_option_arguments.is_empty ())
+ {
+ /* Consider post-overrided values for --help=*. */
+ target_option_override_hook ();
+
+ FOR_EACH_VEC_ELT (help_option_arguments, i, arg)
+ print_help (opts, lang_mask, arg);
+ }
}
/* Hold command-line options associated with stack limitation. */
