Subject: bug#55635: closed (Re: bug#55635: `make-decoded-time' incorrectly sets DST to nil, it should be -1 (guess))

From: [email-masked] (GNU bug Tracking System)
Date: Fri, 27 May 2022 19:28:03 +0000

Your bug report

#55635: `make-decoded-time' incorrectly sets DST to nil, it should be -1 (guess)

which was filed against the emacs package, has been closed.

The explanation is attached below, along with your original report.
If you require more details, please reply to [email-masked].

-- 
55635: https://debbugs.gnu.org/cgi/bugreport.cgi?bug=55635
GNU Bug Tracking System
Contact [email-masked] with problems

Subject: Re: bug#55635: `make-decoded-time' incorrectly sets DST to nil, it should be -1 (guess)
From: Paul Eggert <[email-masked]>
To: Lars Ingebrigtsen <[email-masked]>
Cc: [email-masked], Maxim Nikulin <[email-masked]>
Date: Fri, 27 May 2022 12:26:56 -0700
List-Id: <debbugs-submit.debbugs.gnu.org>

On 5/27/22 03:40, Lars Ingebrigtsen wrote:
>> This looks wrong. Shouldn't it leave the DST flag alone? I.e., just this:
>>
>>    (unless (decoded-time-zone time)
>>      (setf (decoded-time-zone-time) default-zone))
>>
>> That is, if we assume that for the DST component -1 means "unknown"
>> and nil means "standard time", it should be OK for
>> decoded-time-set-defaults to leave the DST component alone, for the
>> same reason that it leaves the DOW component alone.

> Yes, I think so.  But you changed this in a391ffa2f03, and you usually
> have a good reason for changes like this, so I thought there must be
> something subtle going on here I didn't quite get.  😀

Thanks for the compliment, not sure it's deserved here....

> The old code doesn't look quite right, either, I think...
> 
> -  ;; When we don't have a time zone and we don't have a DST, then mark
> -  ;; it as unknown.
> -  (when (and (not (decoded-time-zone time))
> -             (not (decoded-time-dst time)))
> -    (setf (decoded-time-dst time) -1))
> -
> -  (when (and (not (decoded-time-zone time))
> -             default-zone)
> -    (setf (decoded-time-zone time) 0))

Yes, that old code was wrong because it incorrectly assumeed that (not 
(decoded-time-dst time)) means the DST flag is unspecified, whereas it 
really means that the DST flag is specifying standard time.

It also looked odd because default-zone was used only as a boolean, even 
though its name suggests that it's the default time zone. This usage 
dates back to commit fa648a59c9818ae284209ac7ae4f3700aebd92c9 which you 
installed in July 2019. The only call using default-zone in Emacs is in 
newsticker--decode-iso8601-date, which passes 0 so that the oddity in 
the implementation makes no difference there.

Part of the confusion here is that nil doesn't mean "no time zone is 
known"; it means "use the Emacs default time zone". In other words, nil 
has the same interpretation problem in time zones that it has in DST 
flags - it doesn't mean "unknown".

To try to lessen the confusion I installed the attached, which fixes the 
reported bug so I'll close the bug report. Please feel free to revert if 
you see a problem with it (I'm just trying to save time here by being bold).

From fe38cbc14fb627f39e145b9a85f029af96f903c7 Mon Sep 17 00:00:00 2001
From: Paul Eggert <[email-masked]>
Date: Fri, 27 May 2022 12:19:43 -0700
Subject: [PATCH] decoded-time-set-defaults now leaves DST alone
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

* lisp/calendar/time-date.el (decoded-time-set-defaults):
Don’t mess with decoded-time-dst (Bug#55635).
---
 lisp/calendar/time-date.el | 13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/lisp/calendar/time-date.el b/lisp/calendar/time-date.el
index dc77a7c7e0..40374c3bb4 100644
--- a/lisp/calendar/time-date.el
+++ b/lisp/calendar/time-date.el
@@ -562,6 +562,9 @@ decoded-time-set-defaults
 This year is used to guarantee portability; see Info
 node `(elisp) Time of Day'.
 
+Optional argument DEFAULT-ZONE specifies what time zone to
+default to when TIME's time zone is nil (meaning local time).
+
 TIME is modified and returned."
   (unless (decoded-time-second time)
     (setf (decoded-time-second time) 0))
@@ -577,13 +580,11 @@ decoded-time-set-defaults
   (unless (decoded-time-year time)
     (setf (decoded-time-year time) 1970))
 
-  ;; When we don't have a time zone, default to DEFAULT-ZONE without
-  ;; DST if DEFAULT-ZONE if given, and to unknown DST otherwise.
   (unless (decoded-time-zone time)
-    (if default-zone
-	(progn (setf (decoded-time-zone time) default-zone)
-	       (setf (decoded-time-dst time) nil))
-      (setf (decoded-time-dst time) -1)))
+    (setf (decoded-time-zone time) default-zone))
+
+  ;; Do not set decoded-time-weekday or decoded-time-dst,
+  ;; as encode-time can infer them well enough when unknown.
 
   time)
 
-- 
2.34.1

Subject: `make-decoded-time' incorrectly sets DST to nil, it should be -1 (guess)
From: Maxim Nikulin <[email-masked]>
To: [email-masked]
Date: Wed, 25 May 2022 21:46:50 +0700
List-Id: <debbugs-submit.debbugs.gnu.org>

Consider the following example:

     (format-time-string
      "%F %T %Z %z"
      (encode-time
       (make-decoded-time :year 2022 :month 3 :day 31
     			 :hour 23 :minute 30 :second 0
     			 :zone "Europe/Madrid"))
      "Europe/Madrid")
     "2022-04-01 00:30:00 CEST +0200"

I believe that the result should be
     "2022-03-31 23:30:00 CEST +0200"
It can be obtained if :dst -1 is explicitly added to the 
`make-decoded-time' arguments.

Since `make-decoded-time' is defined using `cl-defun', I think, it is 
better to use -1 ("guess") default value for the :dst argument, not nil 
that explicitly says "no daylight saving time".

There is `decoded-time-set-defaults', but it does not help

     (format-time-string
      "%F %T %Z %z"
      (encode-time
       (decoded-time-set-defaults
        (make-decoded-time :year 2022 :month 3 :day 31
			  :hour 23 :minute 30)
        "Europe/Madrid"))
      "Europe/Madrid")
     "2022-04-01 01:30:00 CEST +0200"

This case I have no idea how to fix the issue.

An example in the `decoded-time-add' docstring
 >   (decoded-time-add (decode-time) (make-decoded-time :month 2))
adds even more confusion. If `make-decoded-time' is intended for 
intervals, not timestamps than it should not have DST and TZ values at 
all. Time interval may be added to timestamp, and time zone and daylight 
saving time flag is the property of particular timestamp while the same 
interval may be added to various timestamps and the actual result 
depends on the base timestamp.

Timestamp and interval are different types and should not be used 
interchangeably. nil/t/-1 interpretation difference for DST causes 
issues like (bug#54731), so it should be handled with care.