difftime is pure, not const

Message ID 20240527010029.1336132-1-eggert@cs.ucla.edu
State Committed
Commit df63f01a30d98f74bb6e82cbe1e27dbf795e433d
Delegated to: Adhemerval Zanella Netto
Headers
Series difftime is pure, not const |

Checks

Context Check Description
redhat-pt-bot/TryBot-apply_patch success Patch applied to master at the time it was sent
redhat-pt-bot/TryBot-32bit success Build for i686
linaro-tcwg-bot/tcwg_glibc_build--master-arm success Testing passed
linaro-tcwg-bot/tcwg_glibc_check--master-arm success Testing passed
linaro-tcwg-bot/tcwg_glibc_build--master-aarch64 success Testing passed
linaro-tcwg-bot/tcwg_glibc_check--master-aarch64 success Testing passed

Commit Message

Paul Eggert May 27, 2024, 1 a.m. UTC
  Because difftime's behavior depends on the floating-point environment,
the function is pure, not const (BZ 31802).
---
 time/time.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
  

Comments

Adhemerval Zanella Netto May 27, 2024, 4:03 p.m. UTC | #1
On 26/05/24 22:00, Paul Eggert wrote:
> Because difftime's behavior depends on the floating-point environment,
> the function is pure, not const (BZ 31802).

LGTM, thanks.

Reviewed-by: Adhemerval Zanella  <adhemerval.zanella@linaro.org>

> ---
>  time/time.h | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/time/time.h b/time/time.h
> index bc043188e1..486367f7dd 100644
> --- a/time/time.h
> +++ b/time/time.h
> @@ -77,7 +77,7 @@ extern time_t time (time_t *__timer) __THROW;
>  
>  /* Return the difference between TIME1 and TIME0.  */
>  extern double difftime (time_t __time1, time_t __time0)
> -     __THROW __attribute__ ((__const__));
> +     __THROW __attribute_pure__;
>  
>  /* Return the `time_t' representation of TP and normalize TP.  */
>  extern time_t mktime (struct tm *__tp) __THROW;
> @@ -85,7 +85,7 @@ extern time_t mktime (struct tm *__tp) __THROW;
>  # ifdef __REDIRECT_NTH
>  extern time_t __REDIRECT_NTH (time, (time_t *__timer), __time64);
>  extern double __REDIRECT_NTH (difftime, (time_t __time1, time_t __time0),
> -                              __difftime64) __attribute__ ((__const__));
> +                              __difftime64) __attribute_pure__;
>  extern time_t __REDIRECT_NTH (mktime, (struct tm *__tp), __mktime64);
>  # else
>  #  define time __time64
  
Andreas Schwab May 27, 2024, 8:17 p.m. UTC | #2
On Mai 26 2024, Paul Eggert wrote:

> Because difftime's behavior depends on the floating-point environment,

Please explain.  Where does it do that?
  
Vincent Lefevre May 27, 2024, 9:09 p.m. UTC | #3
On 2024-05-27 22:17:11 +0200, Andreas Schwab wrote:
> On Mai 26 2024, Paul Eggert wrote:
> 
> > Because difftime's behavior depends on the floating-point environment,
> 
> Please explain.  Where does it do that?

Because time_t is typically a 64-bit type and double typically
has a 53-bit significand, so that the result needs to be rounded
in some unspecified way[*]?

[*] According to the current rounding mode or always to nearest?
May it generate an "inexact" exception as a consequence of rounding?
  
Andreas Schwab May 27, 2024, 10:25 p.m. UTC | #4
On Mai 27 2024, Vincent Lefevre wrote:

> On 2024-05-27 22:17:11 +0200, Andreas Schwab wrote:
>> On Mai 26 2024, Paul Eggert wrote:
>> 
>> > Because difftime's behavior depends on the floating-point environment,
>> 
>> Please explain.  Where does it do that?
>
> Because time_t is typically a 64-bit type and double typically
> has a 53-bit significand, so that the result needs to be rounded
> in some unspecified way[*]?

That can never happen for existing timestamps.
  
