emacs-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Try and detect common problems in lexical code


From: Stefan Monnier
Subject: Try and detect common problems in lexical code
Date: Wed, 18 Mar 2020 16:30:28 -0400
User-agent: Gnus/5.13 (Gnus v5.13) Emacs/28.0.50 (gnu/linux)

One of the most common problem with the use of lexical binding seems to
be when we do something like:

    (let ((byte-compile-dest-file-function …))
      (byte-compile ...))

because the file that defines `byte-compile-dest-file-function` might be
loaded after the let binding is created (i.e. in response to the call
to `byte-compile`).

We already try to handle the same problem with dynamic bindings (where
the problem was that `byte-compile-dest-file-function` revert to being
unbound once we exit the `let`; problem we fixed by making `defvar`
affect the `default-toplevel-value` rather than the `default-value`),

The patch below does something similar for the lexical case.  It's only
*similar* because for the dynamic scoping case we found a solution that
tries to guess the real intent and hence makes the code work, whereas
the patch below only detects the problem and signals an error.  Maybe we
could actually try to also guess the intent and fix the problem on the
spot, but that seems more delicate, so I haven't tried to go there yet.

Another difference is that this probably has a more noticeable
performance impact: the dynamic case needs to walk the specpdl stack
only when the var already has a value when we get to `defvar` (which is
the less usual case), whereas here we have to walk the the specpdl stack
when the var does not yet have a value (which is the usual case), and on
top of that, our walk is more expensive.

WDYT?


        Stefan


diff --git a/src/eval.c b/src/eval.c
index 4559a0e1f6..40ad0a2136 100644
--- a/src/eval.c
+++ b/src/eval.c
@@ -688,6 +688,46 @@ default_toplevel_binding (Lisp_Object symbol)
   return binding;
 }
 
+/* Look for a lexical-binding of SYMBOL somewhere up the stack.
+   This will only find bindings created with interpreted code, since once
+   compiled names of lexical variables are basically gone anyway.  */
+static bool
+lexbound_p (Lisp_Object symbol)
+{
+  union specbinding *binding = NULL;
+  union specbinding *pdl = specpdl_ptr;
+  while (pdl > specpdl)
+    {
+      switch ((--pdl)->kind)
+       {
+       case SPECPDL_LET_DEFAULT:
+       case SPECPDL_LET:
+         if (EQ (specpdl_symbol (pdl), Qinternal_interpreter_environment))
+           {
+             Lisp_Object env = specpdl_old_value (pdl);
+             if (CONSP (env) && !NILP (Fassq (symbol, env)))
+               return true;
+           }
+         break;
+
+       case SPECPDL_UNWIND:
+       case SPECPDL_UNWIND_ARRAY:
+       case SPECPDL_UNWIND_PTR:
+       case SPECPDL_UNWIND_INT:
+       case SPECPDL_UNWIND_INTMAX:
+       case SPECPDL_UNWIND_EXCURSION:
+       case SPECPDL_UNWIND_VOID:
+       case SPECPDL_BACKTRACE:
+       case SPECPDL_LET_LOCAL:
+         break;
+
+       default:
+         emacs_abort ();
+       }
+    }
+  return false;
+}
+
 DEFUN ("default-toplevel-value", Fdefault_toplevel_value, 
Sdefault_toplevel_value, 1, 1, 0,
        doc: /* Return SYMBOL's toplevel default value.
 "Toplevel" means outside of any let binding.  */)
@@ -723,6 +763,15 @@ DEFUN ("internal--define-uninitialized-variable",
 value.  */)
   (Lisp_Object symbol, Lisp_Object doc)
 {
+  if (!XSYMBOL (symbol)->u.s.declared_special
+      && lexbound_p (symbol))
+    /* This test tries to catch the situation where we do
+       (let ((<foo-var> ...)) ...(<foo-function> ...)....)
+       and where the `foo` package only gets loaded when <foo-function>
+       is called, so the outer `let` incorrectly made the binding lexical
+       because the <foo-var> wasn't defined yet at that point.  */
+    error ("Defining as dynamic an already lexical var");
+
   XSYMBOL (symbol)->u.s.declared_special = true;
   if (!NILP (doc))
     {




reply via email to

[Prev in Thread] Current Thread [Next in Thread]