bug#30051: 27.0.50; diary-lib.el uses lexical scoping when dynamic scope is needed

bug#30051: 27.0.50; diary-lib.el uses lexical scoping when dynamic scope is needed

[Richard Kim]

I started getting the following error from recently compiled emacs:

    Debugger entered--Lisp error: (void-variable number)
    appt-make-list()
    run-hooks(diary-hook)
    diary-list-entries((1 8 2018) 1)
    org-get-entries-from-diary((1 8 2018))
    org-agenda-list(nil)
    funcall-interactively(org-agenda-list nil)
    call-interactively(org-agenda-list)
    (let nil (call-interactively 'org-agenda-list))
    (let nil (let nil (call-interactively 'org-agenda-list)))
    eval((let nil (let nil (call-interactively 'org-agenda-list))))
    org-let2(nil nil (call-interactively 'org-agenda-list))
    org-agenda-run-series("Agenda and all TODOs" (((agenda "") (alltodo ""))))
    eval((org-agenda-run-series "Agenda and all TODOs" '(((agenda "") (alltodo "")))))
    org-agenda-redo(nil)
    funcall-interactively(org-agenda-redo nil)
    call-interactively(org-agenda-redo nil nil)
    command-execute(org-agenda-redo)

This appears to be due to using lexical binding for diary-lib.el.

diary-list-entries is defined in diary-lib.el which uses lexical-binding as
of master branch change 559d685f68 checked in on 2017/12/04.
However appt-make-list depends on dynamically scoped `number' variable
to be defined by diary-list-entries as documented in the doc-string:

    We assume that the variables DATE and NUMBER hold the arguments that
    `diary-list-entries' received.

In GNU Emacs 27.0.50 (build 1, x86_64-pc-linux-gnu, GTK+ Version 2.24.23)
 of 2017-12-28 built on rutro-int1
Repository revision: ace8f2ed4d6ea49d179333f8e7455e5208417da7
Windowing system distributor 'CentOS', version 11.0.11500000
System Description: CentOS release 6.6 (Final)

bug#30051: Acknowledgement (27.0.50; diary-lib.el uses lexical scoping when dynamic scope is needed)

[Richard Kim]

I should have also mentioned that if I revert 2017/12/04 change of
diary-lib.el so that it goes back to using dynamic scoping (by removing
"-*- lexical-binding: t -*-" in the first line of the file), then I no
longer encounter the error I reported.

bug#30051: 27.0.50; diary-lib.el uses lexical scoping when dynamic scope is needed

[Glenn Morris]

Hi Stefan, please could you take a look at this report.
I can't personally reproduce an error, due to appt-check's use of
ignore-errors, but indeed it no longer seems to work.

Richard Kim wrote:

> I started getting the following error from recently compiled emacs:
>
>     Debugger entered--Lisp error: (void-variable number)
>     appt-make-list()
>     run-hooks(diary-hook)
>     diary-list-entries((1 8 2018) 1)
>     org-get-entries-from-diary((1 8 2018))
>     org-agenda-list(nil)
>     funcall-interactively(org-agenda-list nil)
>     call-interactively(org-agenda-list)
>     (let nil (call-interactively 'org-agenda-list))
>     (let nil (let nil (call-interactively 'org-agenda-list)))
>     eval((let nil (let nil (call-interactively 'org-agenda-list))))
>     org-let2(nil nil (call-interactively 'org-agenda-list))
>     org-agenda-run-series("Agenda and all TODOs" (((agenda "") (alltodo ""))))
>     eval((org-agenda-run-series "Agenda and all TODOs" '(((agenda "") (alltodo "")))))
>     org-agenda-redo(nil)
>     funcall-interactively(org-agenda-redo nil)
>     call-interactively(org-agenda-redo nil nil)
>     command-execute(org-agenda-redo)
>
> This appears to be due to using lexical binding for diary-lib.el.
>
> diary-list-entries is defined in diary-lib.el which uses lexical-binding as
> of master branch change 559d685f68 checked in on 2017/12/04.
> However appt-make-list depends on dynamically scoped `number' variable
> to be defined by diary-list-entries as documented in the doc-string:
>
>     We assume that the variables DATE and NUMBER hold the arguments that
>     `diary-list-entries' received.
>
> In GNU Emacs 27.0.50 (build 1, x86_64-pc-linux-gnu, GTK+ Version 2.24.23)
>  of 2017-12-28 built on rutro-int1
> Repository revision: ace8f2ed4d6ea49d179333f8e7455e5208417da7

bug#30051: 27.0.50; diary-lib.el uses lexical scoping when dynamic scope is needed

[Stefan Monnier]

>> Debugger entered--Lisp error: (void-variable number)
>> appt-make-list()
>> run-hooks(diary-hook)
>> diary-list-entries((1 8 2018) 1)
[...]
>> This appears to be due to using lexical binding for diary-lib.el.

Indeed.

>> However appt-make-list depends on dynamically scoped `number' variable
>> to be defined by diary-list-entries as documented in the doc-string:
>> 
>> We assume that the variables DATE and NUMBER hold the arguments that
>> `diary-list-entries' received.

Indeed, thanks.  I pushed the patch below which should fix this problem.
Thank you,


        Stefan


diff --git a/lisp/calendar/diary-lib.el b/lisp/calendar/diary-lib.el
index 159dd9ba3d..c327717c8a 100644
--- a/lisp/calendar/diary-lib.el
+++ b/lisp/calendar/diary-lib.el
@@ -151,8 +151,10 @@ diary-comment-end
   :group 'diary)
 
 (defcustom diary-hook nil
-  "List of functions called after the display of the diary.
-Used for example by the appointment package - see `appt-activate'."
+  "Hook run after displaying the diary.
+Used for example by the appointment package - see `appt-activate'.
+The variables `number' and `original-date' are dynamically bound around
+the call."
   :type 'hook
   :group 'diary)
 
@@ -779,10 +781,10 @@ diary-list-entries
   `diary-hook' runs last, after the diary is displayed.
       This is used e.g. by `appt-check'.
 
-Functions called by these hooks may use the variables ORIGINAL-DATE
-and NUMBER, which are the arguments with which this function was called.
-Note that hook functions should _not_ use DATE, but ORIGINAL-DATE.
-\(Sexp diary entries may use DATE - see `diary-list-sexp-entries'.)
+Functions called by these hooks may use the variables `original-date'
+and `number', which are the arguments with which this function was called.
+Note that hook functions should _not_ use `date', but `original-date'.
+\(Sexp diary entries may use `date' - see `diary-list-sexp-entries'.)
 
 This function displays the list using `diary-display-function', unless
 LIST-ONLY is non-nil, in which case it just returns the list."
@@ -872,7 +874,9 @@ diary-list-entries
                                   (copy-sequence
                                    (car display-buffer-fallback-action))))))
                       (funcall diary-display-function)))
-                  (run-hooks 'diary-hook)))))
+                  (calendar-dlet* ((number number)
+                                   (original-date original-date))
+                    (run-hooks 'diary-hook))))))
         (and temp-buff (buffer-name temp-buff) (kill-buffer temp-buff)))
       (or d-incp (message "Preparing diary...done"))
       diary-entries-list)))