[boost] Re: Patch for the Date-time library

From: Guillaume Melquiond (gmelquio_at_[hidden])
Date: 2003-05-19 00:45:30

On Sun, 18 May 2003, Jeff Garland wrote:

> On Sun, 18 May 2003 16:50:05 +0200 (MEST), Guillaume Melquiond wrote
> > This patch avoids the use of an integer constant equal to 60
> > billions. Since exactly the same method was already used in the test
> > program for 'one_hour_micro' and 'one_hour_nano', I consider the use
> > of 'one_minute_nano' to be a trivial patch (although it doesn't seem
> > so at first). It also cleans the code.
>
> My only concern is that initialization and compile time
> calculation of values that exceed a 32-bit integer has
> marginal portability.

Did you really read the patch I propose?

The original code does:

  check("nano tick calculations",
        nano_res::to_tick_count(0,1,0,0) == 60000000000);

So there is an integer constant that exceeds a 32-bit integer. And as you
said, this kind of code "has marginal portability". It's why I want to
remove it from Boost and replace it by:

  boost::int64_t one_minute_nano = 60*1000*1000;
  one_minute_nano = one_minute_nano*1000;
  check("nano tick calculations",
        nano_res::to_tick_count(0,1,0,0) == one_minute_nano);

So there is no more initialization and computation of values that exceeds
a 32-bit integer. The code is now portable.

> So the question is does this fix
> an error on some compiler or is it just an 'eye of the
> beholder' beautification?

In the original code, there are already these lines:

  boost::int64_t one_hour_micro = 3600*1000;
  one_hour_micro = one_hour_micro*1000; //avoid compiler overflow!
  check("micro tick calculations",
        micro_res::to_tick_count(1,0,0,0) == one_hour_micro);

I did not invent anything: the code was already present to "avoid
compiler overflow" on 'one_hour_micro'. But not on 'one_minute_nano'.

> Bottom line is if it is the later and you really want to
> commit this change I'd like you to take responsibility to
> watch the regression tests after the commit and ensure
> that the results have not changed as a result of the patch.

As I explained in a previous mail, I propose this patch because it
actually fixes a complaint of gcc in the regression tests. So I will
commit this patch myself and I will take responsibility for it.

Regards,

Guillaume