bbatsov / emacs-lisp-style-guide

A community-driven Emacs Lisp style guide
1.08k stars 53 forks source link

Currently left out topic: Use lexical binding. #53

Open TobiasZawada opened 5 years ago

TobiasZawada commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

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 commented 4 years ago

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