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.
It could take quadratic time in alt1... Mostly a theoretical problem I
think, but still.
All options look fine to me, but you need someone else to approve this ;-)
One thing:
> @@ -1664,6 +1665,14 @@ print_specific_help (unsigned int include_flags,
> }
> }
>
> + static bool call_override_once_p = false;
> + if (!call_override_once_p)
> + {
> + gcc_assert (target_option_override_hook);
> + target_option_override_hook ();
> + call_override_once_p = true;
> + }
That assert is pretty random, nothing else using the hook assert it
isn't zero; it immediately and always calls the hook right away, so you
will get a nice ICE with backtrace if it is zero *anyway*.
Cheers,
Segher
