Re: [boost] [boost.python] Wrong version check?

From: Daryle Walker (darylew_at_[hidden])
Date: 2005-12-04 15:18:26

On 12/2/05 12:55 PM, "David Abrahams" <dave_at_[hidden]> wrote:

> Daryle Walker <darylew_at_[hidden]> writes:
> 
>> On 11/30/05 2:07 PM, "David Abrahams" <dave_at_[hidden]> wrote:
>> 
>>> Daryle Walker <darylew_at_[hidden]> writes:
>>> 
>>>> On 11/29/05 4:27 PM, "Jason Kankiewicz" <jkankiewicz_at_[hidden]> wrote:
>>>> 
>>>>> +# elif (defined(__GNUC__) && ((__GNUC__ >= 3 && __GNUC_MINOR__ >=5) ||
>>>>> __GNUC__ >= 4))
>>>> 
>>>> Shouldn't the check for GCC 3.x be:
>>>> 
>>>>     __GNUC__ == 3 && __GNUC_MINOR__ >= 5
>>>> 
>>>> instead of the currently over-broad version?  Otherwise version numbers
>>>> like 4.6 or 5.9 would match on that phrase, which is wrong.  They should
>>>> match on the "__GNUC__ >= 4" phrase instead.
>>> 
>>> Well, that would certainly be clearer, but it is semantically equivalent, is
>>> it not?
>> 
>> Sort of; it happens to be equivalent for the 3.x series.  The phrase implies
>> it's applicable to any higher series (4.x, 5.x, etc.),
> 
> Isn't it?

It shouldn't apply to higher series; it should only be a fix for the 3.x
series.  As written, it's an erroneous over-generalization.

>> but that's wrong because it fails numbers like 4.2.
> 
> What fails numbers like 4.2?

The "__GNUC__ >= 3 && __GNUC_MINOR__ >=5" will match numbers from the 4.x
series only if the "x" is at least 5.  So numbers like 4.2 will mistakenly
fail.  You should only use this phrase if you really mean "[3.5, 3.infinity)
|| [4.5, 4.infinity) || [5.5, 5.infinity) || ...".  But we really mean only
[3.5, 3.infinity) and we should state that directly.

>> The "__GNUC__ >= 4" phrase saves you
>> from that.  
> 
> So, as I was saying, it's semantically equivalent.  I was trying to
> get an idea how urgent it was to make a change, so close to the
> release date.

It's not a fatal error, because the [4.0, infinity) check covers up the
sloppiness of the GCC 3.x check.  But we shouldn't excuse the sloppiness.
And we should educate now, so someone else doesn't make a more insidious
version of this error later.  However, for this case, you can change it at
your convenience.

>> The next similar error may be more insidious.
> 
> It's a nonissue, fortunately, since the code in question does not
> appear anywhere in CVS.

-- 
Daryle Walker
Mac, Internet, and Video Game Junkie
darylew AT hotmail DOT com