From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.io!.POSTED.ciao.gmane.io!not-for-mail From: Stefan Monnier Newsgroups: gmane.emacs.devel Subject: Try and detect common problems in lexical code Date: Wed, 18 Mar 2020 16:30:28 -0400 Message-ID: Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable Injection-Info: ciao.gmane.io; posting-host="ciao.gmane.io:159.69.161.202"; logging-data="38766"; mail-complaints-to="usenet@ciao.gmane.io" User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/28.0.50 (gnu/linux) To: emacs-devel@gnu.org Original-X-From: emacs-devel-bounces+ged-emacs-devel=m.gmane-mx.org@gnu.org Wed Mar 18 21:34:39 2020 Return-path: Envelope-to: ged-emacs-devel@m.gmane-mx.org Original-Received: from lists.gnu.org ([209.51.188.17]) by ciao.gmane.io with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.92) (envelope-from ) id 1jEfOg-0009sk-Bc for ged-emacs-devel@m.gmane-mx.org; Wed, 18 Mar 2020 21:34:38 +0100 Original-Received: from localhost ([::1]:58314 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1jEfOf-0004Tg-A7 for ged-emacs-devel@m.gmane-mx.org; Wed, 18 Mar 2020 16:34:37 -0400 Original-Received: from eggs.gnu.org ([2001:470:142:3::10]:60252) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1jEfKn-0007xT-6d for emacs-devel@gnu.org; Wed, 18 Mar 2020 16:30:38 -0400 Original-Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1jEfKk-0004aq-Va for emacs-devel@gnu.org; Wed, 18 Mar 2020 16:30:36 -0400 Original-Received: from mailscanner.iro.umontreal.ca ([132.204.25.50]:7871) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1jEfKk-0004ST-4r for emacs-devel@gnu.org; Wed, 18 Mar 2020 16:30:34 -0400 Original-Received: from pmg3.iro.umontreal.ca (localhost [127.0.0.1]) by pmg3.iro.umontreal.ca (Proxmox) with ESMTP id 7DB2F44F35E; Wed, 18 Mar 2020 16:30:31 -0400 (EDT) Original-Received: from mail01.iro.umontreal.ca (unknown [172.31.2.1]) by pmg3.iro.umontreal.ca (Proxmox) with ESMTP id 87FB844F35A; Wed, 18 Mar 2020 16:30:29 -0400 (EDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=iro.umontreal.ca; s=mail; t=1584563429; bh=fRwut8VKYduDkQgeRoufjyEthOj5/7+n12ZOsvu8wWQ=; h=From:To:Subject:Date:From; b=HubWQ7psR9Kza8OwukVjsDLzvgJ7Ght/f02NFKHSVgyS3oqju0nV9kfqIvVaFJ1eS F5EY6kiWmh9QZsiTA4kFXv45w4PGZmdJDpRSFjk7eOwA/tnumXh5EO9Rvqmi0iqcBc kh3oHnwT1bsLHFdNufc3XkM4zgr5e6FV3oUJnvp53dVatAD81DtGSfNGkVSvm7VAhz SbHepfuvoZ1/oD75ldE0DbDJsab2H6Vfg4n029ZRL6GDlUbzqaGFO0zRRF8/amnkn1 Kf0v4XnwxdIiQ4YFr7Wg7S0mOcOLoAPuZi/G7mlEG3edF2ECG++n6DmgDDgtj7N/f9 dcdHeHox7IsHg== Original-Received: from alfajor (unknown [104.247.241.114]) by mail01.iro.umontreal.ca (Postfix) with ESMTPSA id 4563412029C; Wed, 18 Mar 2020 16:30:29 -0400 (EDT) X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] X-Received-From: 132.204.25.50 X-BeenThere: emacs-devel@gnu.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: "Emacs development discussions." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: emacs-devel-bounces+ged-emacs-devel=m.gmane-mx.org@gnu.org Original-Sender: "Emacs-devel" Xref: news.gmane.io gmane.emacs.devel:245572 Archived-At: 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 =E2=80=A6)) (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; } =20 +/* 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 =3D NULL; + union specbinding *pdl =3D 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 =3D 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_topleve= l_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 (( ...)) ...( ...)....) + and where the `foo` package only gets loaded when + is called, so the outer `let` incorrectly made the binding lexical + because the wasn't defined yet at that point. */ + error ("Defining as dynamic an already lexical var"); + XSYMBOL (symbol)->u.s.declared_special =3D true; if (!NILP (doc)) {