Re: [PATCH 0/2] Make option-lookup macros explicit

[Jeff Law <law@redhat.com>]

On 05/09/14 15:14, David Malcolm wrote:
> GCC's code is full of references to options like:
>
>    static bool
>    gate_vrp (void)
>    {
>      return flag_tree_vrp != 0;
>    }
>
> where "flag_tree_vrp" is actually an autogenerated macro to:
>
>    global_options.x_flag_tree_vrp
>
> This is deeply confusing to a newbie (and indeed still to me after ~two
> years of working with GCC's internals) e.g. when stepping through code and
> trying to query the value in gdb - what is an actual variable, and what is
> an option?  Why isn't tab-completion working?  etc
>
> The idea of the following patch series is to replace the above with:
>
>    static bool
>    gate_vrp (void)
>    {
>      return GCC_OPTION (flag_tree_vrp) != 0;
>    }
>
> thus making it obvious when macro magic is occurring.
Conceptually this patch seems fine to me.  I think the macro magic 
around flag_xxx is annoying, so this is definitely an improvement.

>
>  From a global-state-removal perspective, it might be nice to associate
> options with a gcc::context, rather than have a single instance of options,
> though that isn't addressed in these patches.
> (e.g. perhaps explicitly adding a gcc::context arg to the macro???)
Should be done as a follow-up IMHO.

>
> The patches were successfully bootstrapped&regrtested on top
> of r208714 (rather old, 2014-03-20) albeit just on x86_64-unknown-linux-gnu
> (Fedora 20), with
>    --enable-languages=c,c++,objc,obj-c++,java,fortran,ada,go,lto
> (i.e. every frontend, I think).
>
> OK for trunk, after 4.9.1 is released?  (clearly I need to test on
> more targets first, given how much config/* code this touches, but I
> wanted to sound the idea out on this list).
Yea, definitely test out some other targets and wait for the 4.9.1 
release.  But I'm inclined to approve the patch as it stands plus any 
trivial changes that have to be made due to 4.9.0->4.9.1 changes in 
nearby code.

jeff