Bug in definition of org-encode-time

Bug in definition of org-encode-time

[Kenneth Stuart]

Hello,

Macro `org-encode-time` (lisp/org-macs.el +1395) does not get defined
for emacs >= 27.1 as the top level if statement is missing its ELSE clause.

#+begin_src elisp
(if (version< emacs-version "27.1")
    (defmacro org-encode-time (&rest time)
      (if (cdr time)
          `(encode-time ,@time)
        `(apply #'encode-time ,@time)))
  (if (ignore-errors (with-no-warnings (encode-time '(0 0 0 1 1 1971))))
      (defmacro org-encode-time (&rest time)
        (pcase (length time) ; Emacs-29 since d75e2c12eb
          (1 `(encode-time ,@time))
          ((or 6 9) `(encode-time (list ,@time)))
          (_ (error "`org-encode-time' may be called with 1, 6, or 9 arguments but %d given"
                    (length time)))))
    (defmacro org-encode-time (&rest time)
      (pcase (length time)
        (1 `(encode-time ,@time))
        (6 `(encode-time (list ,@time nil -1 nil)))
        (9 `(encode-time (list ,@time)))
        (_ (error "`org-encode-time' may be called with 1, 6, or 9 arguments but %d given"
                  (length time)))))) //MISSING ELSE//)
#+end_src

I'm assuming it should be:

#+begin_src diff
  diff --git a/lisp/org-macs.el b/lisp/org-macs.el
index 5931dd260..bbdacbdf8 100644
--- a/lisp/org-macs.el
+++ b/lisp/org-macs.el
@@ -1403,14 +1403,14 @@ nil, just return 0."
           (1 `(encode-time ,@time))
           ((or 6 9) `(encode-time (list ,@time)))
           (_ (error "`org-encode-time' may be called with 1, 6, or 9 arguments but %d given"
-                    (length time)))))
-    (defmacro org-encode-time (&rest time)
+                    (length time))))))
+  (defmacro org-encode-time (&rest time)
       (pcase (length time)
         (1 `(encode-time ,@time))
         (6 `(encode-time (list ,@time nil -1 nil)))
         (9 `(encode-time (list ,@time)))
         (_ (error "`org-encode-time' may be called with 1, 6, or 9 arguments but %d given"
-                  (length time)))))))
+                  (length time))))))
 (put 'org-encode-time 'function-documentation
      "Compatibility and convenience helper for `encode-time'.
 May be called with 9 components list (SECONDS ... YEAR IGNORED DST ZONE)
#+end_src

regards
Ken

Re: Bug in definition of org-encode-time

[Ihor Radchenko]

Kenneth Stuart <kstuart@hotmail.co.uk> writes:

> Macro `org-encode-time` (lisp/org-macs.el +1395) does not get defined
> for emacs >= 27.1 as the top level if statement is missing its ELSE clause.

Are you sure?

> #+begin_src elisp
> (if (version< emacs-version "27.1")
>     (defmacro org-encode-time (&rest time)
>       (if (cdr time)
>           `(encode-time ,@time)
>         `(apply #'encode-time ,@time)))
>   (if (ignore-errors (with-no-warnings (encode-time '(0 0 0 1 1 1971))))
>       (defmacro org-encode-time (&rest time)
>         (pcase (length time) ; Emacs-29 since d75e2c12eb
>           (1 `(encode-time ,@time))
>           ((or 6 9) `(encode-time (list ,@time)))
>           (_ (error "`org-encode-time' may be called with 1, 6, or 9 arguments but %d given"
>                     (length time)))))
>     (defmacro org-encode-time (&rest time)
>       (pcase (length time)
>         (1 `(encode-time ,@time))
>         (6 `(encode-time (list ,@time nil -1 nil)))
>         (9 `(encode-time (list ,@time)))
>         (_ (error "`org-encode-time' may be called with 1, 6, or 9 arguments but %d given"
>                   (length time)))))) //MISSING ELSE//)
> #+end_src

This is equivalent to

(if condition
 (defmacro ...)
 ;; else
 (if another-condition
  (defmacro ...)
  ;; else
  (defmacro)))

Best,
Ihor

Re: Bug in definition of org-encode-time

[Kenneth Stuart]

Yes, you're right I should have looked more closely, I also see 'Morgan
Smith' has already raised the issue more clearly.

Please ignore the noise ;)

Ihor Radchenko <yantar92@gmail.com> writes:

> Kenneth Stuart <kstuart@hotmail.co.uk> writes:
>
>> Macro `org-encode-time` (lisp/org-macs.el +1395) does not get defined
>> for emacs >= 27.1 as the top level if statement is missing its ELSE clause.
>
> Are you sure?
>
>> #+begin_src elisp
>> (if (version< emacs-version "27.1")
>>     (defmacro org-encode-time (&rest time)
>>       (if (cdr time)
>>           `(encode-time ,@time)
>>         `(apply #'encode-time ,@time)))
>>   (if (ignore-errors (with-no-warnings (encode-time '(0 0 0 1 1 1971))))
>>       (defmacro org-encode-time (&rest time)
>>         (pcase (length time) ; Emacs-29 since d75e2c12eb
>>           (1 `(encode-time ,@time))
>>           ((or 6 9) `(encode-time (list ,@time)))
>>           (_ (error "`org-encode-time' may be called with 1, 6, or 9 arguments but %d given"
>>                     (length time)))))
>>     (defmacro org-encode-time (&rest time)
>>       (pcase (length time)
>>         (1 `(encode-time ,@time))
>>         (6 `(encode-time (list ,@time nil -1 nil)))
>>         (9 `(encode-time (list ,@time)))
>>         (_ (error "`org-encode-time' may be called with 1, 6, or 9 arguments but %d given"
>>                   (length time)))))) //MISSING ELSE//)
>> #+end_src
>
> This is equivalent to
>
> (if condition
>  (defmacro ...)
>  ;; else
>  (if another-condition
>   (defmacro ...)
>   ;; else
>   (defmacro)))
>
> Best,
> Ihor

Re: Bug in definition of org-encode-time

[Kenneth Stuart]

This appears to be an issue with the emacs native compiler, as using a
build without it enabled works as expected, and using a build with the
native compiler enabled but before the relevant .eln files have been
built also works as expected, once they've been built the issue
mentioned by Morgan on the 22nd is once again present.

Quote:
> When I re-evaluate the defun for `org-matcher-time' everything is happy
> again.  I'm not sure what the issue is but I assume it's related to
> compilation or something.

emacs commit: 928ea0fbf13671e17c9839791163d1da056df490

I'll try reporting the issue using `report-emacs-bug`.

Best,
Ken

Kenneth Stuart <kstuart@hotmail.co.uk> writes:

> Yes, you're right I should have looked more closely, I also see 'Morgan
> Smith' has already raised the issue more clearly.
>
> Please ignore the noise ;)
>
> Ihor Radchenko <yantar92@gmail.com> writes:
>
>> Kenneth Stuart <kstuart@hotmail.co.uk> writes:
>>
>>> Macro `org-encode-time` (lisp/org-macs.el +1395) does not get defined
>>> for emacs >= 27.1 as the top level if statement is missing its ELSE clause.
>>
>> Are you sure?
>>
>>> #+begin_src elisp
>>> (if (version< emacs-version "27.1")
>>>     (defmacro org-encode-time (&rest time)
>>>       (if (cdr time)
>>>           `(encode-time ,@time)
>>>         `(apply #'encode-time ,@time)))
>>>   (if (ignore-errors (with-no-warnings (encode-time '(0 0 0 1 1 1971))))
>>>       (defmacro org-encode-time (&rest time)
>>>         (pcase (length time) ; Emacs-29 since d75e2c12eb
>>>           (1 `(encode-time ,@time))
>>>           ((or 6 9) `(encode-time (list ,@time)))
>>>           (_ (error "`org-encode-time' may be called with 1, 6, or 9 arguments but %d given"
>>>                     (length time)))))
>>>     (defmacro org-encode-time (&rest time)
>>>       (pcase (length time)
>>>         (1 `(encode-time ,@time))
>>>         (6 `(encode-time (list ,@time nil -1 nil)))
>>>         (9 `(encode-time (list ,@time)))
>>>         (_ (error "`org-encode-time' may be called with 1, 6, or 9 arguments but %d given"
>>>                   (length time)))))) //MISSING ELSE//)
>>> #+end_src
>>
>> This is equivalent to
>>
>> (if condition
>>  (defmacro ...)
>>  ;; else
>>  (if another-condition
>>   (defmacro ...)
>>   ;; else
>>   (defmacro)))
>>
>> Best,
>> Ihor