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

[GNU bug Tracking System]

Your message dated Fri, 27 May 2022 12:26:56 -0700
with message-id <e4fdefdc-c2ea-de99-3745-66e75e6ad04c@cs.ucla.edu>
and subject line Re: bug#55635: `make-decoded-time' incorrectly sets DST to 
nil, it should be -1 (guess)
has caused the debbugs.gnu.org bug report #55635,
regarding `make-decoded-time' incorrectly sets DST to nil, it should be -1 
(guess)
to be marked as done.

(If you believe you have received this mail in error, please contact
help-debbugs@gnu.org.)


-- 
55635: https://debbugs.gnu.org/cgi/bugreport.cgi?bug=55635
GNU Bug Tracking System
Contact help-debbugs@gnu.org with problems
> --- Begin Message ---
>
>
>
> Subject: 
>
> `make-decoded-time' incorrectly sets DST to nil, it should be -1 (guess)
>
>
>
>
> Date: 
>
> Wed, 25 May 2022 21:46:50 +0700
>
>
>
>
> User-agent: 
>
> Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Thunderbird/91.8.1
>
>
>
>
> 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.
>
>
>
> --- End Message ---

--- Begin Message --- Subject: Re: bug#55635: `make-decoded-time' incorrectly sets DST to nil, it should be -1 (guess) Date: Fri, 27 May 2022 12:26:56 -0700 User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Thunderbird/91.9.1

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

0001-decoded-time-set-defaults-now-leaves-DST-alone.patch
Description: Text Data

--- End Message ---