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 jos...@codesourcery.com