Evil defvars in org.el

Evil defvars in org.el

[Chong Yidong]

org.el (and org-agenda.el, and maybe others) have defvars like this:

(defvar entry)
(defvar date)

This is going to blow up in someone's face, because it makes `entry' and
`date' unusable as lexical variables, even though those are very common
variable names.  That is to say, loading Org mode would change the
behavior of packages using lexical binding in some unpredictable way.

Why does org need to do this?  Could you try to clean it up?

Re: Evil defvars in org.el

[Bastien]

Chong Yidong <cyd@gnu.org> writes:

> org.el (and org-agenda.el, and maybe others) have defvars like this:
>
> (defvar entry)
> (defvar date)
>
> This is going to blow up in someone's face, because it makes `entry' and
> `date' unusable as lexical variables, even though those are very common
> variable names.  That is to say, loading Org mode would change the
> behavior of packages using lexical binding in some unpredictable way.

Yes, this is bugging me too.

> Why does org need to do this?  

From latest calendar.el in Emacs:

;; A note on free variables:

;; The calendar passes around a few dynamically bound variables, which
;; unfortunately have rather common names.  They are meant to be
;; available for external functions, so the names can't be changed.

;; displayed-month, displayed-year: bound in calendar-generate, the
;;   central month of the 3 month calendar window
;; original-date, number: bound in diary-list-entries, the arguments
;;   with which that function was called.
;; date, entry: bound in diary-list-sexp-entries (qv)
   ^^^^  ^^^^^

Org uses those two free variables in several places.

> Could you try to clean it up?

I would love too.  Is there anything I can do without messing up with
Calendar's code?

-- 
 Bastien

Re: Evil defvars in org.el

[Bastien]

Hi Glenn,

Bastien <bzg@gnu.org> writes:

> From latest calendar.el in Emacs:
>
> ;; A note on free variables:
>
> ;; The calendar passes around a few dynamically bound variables, which
> ;; unfortunately have rather common names.  They are meant to be
> ;; available for external functions, so the names can't be changed.
>
> ;; displayed-month, displayed-year: bound in calendar-generate, the
> ;;   central month of the 3 month calendar window
> ;; original-date, number: bound in diary-list-entries, the arguments
> ;;   with which that function was called.
> ;; date, entry: bound in diary-list-sexp-entries (qv)
>    ^^^^  ^^^^^

I'm not entirely sure whether the comment quoted above reflects 
the situation in Calendar's code.

Could you have a look and tell me if something needs to be fixed
in Calendar?

Thanks!

-- 
 Bastien

Re: Evil defvars in org.el

[Edward Reingold]

[-- Attachment #1: Type: text/plain, Size: 1586 bytes --]

Although I no longer maintain the code (and have not for years), my memory
is that many things will break if this is changed willy-nilly.  It is part
of the earliest-written (circa 1983), deepest levels of the code, so the
changes will be pervasive, affecting almost every .el file.  When I wrote
the code (primarily for my own use and amusement), I saw little choice in
using the mechanism, though I knew it was clunky.  Emacs at the time was
MUCH smaller and I knew most of its internals well, so I felt pretty
safe...who'd have guessed how much it would grow over almost 30 years, or
that my code would still be around.  Sorry.

On Fri, Apr 27, 2012 at 9:48 AM, Bastien <bzg@gnu.org> wrote:

> Hi Glenn,
>
> Bastien <bzg@gnu.org> writes:
>
> > From latest calendar.el in Emacs:
> >
> > ;; A note on free variables:
> >
> > ;; The calendar passes around a few dynamically bound variables, which
> > ;; unfortunately have rather common names.  They are meant to be
> > ;; available for external functions, so the names can't be changed.
> >
> > ;; displayed-month, displayed-year: bound in calendar-generate, the
> > ;;   central month of the 3 month calendar window
> > ;; original-date, number: bound in diary-list-entries, the arguments
> > ;;   with which that function was called.
> > ;; date, entry: bound in diary-list-sexp-entries (qv)
> >    ^^^^  ^^^^^
>
> I'm not entirely sure whether the comment quoted above reflects
> the situation in Calendar's code.
>
> Could you have a look and tell me if something needs to be fixed
> in Calendar?
>
> Thanks!
>
> --
>  Bastien
>
>

[-- Attachment #2: Type: text/html, Size: 2134 bytes --]

Re: Evil defvars in org.el

[Glenn Morris]

I discussed this with Stefan off-list some time ago.
Still waiting for "dlet"... :)

Stefan Monnier wrote:

> > What to do about things like the calendar, where dynamic variables
> > with common names are an advertized part of the interface?
> 
> Good question.  I indeed looked at calendar as one of the first things
> where I tried to fix those top-level identifiers and bumped into this
> very problem, which kind of killed my motivation.
> 
> > Eg diary-list-entries has always advertized NUMBER and ORIGINAL-DATE
> > as available for use in the hook functions it calls. It's widely used
> > within the calendar, and probably outside as well. It's not how I
> > would have implemented it, but here we are.
> 
> > The only way I can think to change it is to make it respect both eg
> > DIARY-NUMBER and NUMBER equally, with a view to removing NUMBER in
> > future.
> 
> I think in order to fix this, we're going to have to add a `dlet'
> (i.e. bind a variable dynamically even if it's not defvar'ed) construct
> of some sort.  And/or maybe some way to specify additional dyn-bindings
> to `eval'.

Re: Evil defvars in org.el

[Bastien]

Hi Edward,

good to read you -- and *thanks* for the calendar!  Whatever the
problems that can emerge from it, no doubt this is one of the most
used feature in Emacs.  And also thanks to remind use "amusement"
is part of the job.  

All best,

-- 
 Bastien

RE: Evil defvars in org.el

[Drew Adams]

> I discussed this with Stefan off-list some time ago.
> Still waiting for "dlet"... :)
> 
> Stefan Monnier wrote:
> 
> > I think in order to fix this, we're going to have to add a `dlet'
> > (i.e. bind a variable dynamically even if it's not defvar'ed)
> > construct of some sort.  And/or maybe some way to specify
> > additional dyn-bindings to `eval'.

How is it that Common Lisp apparently has no need for something like `dlet'?  Is
it because it has a namespace system ("packages")?  (Not rhetorical questions.)

