On Mon, Oct 15, 2012 at 03:52:01PM +0000, Joseph S. Myers wrote: > 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.
Well to be safest, we should have a prefix for each word if you define more than one flag word. Preferably a name that the user can specify in the .opt file. > 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 can see the MASK/OPTION_MASK thing, but not having TARGET_* means there are lots and lots of code changes. Unfortunately in order to bring the number of changes down to a point where the patches can be reviewed, my previous patches did: #define TARGET_FOO OPTION_FOO #define MASK_FOO OPTION_MASK_FOO > 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. I would like a way to delete the target_flags field if we don't define any flags using it (it would affect the pch stuff that preserves and checks the target_flags). David and I have talked about moving to accessor macros. I'm thinking of something like: mfoo Target Report Mask(FOO) SetFunction ExplicitFunction TargetName If TargetName were defined, it would use TARGET_<xxx> instead of OPTION_<xxx>, but the OPTION_MASK_<xxx> would not be changed. If SetFunction was defined, the opt*.awk files would generate: #define SET_FOO(VALUE) \ do { \ if (VALUE) \ target_flags &= ~MASK_FOO; \ else \ target_flags |= MASK_FOO; \ } while (0) If ExplicitFunction was defined, the opt*.awk files would generate: #define EXPLICIT_FOO(VALUE) \ ((global_options_set.x_target_flags & MASK_FOO) != 0) And then I would change options a few at a time. When I've converted all of the options, I would then go back to adding the Var(yyy) options, but the SET_<xxx> and EXPLICIT_<xxx> would not change (or it could key off of TargetName). How would you feel about SetFunction, ExplicitFunction, and the reduced TargetName? -- Michael Meissner, IBM 5 Technology Place Drive, M/S 2757, Westford, MA 01886-3141, USA meiss...@linux.vnet.ibm.com fax +1 (978) 399-6899