[Ada] Correct time zone in GNAT.Calendar.Time_IO.Value

Message ID 20200706113855.GA135587@adacore.com
State New
Headers show
Series
  • [Ada] Correct time zone in GNAT.Calendar.Time_IO.Value
Related show

Commit Message

Pierre-Marie de Rodat July 6, 2020, 11:38 a.m.
This patch fixes a bug in which if GNAT.Calendar.Time_IO.Value is called
with an ISO date string, the time zone offset is computed incorrectly.

Tested on x86_64-pc-linux-gnu, committed on trunk

gcc/ada/

	* libgnat/g-catiio.adb (Parse_ISO_8601): New name for
	Parse_ISO_8861_UTC.  8601 is the correct ISO standard number.
	Also, "UTC" was confusing. All Time values are represented in
	UTC, but the ISO 8601 date strings include a time zone.

	If a time zone was specified, call
	Ada.Calendar.Formatting.Time_Of instead of
	GNAT.Calendar.Time_Of, because the latter adjusts to the current
	time zone, whereas we want to use (just) the time zone specified
	in the ISO string.  This allows us to pass Time_Zone instead to
	Time_Of, instead of adjusting by Local_Disp by hand.

	If no time zone was specified, call GNAT.Calendar.Time_Of as
	before.

	Use expanded names to clarify which Time_Of is being called.
	Remove redundant comment, and move nonredundant part of the
	commment to the spec.
	(Value): Minor: use "not in" instead of "or else".
	* libgnat/g-catiio.ads: Comment moved here. Correct the ISO
	standard number.
	* libgnat/g-calend.adb: Add ??? comments.
	* libgnat/a-calend.ads, libgnat/a-calend.adb: Update obsolete
	comments regarding the representation of type Time. Move the
	information about the epoch (year 2150) to the spec, and avoid
	uttering "2150" more than once.
	* libgnat/a-catizo.ads (Time_Offset): Add comment.

Patch

diff --git a/gcc/ada/libgnat/a-calend.adb b/gcc/ada/libgnat/a-calend.adb
--- a/gcc/ada/libgnat/a-calend.adb
+++ b/gcc/ada/libgnat/a-calend.adb
@@ -167,9 +167,9 @@  is
    Secs_In_Non_Leap_Year : constant := 365 * Secs_In_Day;
    Nanos_In_Four_Years   : constant := Secs_In_Four_Years * Nano;
 
-   --  Lower and upper bound of Ada time. The zero (0) value of type Time is
-   --  positioned at year 2150. Note that the lower and upper bound account
-   --  for the non-leap centennial years.
+   --  Lower and upper bound of Ada time. Note that the lower and upper bound
+   --  account for the non-leap centennial years. See "Implementation of Time"
+   --  in the spec for what the zero value represents.
 
    Ada_Low  : constant Time_Rep := -(61 * 366 + 188 * 365) * Nanos_In_Day;
    Ada_High : constant Time_Rep :=  (60 * 366 + 190 * 365) * Nanos_In_Day;


diff --git a/gcc/ada/libgnat/a-calend.ads b/gcc/ada/libgnat/a-calend.ads
--- a/gcc/ada/libgnat/a-calend.ads
+++ b/gcc/ada/libgnat/a-calend.ads
@@ -157,16 +157,20 @@  private
    -- Implementation of Time --
    ----------------------------
 
-   --  Time is represented as a signed 64 bit integer count of nanoseconds
-   --  since the start of Ada time (1901-01-01 00:00:00.0 UTC). Time values
-   --  produced by Time_Of are internally normalized to UTC regardless of their
-   --  local time zone. This representation ensures correct handling of leap
-   --  seconds as well as performing arithmetic. In Ada 95, Split and Time_Of
-   --  will treat a time value as being in the local time zone, in Ada 2005,
-   --  Split and Time_Of will treat a time value as being in the designated
-   --  time zone by the formal parameter or in UTC by default. The size of the
-   --  type is large enough to cover the Ada 2005 range of time (1901-01-01
-   --  00:00:00.0 UTC - 2399-12-31-23:59:59.999999999 UTC).
+   --  Time is represented as a signed 64 bit signed integer count of
+   --  nanoseconds since the "epoch" 2150-01-01 00:00:00 UTC. Thus a value of 0
+   --  represents the epoch.  As of this writing, the epoch is in the future,
+   --  so Time values returned by Clock will be negative.
+   --
+   --  Time values produced by Time_Of are internally normalized to UTC
+   --  regardless of their local time zone. This representation ensures correct
+   --  handling of leap seconds as well as performing arithmetic. In Ada 95,
+   --  Split and Time_Of will treat a time value as being in the local time
+   --  zone, in Ada 2005, Split and Time_Of will treat a time value as being in
+   --  the designated time zone by the formal parameter or in UTC by
+   --  default. The size of the type is large enough to cover the Ada
+   --  range of time (1901-01-01T00:00:00.0 UTC - 2399-12-31T23:59:59.999999999
+   --  UTC).
 
    ------------------
    -- Leap Seconds --
@@ -234,8 +238,8 @@  private
 
    function Epoch_Offset return Time_Rep;
    pragma Inline (Epoch_Offset);
-   --  Return the difference between 2150-1-1 UTC and 1970-1-1 UTC expressed in
-   --  nanoseconds. Note that year 2100 is non-leap.
+   --  Return the difference between our epoch and 1970-1-1 UTC (the Unix
+   --  epoch) expressed in nanoseconds. Note that year 2100 is non-leap.
 
    Days_In_Month : constant array (Month_Number) of Day_Number :=
                      (31, 28, 31, 30, 31, 30, 31, 31, 30, 31, 30, 31);


diff --git a/gcc/ada/libgnat/a-catizo.ads b/gcc/ada/libgnat/a-catizo.ads
--- a/gcc/ada/libgnat/a-catizo.ads
+++ b/gcc/ada/libgnat/a-catizo.ads
@@ -21,6 +21,7 @@  package Ada.Calendar.Time_Zones is
    --  Time zone manipulation
 
    type Time_Offset is range -(28 * 60) .. 28 * 60;
+   --  Offset in minutes
 
    Unknown_Zone_Error : exception;
 


diff --git a/gcc/ada/libgnat/g-calend.adb b/gcc/ada/libgnat/g-calend.adb
--- a/gcc/ada/libgnat/g-calend.adb
+++ b/gcc/ada/libgnat/g-calend.adb
@@ -226,7 +226,8 @@  package body GNAT.Calendar is
 
    begin
       --  Even though the input time zone is UTC (0), the flag Use_TZ will
-      --  ensure that Split picks up the local time zone.
+      --  ensure that Split picks up the local time zone. ???But Use_TZ is
+      --  False below, and anyway, Use_TZ has no effect if Time_Zone is 0.
 
       Ada_Calendar_Split
         (Date        => Date,
@@ -315,7 +316,8 @@  package body GNAT.Calendar is
 
    begin
       --  Even though the input time zone is UTC (0), the flag Use_TZ will
-      --  ensure that Split picks up the local time zone.
+      --  ensure that Split picks up the local time zone. ???But there is no
+      --  call to Split here.
 
       return
         Ada_Calendar_Time_Of


diff --git a/gcc/ada/libgnat/g-catiio.adb b/gcc/ada/libgnat/g-catiio.adb
--- a/gcc/ada/libgnat/g-catiio.adb
+++ b/gcc/ada/libgnat/g-catiio.adb
@@ -30,6 +30,7 @@ 
 ------------------------------------------------------------------------------
 
 with Ada.Calendar;            use Ada.Calendar;
+with Ada.Calendar.Time_Zones;
 with Ada.Characters.Handling;
 with Ada.Strings.Unbounded;   use Ada.Strings.Unbounded;
 with Ada.Text_IO;
@@ -93,19 +94,14 @@  package body GNAT.Calendar.Time_IO is
       Length  : Natural := 0) return String;
    --  As above with N provided in Integer format
 
-   procedure Parse_ISO_8861_UTC
+   procedure Parse_ISO_8601
       (Date    : String;
        Time    : out Ada.Calendar.Time;
        Success : out Boolean);
    --  Subsidiary of function Value. It parses the string Date, interpreted as
-   --  an ISO 8861 time representation, and returns corresponding Time value.
-   --  Success is set to False when the string is not a supported ISO 8861
-   --  date. The following regular expression defines the supported format:
-   --
-   --    (yyyymmdd | yyyy'-'mm'-'dd)'T'(hhmmss | hh':'mm':'ss)
-   --      [ ('Z' | ('.' | ',') s{s} | ('+'|'-')hh':'mm) ]
-   --
-   --  Trailing characters (in particular spaces) are not allowed.
+   --  an ISO 8601 time representation, and returns corresponding Time value.
+   --  Success is set to False when the string is not a supported ISO 8601
+   --  date.
    --
    --  Examples:
    --
@@ -565,11 +561,11 @@  package body GNAT.Calendar.Time_IO is
       return Abbrev_Upper_Month_Names'First;
    end Month_Name_To_Number;
 
-   ------------------------
-   -- Parse_ISO_8861_UTC --
-   ------------------------
+   --------------------
+   -- Parse_ISO_8601 --
+   --------------------
 
-   procedure Parse_ISO_8861_UTC
+   procedure Parse_ISO_8601
       (Date    : String;
        Time    : out Ada.Calendar.Time;
        Success : out Boolean)
@@ -813,6 +809,8 @@  package body GNAT.Calendar.Time_IO is
 
       --  Local variables
 
+      use Time_Zones;
+
       Date_Separator : constant Character := '-';
       Hour_Separator : constant Character := ':';
 
@@ -827,7 +825,7 @@  package body GNAT.Calendar.Time_IO is
       Local_Hour   : Hour_Number     := 0;
       Local_Minute : Minute_Number   := 0;
       Local_Sign   : Character       := ' ';
-      Local_Disp   : Duration;
+      Time_Zone    : Time_Offset; -- initialized when Local_Sign is set
 
       Sep_Required : Boolean := False;
       --  True if a separator is seen (and therefore required after it!)