Re: Evil defvars in org.el

[SAKURAI Masashi]

Hi Edward,

In last year, I made calfw.el, the calendar view like the google calendar.
Then, I had read the calendar.el code many times and studied the algorithm.
So, I'm glad to know the old story at your post.

Thank you so much.

--
SAKURAI, Masashi (family, given)
m.sakurai@kiwanami.net


At Fri, 27 Apr 2012 09:57:36 -0500,
Edward Reingold wrote:
> 
> [1  <text/plain; ISO-8859-1 (7bit)>]
> 
> [2  <text/html; ISO-8859-1 (quoted-printable)>]
> Although I no longer maintain the code (and have not for years), my memory is that many things
> will break if this is changed willy-nilly.  It is part of the earliest-written (circa 1983),
> deepest levels of the code, so the changes will be pervasive, affecting almost every .el file. 
> When I wrote the code (primarily for my own use and amusement), I saw little choice in using the
> mechanism, though I knew it was clunky.  Emacs at the time was MUCH smaller and I knew most of its
> internals well, so I felt pretty safe...who'd have guessed how much it would grow over almost 30
> years, or that my code would still be around.  Sorry.

Re: Evil defvars in org.el

[Miles Bader]

"Drew Adams" <drew.adams@oracle.com> writes:
> How is it that Common Lisp apparently has no need for something like
> `dlet'?  Is it because it has a namespace system ("packages")?  (Not
> rhetorical questions.)

It has somethig like dlet:

  (let ((foo ...))
    (declare (special foo))
    ...)

-miles

-- 
The key to happiness
 is having dreams.      [from a fortune cookie]

RE: Evil defvars in org.el

[Drew Adams]

> It has somethig like dlet:
>   (let ((foo ...)) (declare (special foo)) ...)

Ah yes.  Thanks for the reminder.

Re: Evil defvars in org.el

[Miles Bader]

"Drew Adams" <drew.adams@oracle.com> writes:
>> It has somethig like dlet:
>>   (let ((foo ...)) (declare (special foo)) ...)
>
> Ah yes.  Thanks for the reminder.

... I suppose since Emacs already pays attention to (declare ...) in
some cases, it could do it in this case too...

-miles

-- 
Yossarian was moved very deeply by the absolute simplicity of
this clause of Catch-22 and let out a respectful whistle.
"That's some catch, that Catch-22," he observed.
"It's the best there is," Doc Daneeka agreed.

Re: Evil defvars in org.el

[Stefan Monnier]

> org.el (and org-agenda.el, and maybe others) have defvars like this:
> (defvar entry)
> (defvar date)

> This is going to blow up in someone's face, because it makes `entry' and
> `date' unusable as lexical variables, even though those are very common
> variable names.

While I strongly support fixing such things, do note that since those
cases are very common, the lexical-binding code had to find a way to
avoid this mess.  So in reality in Emacs-24 such (defvar <foo>) only
affect the current file (actually only the current scope, so you can do
(defun test () (defvar toto) (let (toto ...) ...)) and `toto' will only
be dynbound within `test'.

On the other hand, in the case of eieio's `this' we can't so easily
avoid the mess, because `this' is declared with a global value, making
it *globally* dynamically bound.


        Stefan

Re: Evil defvars in org.el

[Chong Yidong]

Glenn Morris <rgm@gnu.org> writes:

> I discussed this with Stefan off-list some time ago.
> Still waiting for "dlet"... :)
>
>> > Eg diary-list-entries has always advertized NUMBER and
>> > ORIGINAL-DATE as available for use in the hook functions it
>> > calls. It's widely used within the calendar, and probably outside
>> > as well. It's not how I would have implemented it, but here we are.
>> 
>> > The only way I can think to change it is to make it respect both eg
>> > DIARY-NUMBER and NUMBER equally, with a view to removing NUMBER in
>> > future.

I think we should bite the bullet and introduce new defvars with
properly prefixed names, as was done for `e'.  We can have a transition
period during which the diary binds both the new variables and the old
ones, then eliminate the old ones in (say) Emacs 25.  Any third party
code that wants backward compatibility with old Emacs versions can check
if the new variables are bound, and use them first.

