Review: Optional features in toolset.flags

From: Vladimir Prus (ghost_at_[hidden])
Date: 2005-09-14 03:23:11

On Thursday 01 September 2005 21:40, Alexey Pakhunov wrote:

> I have one small problem with newly added <architecture>,
> <address-model> and <instruction-set> features. It seems we still need
> to have '<address-model>default', '<architecture>native' and
> '<instruction-set>default' as default values for them.
>
> For Visual Studio 8 the configuration script 'vcvarsall.bat' should be
> called with different parameters depending on the target CPU. So I
> defined something like this:
>
> flags msvc .SETUP <architecture>x86/<address-model>32 : " x86" ;
> flags msvc .SETUP <architecture>x86/<address-model>64 : " x86_amd64" ;
> flags msvc .SETUP <architecture>ia64/<address-model>64 : " x86_ia64" ;
>
> Now I want the first prefix to be used even if <architecture> or
> <address-model> omitted:

Hi Alexey,

The trick used in msvc.jam for that is to define a global variable. Say:

.SETUP = "x86" ;
flags msvc .SETUP <architecture>x86/<address-model>32 : " x86" ;
flags msvc .SETUP <architecture>x86/<address-model>64 : " x86_amd64" ;
flags msvc .SETUP <architecture>ia64/<address-model>64 : " x86_ia64" ;

Now, if neithe condition matches, you get the global value. Will that be 
enough?

However, this solution is somewhat obscure, and depends on Jam language.

So, comments on your patch:

> -# returns true iff all elements of names are valid features.
> +# returns true if all elements of names are valid features.

It's not a typo, I think. "iff" is math means "if and only if" ;-)

> + # cut property values off
> + local prop-keys ;
> + for local i in $(properties)
> + {
> + prop-keys += $(i:G) ;
> + }

What about 

local prop-keys = $(properties:G) ;

?

> + local subset ;
> + for local i in [ feature.split $(s) ]
> + {
> + # Hanlde properties without value (like '<architecture>').
> + # Such properties should be used for matching only if
> + # 'properties' contains such properties

This comment is hard to understand, due to double use of "such" and unclear 
meaning of "used for matching". Can you try to improve?

In fact, I looked on this code for 5 mins already, and can't understand why it 
works ;-)

Also, I suggest that you change the description of the 'flags' rule to mention
this use case. 

- Volodya

-- 
Vladimir Prus
http://vladimir_prus.blogspot.com
Boost.Build V2: http://boost.org/boost-build2