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

From: Ben Artin (macdev_at_[hidden])
Date: 2005-12-04 23:31:53

In article <BAY104-DAV1332E03766A05E111E6847BF4E0_at_phx.gbl>,
 Daryle Walker <darylew_at_[hidden]> wrote:

> 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.

I think that this particular nit is hardly worth the amount of discussion it's 
generated, but I agree with Daryle. If the code says:

((__GNUC__ >= 3 && __GNUC_MINOR__ >=5) || __GNUC__ >= 4))

then changes that only apply to gcc 4 and higher will interfere with the first 
clause; on the other hand, if the code says

((__GNUC__ == 3 && __GNUC_MINOR__ >=5) || __GNUC__ >= 4))

then changes that only apply to gcc 4 and higher will not interfere with the 
first clause. To see what I am trying to say, consider the case where gcc 4.5 
comes out with new syntax and needs to be *excluded* by this #if statement. If 
you take the proposed patch:

((__GNUC__ >= 3 && __GNUC_MINOR__ >=5) || __GNUC__ >= 4))

and naively change it to

((__GNUC__ >= 3 && __GNUC_MINOR__ >=5) || (__GNUC__ >= 4 && __GNUC_MINOR__ < 5))

you would not be making the correct change. You would, of course, soon discover 
your error, and then you would rewrite your condition to 

((__GNUC__ == 3 && __GNUC_MINOR__ >=5) || (__GNUC__ >= 4 && __GNUC_MINOR__ < 5))

which is why Daryle and I believe that the change should be made now to make 
subsequent modifications easier. 

Ben

-- 
I changed my name: <http://periodic-kingdom.org/People/NameChange.php>