Gabriel Ravier May 27, 2024, 10:42 p.m. UTC | #5
On 5/28/24 12:25 AM, Andreas Schwab wrote:
> On Mai 27 2024, Vincent Lefevre wrote:
>
>> On 2024-05-27 22:17:11 +0200, Andreas Schwab wrote:
>>> On Mai 26 2024, Paul Eggert wrote:
>>>
>>>> Because difftime's behavior depends on the floating-point environment,
>>> Please explain.  Where does it do that?
>> Because time_t is typically a 64-bit type and double typically
>> has a 53-bit significand, so that the result needs to be rounded
>> in some unspecified way[*]?
> That can never happen for existing timestamps.
>
It's illegal for someone to create a timestamp that's e.g. 300 million 
years in the future ? It might be a reasonable to say that current 
versions of glibc are unlikely to still be in use by then, but I'm 
unaware of anything stopping someone from simply creating such a 
timestamp right now (I've tested it on my laptop and `date` is fine with 
a timestamp that's more than 2 billion years into the future).
  
Vincent Lefevre May 27, 2024, 11:01 p.m. UTC | #6
On 2024-05-28 00:25:01 +0200, Andreas Schwab wrote:
> On Mai 27 2024, Vincent Lefevre wrote:
> 
> > On 2024-05-27 22:17:11 +0200, Andreas Schwab wrote:
> >> On Mai 26 2024, Paul Eggert wrote:
> >> 
> >> > Because difftime's behavior depends on the floating-point environment,
> >> 
> >> Please explain.  Where does it do that?
> >
> > Because time_t is typically a 64-bit type and double typically
> > has a 53-bit significand, so that the result needs to be rounded
> > in some unspecified way[*]?
> 
> That can never happen for existing timestamps.

What do you mean by "existing timestamps"?

Actually, what matters is the supported range for time_t (which
may be smaller than the full range of the type), but this is not
documented, while the ISO C17 standard says

  The range and precision of times representable in clock_t and time_t
  are implementation-defined.

I've reported a bug:

  https://sourceware.org/bugzilla/show_bug.cgi?id=31808
  
Vincent Lefevre May 27, 2024, 11:07 p.m. UTC | #7
On 2024-05-28 00:42:14 +0200, Gabriel Ravier wrote:
> It's illegal for someone to create a timestamp that's e.g. 300 million years
> in the future ? It might be a reasonable to say that current versions of
> glibc are unlikely to still be in use by then, but I'm unaware of anything
> stopping someone from simply creating such a timestamp right now (I've
> tested it on my laptop and `date` is fine with a timestamp that's more than
> 2 billion years into the future).

Note that time_t may have other uses than timestamps. For instance,
it may be used for date calculations, in which case the support of
distant dates in the future and in the past may be useful.
  
Andreas Schwab May 28, 2024, 7:37 a.m. UTC | #8
On Mai 28 2024, Vincent Lefevre wrote:

> documented, while the ISO C17 standard says
>
>   The range and precision of times representable in clock_t and time_t
>   are implementation-defined.

The C standard is irrelevant here, we are talking about glibc, which
sets certain minimum standards.
  
Andreas Schwab May 28, 2024, 7:39 a.m. UTC | #9
On Mai 28 2024, Gabriel Ravier wrote:

> On 5/28/24 12:25 AM, Andreas Schwab wrote:
>> On Mai 27 2024, Vincent Lefevre wrote:
>>
>>> On 2024-05-27 22:17:11 +0200, Andreas Schwab wrote:
>>>> On Mai 26 2024, Paul Eggert wrote:
>>>>
>>>>> Because difftime's behavior depends on the floating-point environment,
>>>> Please explain.  Where does it do that?
>>> Because time_t is typically a 64-bit type and double typically
>>> has a 53-bit significand, so that the result needs to be rounded
>>> in some unspecified way[*]?
>> That can never happen for existing timestamps.
>>
> It's illegal for someone to create a timestamp that's e.g. 300 million
> years in the future ? It might be a reasonable to say that current
> versions of glibc are unlikely to still be in use by then, but I'm unaware
> of anything stopping someone from simply creating such a timestamp right
> now (I've tested it on my laptop and `date` is fine with a timestamp
> that's more than 2 billion years into the future).

If the precision of double is not enough to represent the difference
exactly, the return value of difftime becomes meaningless.
  
Vincent Lefevre May 28, 2024, 8:16 a.m. UTC | #10
On 2024-05-28 09:39:46 +0200, Andreas Schwab wrote:
> On Mai 28 2024, Gabriel Ravier wrote:
> 
> > On 5/28/24 12:25 AM, Andreas Schwab wrote:
> >> On Mai 27 2024, Vincent Lefevre wrote:
> >>
> >>> On 2024-05-27 22:17:11 +0200, Andreas Schwab wrote:
> >>>> On Mai 26 2024, Paul Eggert wrote:
> >>>>
> >>>>> Because difftime's behavior depends on the floating-point environment,
> >>>> Please explain.  Where does it do that?
> >>> Because time_t is typically a 64-bit type and double typically
> >>> has a 53-bit significand, so that the result needs to be rounded
> >>> in some unspecified way[*]?
> >> That can never happen for existing timestamps.
> >>
> > It's illegal for someone to create a timestamp that's e.g. 300 million
> > years in the future ? It might be a reasonable to say that current
> > versions of glibc are unlikely to still be in use by then, but I'm unaware
> > of anything stopping someone from simply creating such a timestamp right
> > now (I've tested it on my laptop and `date` is fine with a timestamp
> > that's more than 2 billion years into the future).
> 
> If the precision of double is not enough to represent the difference
> exactly, the return value of difftime becomes meaningless.

Not necessarily meaningless. An approximate number of seconds
between 2 distant dates may be acceptable for the user, though
a plain subtraction is probably better in general.
  

Patch

diff --git a/time/time.h b/time/time.h
index bc043188e1..486367f7dd 100644
--- a/time/time.h
+++ b/time/time.h
@@ -77,7 +77,7 @@  extern time_t time (time_t *__timer) __THROW;
 
 /* Return the difference between TIME1 and TIME0.  */
 extern double difftime (time_t __time1, time_t __time0)
-     __THROW __attribute__ ((__const__));
+     __THROW __attribute_pure__;
 
 /* Return the `time_t' representation of TP and normalize TP.  */
 extern time_t mktime (struct tm *__tp) __THROW;
@@ -85,7 +85,7 @@  extern time_t mktime (struct tm *__tp) __THROW;
 # ifdef __REDIRECT_NTH
 extern time_t __REDIRECT_NTH (time, (time_t *__timer), __time64);
 extern double __REDIRECT_NTH (difftime, (time_t __time1, time_t __time0),
-                              __difftime64) __attribute__ ((__const__));
+                              __difftime64) __attribute_pure__;
 extern time_t __REDIRECT_NTH (mktime, (struct tm *__tp), __mktime64);
 # else
 #  define time __time64