Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Currently left out topic: Use lexical binding. #53

Open
TobiasZawada opened this issue Jun 19, 2019 · 6 comments
Open

Currently left out topic: Use lexical binding. #53

TobiasZawada opened this issue Jun 19, 2019 · 6 comments

Comments

@TobiasZawada
Copy link

TobiasZawada commented Jun 19, 2019

Nowdays one should use lexical binding for library programming in Elisp. Optimization is better with lexical binding.

The new Section should contain the following warning:
If one uses a special variable from another library in a function one must have either a top-level defvar for that variable in the file where it is used or a require for the other library at top-level. Otherwise let in functions can go terribly wrong for variables that are declared special in other libraries. The lexical binding of that variables is not visible in functions called within the let-form.

@TobiasZawada
Copy link
Author

TobiasZawada commented Jun 19, 2019

Example:

File /tmp/a.el:

;;; -*- lexical-binding: t -*-
(defvar a-var 0
  "Dynamically bound variable.")

(defun a-print-var:0 ()
  "Print var."
  (message "Value of var: %s" a-var))

(provide 'a)

File /tmp/b.el:

;;; -*- lexical-binding: t -*-
(declare-function a-print-var:0 "a")

(defun b-print-a:1 ()
  (let ((a-var 1))
    (funcall #'a-print-var:0)))

(provide 'b)

Either (defvar a-var), (require 'a), or deleting -*- lexical-binding: t -*- on top of b.el avoids the problem demonstrated below.

Compilation rules. Also work repeatedly:

(dolist (sym '(a-print-var:0 b-print-a:1 a-var))
  (unintern sym nil))

(setq features (remove 'b (remove 'a features)))
(byte-compile-file "/tmp/a.el")
(byte-compile-file "/tmp/b.el")
(add-to-list 'load-path "/tmp")

Evaluation:

(load "/tmp/a.elc")
(load "/tmp/b.elc")
(b-print-a:1)

Result:

Value of var: 0

This might be unexpected since a-var is bound to 1 in b-print-a:1.

Note that (require 'a) at top-level is sometimes not wanted if b-print-a:1 is seldom executed even with package b loaded.
In that case one tends to include the require-form in the body of b-print-a:1 as in the following example:

;;; -*- lexical-binding: t -*-
(declare-function a-print-var:0 "a")

(defun b-print-a:1 ()
  (require 'a)
  (let ((a-var 1))
    (funcall #'a-print-var:0)))

(provide 'b)

In this form, we get again the unexpected result Value of var: 0 when evaluating b-print-a:1.
It is essential to insert a defvar for a-var at top level to avoid the unexpected result:

;;; -*- lexical-binding: t -*-
(declare-function a-print-var:0 "a")

(defvar a-var)

(defun b-print-a:1 ()
  (require 'a)
  (let ((a-var 1))
    (funcall #'a-print-var:0)))

(provide 'b)

@Fuco1
Copy link
Collaborator

Fuco1 commented Jun 19, 2019

Otherwise let in functions can go terribly wrong for variables that are declared special in other libraries. The lexical binding of that variables is not visible in functions called within the let-form.

Isn't this the whole point of lexical binding? To bind variables lexically and not overflow to the context of other libraries? Or is this what you are pointing out?

I'm a bit confused what the action here should be.

@TobiasZawada
Copy link
Author

TobiasZawada commented Jun 19, 2019

It is essentially a remainder to put a defvar into top-level when one uses a dynamically bound variable of another package in a function and requires the other package only within the function. I think the defvar in the last code block is the most important hint here. Maybe the definition of file a and the last code block are sufficient as an example. One should explicitly mention the defvar in file b.el.

I stress that because of my own experiences: Missing the defvar can cause strange effects that are difficult to debug.

About your question:

Isn't this the whole point of lexical binding? To bind variables lexically and not overflow to the context of other libraries? Or is this what you are pointing out?

We have here the opposite case. I tried to indicate that by the package names a and b. In package b the special variable a-var from a is let-bound to get some effect in function a-print-a:0 from a when called from b-print-a:1.

@Fuco1
Copy link
Collaborator

Fuco1 commented Jun 19, 2019

That implies to me that the user here would be aware of the existence of a-var and the let-binding would be deliberate. Is that correct?

In that case I agree that if the defvar or the import is omited things will not work as expected. I'm not sure if this is a style choice or more of a "FAQ: My code doesn't work" kind of entry.

I don't care much either way, it is probably useful to the reader. OTOH, it kind of feels out of scope of a style guide as it's more of a programming rule.

@TobiasZawada
Copy link
Author

TobiasZawada commented Jun 20, 2019

Thanks for the comment.

I don't care much either way, it is probably useful to the reader. OTOH, it kind of feels out of scope of a style guide as it's more of a programming rule.

The actual style guid line is to use lexical-scoping. The rest is a warning directly related to the guide line.

The warning has the most relevance when one changes the scoping of a library from dynamical to lexical.
With dynamical scoping the let-forms work without the defvar with lexical scoping they do not work.

By "work" I mean that the value 1 let-bound to a-var in b-print-a:1 is visible in a-print-a:0 within the body of the let-form.

@skangas
Copy link

skangas commented Oct 24, 2020

Nowdays one should use lexical binding for library programming in Elisp. Optimization is better with lexical binding.

Emacs 27.2 will include such a recommendation in its Coding Conventions:
https://git.savannah.gnu.org/gitweb/?p=emacs.git;a=commit;h=8b87ea6844036c168c9ec67dd318ee3ba8dab5ae

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants