On Mon, 15 Oct 2012, Michael Meissner wrote: > > 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.
Yes, for MASK_*. > > 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 The first of those #defines might be an intermediate step towards actually replacing OPTION_FOO by TARGET_FOO everywhere (since there seems to be no actual need for the different naming convention there, only for the masks). But I don't really think we should delay the mechanical replacement much (changing all OPTION_* that aren't OPTION_MASK_* to be TARGET_* should be a straightforward change to make automatically, although a pain to review the results of that substitution so maybe best kept in a separate patch from one doing anything more substantive). That is: 1. Patch adding TARGET_FOO aliases for OPTION_FOO (small change to the awk scripts and associated documentation, I expect). 2. Large, mechanical, automatically generated patch to change existing OPTION_FOO users (or maybe one such patch per target). 3. Patch removing the OPTION_FOO name (small change to awk scripts and documentation). Then you've eliminated one unnecessary cause of changes when moving bits out of target_flags. > If TargetName were defined, it would use TARGET_<xxx> instead of OPTION_<xxx>, > but the OPTION_MASK_<xxx> would not be changed. Not needed, given the above sequence of changes. > 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) I'd like any such new macros to take an argument that's the pointer to the relevant options structure (global_options, global_options_set). If the place where the macro is called has a pointer available, then it can be passed in, otherwise pass in &global_options or &global_options_set unless and until such a pointer becomes available in the relevant place. > How would you feel about SetFunction, ExplicitFunction, and the reduced > TargetName? The principle of having macros for setting flags or testing if they are explicitly set is fine, though it's not clear to me that they need any such special settings as SetFunction and ExplicitFunction (rather than being generated unconditionally). I'd quite like the macros such as target_flags that refer to global options to end up not being lvalues at all. That helps ensure that option settings are only modified in limited places that have options pointers. It would be nice eventually for such things as "optimize" and "target" attributes to be able to swap options structures, and to work closer to how options on the command line are processed - for that, you want careful control on what places actually modify options at all. -- Joseph S. Myers jos...@codesourcery.com