Re: Evil defvars in org.el

[Achim Gratz]

Stefan Monnier writes:
> While I strongly support fixing such things, do note that since those
> cases are very common, the lexical-binding code had to find a way to
> avoid this mess.

It would certainly be a good idea if the Elisp manual said something
about this.

>  So in reality in Emacs-24 such (defvar <foo>) only
> affect the current file (actually only the current scope, so you can do
> (defun test () (defvar toto) (let (toto ...) ...)) and `toto' will only
> be dynbound within `test'.

I had no idea this is even possible... and the manual advises that the
defvar should "usually be at top-level".  So, commenting out all the
file-level (defvar date) in all of org and then changing in org-bbdb.el
like this:

--8<---------------cut here---------------start------------->8---
(defun org-bbdb-anniversaries()
  "Extract anniversaries from BBDB for display in the agenda."
  (with-no-warnings (defvar date)) ;; dynamically scoped from calendar
  (require 'bbdb)
  (require 'diary-lib)
  (unless (hash-table-p org-bbdb-anniv-hash)
    (setq org-bbdb-anniv-hash
	  (make-hash-table :test 'equal :size 366)))

  (when (or org-bbdb-updated-p
            (= 0 (hash-table-count org-bbdb-anniv-hash)))
    (org-bbdb-make-anniv-hash))

  (let* ((m (car date))    ; month
         (d (nth 1 date))  ; day
         (y (nth 2 date))  ; year
         (annivs (gethash (list m d) org-bbdb-anniv-hash))
         (text ())
         rec recs)
--8<---------------cut here---------------end--------------->8---

should create a dynamic binding that exists only within the scope of
org-bbdb-anniversary, is that right?  Could it possibly leak into other
scopes due to the (require ...) present?  I still have to supress the
warning due to unprefixed "date", should this warning automatically be
suppressed in this idiom?


Regards,
Achim.
-- 
+<[Q+ Matrix-12 WAVE#46+305 Neuron microQkb Andromeda XTk Blofeld]>+

SD adaptation for Waldorf Blofeld V1.15B11:
http://Synth.Stromeko.net/Downloads.html#WaldorfSDada

RE: Evil defvars in org.el

[Drew Adams]

> So in reality in Emacs-24 such (defvar <foo>) only
> affect the current file (actually only the current scope, so 
> you can do (defun test () (defvar toto) (let (toto ...) ...))
> and `toto' will only be dynbound within `test'.

Good to know.  Where is that documented (besides here)?

Re: Evil defvars in org.el

[Stefan Monnier]

>> So in reality in Emacs-24 such (defvar <foo>) only
>> affect the current file (actually only the current scope, so you can do
>> (defun test () (defvar toto) (let (toto ...) ...)) and `toto' will only
>> be dynbound within `test'.

> I had no idea this is even possible... and the manual advises that the
> defvar should "usually be at top-level".  So, commenting out all the
> file-level (defvar date) in all of org and then changing in org-bbdb.el
> like this:

In general, having them at file-level is just as good as having it
within a defun.


        Stefan

Re: Evil defvars in org.el

[Achim Gratz]

Stefan Monnier writes:
>> I had no idea this is even possible... and the manual advises that the
>> defvar should "usually be at top-level".  So, commenting out all the
>> file-level (defvar date) in all of org and then changing in org-bbdb.el
>> like this:
>
> In general, having them at file-level is just as good as having it
> within a defun.

Either I've lost you here or the conclusion is that we can't do anything
in org to make these defvars less evil and should just silence the
warnings associated with them?


Regards,
Achim.
-- 
+<[Q+ Matrix-12 WAVE#46+305 Neuron microQkb Andromeda XTk Blofeld]>+

SD adaptations for Waldorf Q V3.00R3 and Q+ V3.54R2:
http://Synth.Stromeko.net/Downloads.html#WaldorfSDada

Re: Evil defvars in org.el

[Stefan Monnier]

>>> I had no idea this is even possible... and the manual advises that the
>>> defvar should "usually be at top-level".  So, commenting out all the
>>> file-level (defvar date) in all of org and then changing in org-bbdb.el
>>> like this:
>> In general, having them at file-level is just as good as having it
>> within a defun.
> Either I've lost you here or the conclusion is that we can't do anything
> in org to make these defvars less evil and should just silence the
> warnings associated with them?

The only thing that can be done for dynamic vars is to make them
namespace-clean, so if renaming them is not an option (which could be
the case for example because those vars appears in user customizations),
then indeed, there's not much you can do.


        Stefan

Re: Evil defvars in org.el

[Glenn Morris]

Chong Yidong wrote:

> I think we should bite the bullet and introduce new defvars with
> properly prefixed names, as was done for `e'.

I guess I can do that at some point if no better solution presents
itself, but I imagine it will cause a lot of external inconvenience. It
seems to me that this is something where there really ought to be a
general Emacs solution (dynamic-let or whatever).

Another example is display-time-string-forms (helpfully hidden by
with-no-warnings). Do you propose to make similar changes everywhere
this same issue occurs?

Re: Evil defvars in org.el

[Stefan Monnier]

> It seems to me that this is something where there really ought to be
> a general Emacs solution (dynamic-let or whatever).

There is such a thing: (defvar <foo>).

> Another example is display-time-string-forms (helpfully hidden by
> with-no-warnings). Do you propose to make similar changes everywhere
> this same issue occurs?

There are many cases where changing the name of the dynamic vars is
too inconvenient (they may even appear in user's customization).


        Stefan

Re: Evil defvars in org.el

[Johan Bockgård]

Stefan Monnier <monnier@iro.umontreal.ca> writes:

>> It seems to me that this is something where there really ought to be
>> a general Emacs solution (dynamic-let or whatever).
>
> There is such a thing: (defvar <foo>).

It currently has a number of problems:

- The scoping of defvar is unclear and changes when the code is
compiled:

(setq lexical-binding t)

(defun a ()
  (let  ((f (lambda () x))
         (g (lambda () (defvar x))))
    (funcall g)
    (let ((x 0))
      (funcall f))))

(defun b ()
  (let (f g)
    (setq f (lambda () x)
          g (lambda () (defvar x)))
    (funcall g)
    (let ((x 0))
      (funcall f))))

(defun c ()
  (let  ((f (lambda () x)))
    (catch nil (defvar x))
    (let ((x 0))
      (funcall f))))

(defun d ()
  (let  ((f (lambda () x)))
    (progn (defvar x) (let (x)))  ; "dlet"
    (let ((x 0))
      (funcall f))))

;; Interpreted
(a) => error: (void-variable x)
(b) => 0
(c) => 0
(d) => 0

;; Compiled
(a) => error
(b) => error
(c) => error
(d) => 0

etc.


- Invalid byte code is generated when the same symbol is used as both a
lexical and a dynamic variable:

(setq lexical-binding t)
(setq foo (byte-compile
           '(lambda ()
              (let (x)
                (defvar x)
                (let (x) 0)))))

(disassemble foo)
byte code:
  doc:   ...
  args: 0
0       constant  nil
1       dup
2       varbind   x    <<<< No matching `unbind'
3       constant  0
4       return

(funcall foo)

   => Fatal error (6)Aborted

Re: Evil defvars in org.el

[Stefan Monnier]

>>> It seems to me that this is something where there really ought to be
>>> a general Emacs solution (dynamic-let or whatever).
>> There is such a thing: (defvar <foo>).
> It currently has a number of problems:
> - The scoping of defvar is unclear and changes when the code is
> compiled:

Indeed, the rules aren't quite the same, which is one of the reasons why
I prefer not to document them.  Basically the rule is "leave them at
top-level", which should work just fine in 99.9% of the situations.

> - Invalid byte code is generated when the same symbol is used as both a
> lexical and a dynamic variable:

That one is a bug.


        Stefan

Re: Evil defvars in org.el

[Glenn Morris]

Then the conclusion I'll take away from this is that there is no need to
make any changes.

Re: Evil defvars in org.el

[Stefan Monnier]

> Then the conclusion I'll take away from this is that there is no need to
> make any changes.

Pretty much: we should try and get rid of those non-prefixed dynamic
vars (e.g. by adding a prefix) and we should work even harder to try and
avoid introducing more of them.  But several of the ones that are
out there will have to stay because fixing them is likely to do more
harm (e.g. by inconveniencing users) than leaving them as they are.


        Stefan

Re: Evil defvars in org.el

[Martyn Jago]

Chong Yidong <cyd@gnu.org> writes:

> org.el (and org-agenda.el, and maybe others) have defvars like this:
>
> (defvar entry)
> (defvar date)
>
> This is going to blow up in someone's face, because it makes `entry' and
> `date' unusable as lexical variables, even though those are very common
> variable names.  That is to say, loading Org mode would change the
> behavior of packages using lexical binding in some unpredictable way.
>
> Why does org need to do this?  Could you try to clean it up?

Perhaps Emacs can prefix the `evil vars' and alias them to the original,
with a deprecation warning?

Best, Martyn

Re: Evil defvars in org.el

[Stefan Monnier]

>> org.el (and org-agenda.el, and maybe others) have defvars like this:
>> 
>> (defvar entry)
>> (defvar date)
>> 
>> This is going to blow up in someone's face, because it makes `entry' and
>> `date' unusable as lexical variables, even though those are very common
>> variable names.  That is to say, loading Org mode would change the
>> behavior of packages using lexical binding in some unpredictable way.
>> 
>> Why does org need to do this?  Could you try to clean it up?

> Perhaps Emacs can prefix the `evil vars' and alias them to the original,
> with a deprecation warning?

Oh, no, no, please no, if you alias them then they become globally
"dynamic vars", whereas currently they're only dynamic vars within the
files that do (defvar entry) so the damage is somewhat contained.


        Stefan