[PATCH] Rs6000 infrastructure cleanup (switches), revised patch #4

Joseph S. Myers joseph@codesourcery.com
Mon Oct 15 15:58:00 GMT 2012


On Fri, 12 Oct 2012, Michael Meissner wrote:

> I decided to see if it was possible to simplify the change over by adding
> another flag word in the .opt handling to give the old names (TARGET_<xxx> and
> MASK_<xxx>).  For Joseph Myers and Neil Booth, the issue is when changing all
> of the switches that use Mask(xxx) and InverseMask(xxx) to also use Var(xxx),
> the option machinery changes the names of the macros to OPTION_<xxx> and
> OPTION_MASK_<xxx>, which in turn causes lots and lots of changes for patch
> review.  Some can't be omitted, where we referred to the 'target_flags' and
> 'target_flags_explicit' fields, but at least it reduces the number of other
> changes.

I think changing the names is safer - it's immediately obvious as a build 
failure if you missed anything.  If you have MASK_* names for bits in more 
than one flags variable, there's a risk of accidentally testing a bit in 
the wrong variable, or ORing together bits that belong in different 
variables in a way that can't possibly work, without this causing 
immediately visible problems.  Maybe you're actually only using the names 
for a single variable, but it still seems error-prone for future changes.

I guess TARGET_* names should be safe in a way that MASK_* ones aren't for 
multiple variables - but then I wouldn't have options to do things two 
different ways, but instead use TARGET_* instead of OPTION_* and fix 
existing uses of OPTION_* for such bits.

I don't know if with C++ it's possible to keep the names the same *and* 
ensure that compile time errors occur if bits from different variables are 
used together or a bit is used with the wrong variable *and* avoid any 
other issues occurring as a consequence of such changes.

-- 
Joseph S. Myers
joseph@codesourcery.com



More information about the Gcc-patches mailing list