@@ -860,17 +858,18 @@  package body GNAT.Calendar.Time_IO is
 
          if Index <= Date'Last then
 
-            --  Suffix 'Z' just confirms that this is an UTC time. No further
-            --  action needed.
+            --  Suffix 'Z' signifies that this is UTC time (time zone 0)
 
             if Symbol = 'Z' then
+               Local_Sign := '+';
+               Time_Zone := 0;
                Advance;
 
             --  A decimal fraction shall have at least one digit, and has as
             --  many digits as supported by the underlying implementation.
             --  The valid decimal separators are those specified in ISO 31-0,
             --  i.e. the comma [,] or full stop [.]. Of these, the comma is
-            --  the preferred separator of ISO-8861.
+            --  the preferred separator of ISO-8601.
 
             elsif Symbol = ',' or else Symbol = '.' then
                Advance; --  past decimal separator
@@ -898,7 +897,11 @@  package body GNAT.Calendar.Time_IO is
 
                --  Compute local displacement
 
-               Local_Disp := Local_Hour * 3600.0 + Local_Minute * 60.0;
+               Time_Zone := Time_Offset (Local_Hour * 60 + Local_Minute);
+
+               if Local_Sign = '-' then
+                  Time_Zone := -Time_Zone;
+               end if;
             else
                raise Wrong_Syntax;
             end if;
@@ -922,24 +925,17 @@  package body GNAT.Calendar.Time_IO is
          raise Wrong_Syntax;
       end if;
 
-      --  Compute time without local displacement
+      --  If no time zone was specified, we call GNAT.Calendar.Time_Of, which
+      --  uses local time. Otherwise, we use Ada.Calendar.Formatting.Time_Of
+      --  and specify the time zone.
 
       if Local_Sign = ' ' then
-         Time := Time_Of (Year, Month, Day, Hour, Minute, Second, Subsec);
-
-      --  Compute time with positive local displacement
-
-      elsif Local_Sign = '+' then
-         Time :=
-           Time_Of (Year, Month, Day, Hour, Minute, Second, Subsec) -
-             Local_Disp;
-
-      --  Compute time with negative local displacement
-
-      elsif Local_Sign = '-' then
-         Time :=
-           Time_Of (Year, Month, Day, Hour, Minute, Second, Subsec) +
-             Local_Disp;
+         Time := GNAT.Calendar.Time_Of
+           (Year, Month, Day, Hour, Minute, Second, Subsec);
+      else
+         Time := Ada.Calendar.Formatting.Time_Of
+           (Year, Month, Day, Hour, Minute, Second, Subsec,
+            Time_Zone => Time_Zone);
       end if;
 
       --  Notify that the input string was successfully parsed
@@ -953,7 +949,7 @@  package body GNAT.Calendar.Time_IO is
          Time :=
            Time_Of (Year_Number'First, Month_Number'First, Day_Number'First);
          Success := False;
-   end Parse_ISO_8861_UTC;
+   end Parse_ISO_8601;
 
    -----------
    -- Value --
@@ -1174,10 +1170,10 @@  package body GNAT.Calendar.Time_IO is
    --  Start of processing for Value
 
    begin
-      --  Let's try parsing Date as a supported ISO-8861 format. If we do not
+      --  Let's try parsing Date as a supported ISO-8601 format. If we do not
       --  succeed, then retry using all the other GNAT supported formats.
 
-      Parse_ISO_8861_UTC (Date, Time, Success);
+      Parse_ISO_8601 (Date, Time, Success);
 
       if Success then
          return Time;
@@ -1185,15 +1181,7 @@  package body GNAT.Calendar.Time_IO is
 
       --  Length checks
 
-      if D_Length /= 8
-        and then D_Length /= 10
-        and then D_Length /= 11
-        and then D_Length /= 12
-        and then D_Length /= 17
-        and then D_Length /= 19
-        and then D_Length /= 20
-        and then D_Length /= 21
-      then
+      if D_Length not in 8 | 10 | 11 | 12 | 17 | 19 | 20 | 21 then
          raise Constraint_Error;
       end if;
 


diff --git a/gcc/ada/libgnat/g-catiio.ads b/gcc/ada/libgnat/g-catiio.ads
--- a/gcc/ada/libgnat/g-catiio.ads
+++ b/gcc/ada/libgnat/g-catiio.ads
@@ -141,11 +141,14 @@  package GNAT.Calendar.Time_IO is
    --     mmm dd, yyyy         - month spelled out
    --     dd mmm yyyy          - month spelled out
    --
-   --  The following ISO-8861 format expressed as a regular expression is also
+   --  The following ISO-8601 format expressed as a regular expression is also
    --  supported:
    --
    --    (yyyymmdd | yyyy'-'mm'-'dd)'T'(hhmmss | hh':'mm':'ss)
    --      [ ('Z' | ('.' | ',') s{s} | ('+'|'-')hh':'mm) ]
+   --  Trailing characters (including spaces) are not allowed.
+   --  In the ISO case, the current time zone is not used; the time zone
+   --  is as specified in the string, defaulting to UTC.
    --
    --  Examples:
    --