[PATCH] options: Make --help= to emit values post-overrided

Kewen.Lin linkw@linux.ibm.com
Fri Aug 14 05:42:24 GMT 2020

Hi Richard,

Thanks for the comments!

on 2020/8/13 上午12:10, Richard Sandiford wrote:
> "Kewen.Lin" <linkw@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.


	* opts-global.c (decode_options): Call target_option_override_hook
	before it prints for --help=*.

-------------- next part --------------
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.  */

More information about the Gcc-patches mailing list