From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.io!.POSTED.blaine.gmane.org!not-for-mail From: Naoya Yamashita Newsgroups: gmane.emacs.devel Subject: Re: [PATCH] * src/eval.c: Stop checking for nvars, and use only CONSP Date: Tue, 02 Mar 2021 12:09:29 +0900 (JST) Message-ID: <20210302.120929.1656994339284323372.conao3@gmail.com> References: <20210302.111043.609653289571449353.conao3@gmail.com> Mime-Version: 1.0 Content-Type: Multipart/Mixed; boundary="--Next_Part(Tue_Mar__2_12_09_29_2021_773)--" Content-Transfer-Encoding: 7bit Injection-Info: ciao.gmane.io; posting-host="blaine.gmane.org:116.202.254.214"; logging-data="31747"; mail-complaints-to="usenet@ciao.gmane.io" Cc: emacs-devel@gnu.org To: monnier@iro.umontreal.ca Original-X-From: emacs-devel-bounces+ged-emacs-devel=m.gmane-mx.org@gnu.org Tue Mar 02 04:21:53 2021 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 1lGvbd-00088V-8M for ged-emacs-devel@m.gmane-mx.org; Tue, 02 Mar 2021 04:21:53 +0100 Original-Received: from localhost ([::1]:54050 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1lGvbc-0005rc-4k for ged-emacs-devel@m.gmane-mx.org; Mon, 01 Mar 2021 22:21:52 -0500 Original-Received: from eggs.gnu.org ([2001:470:142:3::10]:36710) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1lGva2-0005IX-2q for emacs-devel@gnu.org; Mon, 01 Mar 2021 22:20:14 -0500 Original-Received: from mail-pj1-x102d.google.com ([2607:f8b0:4864:20::102d]:51752) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1lGvZo-0005hx-KC for emacs-devel@gnu.org; Mon, 01 Mar 2021 22:20:13 -0500 Original-Received: by mail-pj1-x102d.google.com with SMTP id jx13so972696pjb.1 for ; Mon, 01 Mar 2021 19:19:57 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=date:message-id:to:cc:subject:from:in-reply-to:references :mime-version:content-transfer-encoding; bh=fRbpM3rX+UXt6zYjL3Zf5bxxeShv+Z9/OrYCZQ5QJ/w=; b=PjMsIF/k7786BRKYquxOZ1ulC76PCwSZF96yktoPnplv/bDLQFzyysHyIcFsc95M88 gaILkXNBheW1hDCvK/C/n/9MRgD6KfHiObayF5tC8Yg0G++JE2wBdvUnp7ScKPA3nXWs Fn8kWCifM8ALNLx9eE+cqFYGXxESqVc/XMonsfXKrbllENSyxv4ubZOeFx7x32pEe/M3 ZjpAuRMV5FoDzMIq0n42YquMZsPlhFfjBiBpzz/NJq7JkI1tocsSM7qJSdb6YB2Z+WJC zFBPZYoL+vgUg9DyGvf4MuT0mBOm4nPUKq8KO/qFratrJkrhytagZ9fSJABHDOXJFXub lkYw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:message-id:to:cc:subject:from:in-reply-to :references:mime-version:content-transfer-encoding; bh=fRbpM3rX+UXt6zYjL3Zf5bxxeShv+Z9/OrYCZQ5QJ/w=; b=WhZqTKB5nzRvQLJIOXFWfhAQFUpeFxTp0MDXG/FT7IsXIJQlyn51kRntGtA8NAv6xr b79+Pm+kzZBHkaU560fdLU4QhUy7fiylX8BKBcG8rkPGOc1lIL0aNH96mgQ0z0Rr7kyw QnPtHsG8fP8R1hAXExu5dwmLi+o5DacRl14oVpaEFjDD0qbLJ0kGeiEoSr/DFnkPoaHk NQTP2goyw8I4OACxWNAFENSn7Qb4xe0DWMTC3TDlHNb7ezM/gO68KvsYhvPeSFcYI2pc qJ0J4ZsgZSIRz5SUQqqTuRC3WgPTVkN0O1OiLcHof2ywEE5NeQoFzCd3eeFJ7Ev6c7Sz tHKQ== X-Gm-Message-State: AOAM531tGTFSclEGsFv1Kh9JWtFOnv9uqOdN8l4jxplIi5A8PpYPSd9w FlZgDh6vl6zKy5yPm8jMaEc= X-Google-Smtp-Source: ABdhPJyzuWTa3i64/XbMsPibz7d22+9eVyzPVOXFA3NWp4bYMfts4MJfkeNb3zmXxtqP70dBXmPFpw== X-Received: by 2002:a17:90a:d497:: with SMTP id s23mr2082124pju.148.1614655196513; Mon, 01 Mar 2021 19:19:56 -0800 (PST) Original-Received: from localhost (p210141-ipngn200407niho.hiroshima.ocn.ne.jp. [118.4.79.141]) by smtp.gmail.com with ESMTPSA id s62sm19708818pfb.148.2021.03.01.19.19.54 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 01 Mar 2021 19:19:55 -0800 (PST) In-Reply-To: X-Mailer: Mew version 6.8 on Emacs 27.1 Received-SPF: pass client-ip=2607:f8b0:4864:20::102d; envelope-from=conao3@gmail.com; helo=mail-pj1-x102d.google.com X-Spam_score_int: -17 X-Spam_score: -1.8 X-Spam_bar: - X-Spam_report: (-1.8 / 5.0 requ) BAYES_00=-1.9, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, DKIM_VALID_EF=-0.1, FREEMAIL_ENVFROM_END_DIGIT=0.25, FREEMAIL_FROM=0.001, RCVD_IN_DNSWL_NONE=-0.0001, SPF_HELO_NONE=0.001, SPF_PASS=-0.001 autolearn=ham autolearn_force=no X-Spam_action: no action 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:265798 Archived-At: ----Next_Part(Tue_Mar__2_12_09_29_2021_773)-- Content-Type: Text/Plain; charset=us-ascii Content-Transfer-Encoding: 7bit > The patch looks fine, thank you. Thanks! > Just one detail about the tests: the change you made affects the > evaluation of `let` in the case where the code is interpreted but it > is not used when the code is byte-compiled. So you might like your > tests to use things like > > (eval '(let ...) t) > > to avoid the compiler getting in the way. Thanks, I didn't notice this point! I fix testcases. > Also, while this `let` is indeed invalid code, I don't think we > guarantee that it will signal an error, and especially not at runtime > (it's more likely to signal an error at macroexpansion or compile > time). > > I think the compiler (or `macroexpand-all`) should make an effort to > detect and diagnose those problems, but I don't think it's important to > catch those problems in the interpreter. That testcase comes from this code (src/eval.c:L1014) which we already had. else if (! NILP (Fcdr (Fcdr (elt)))) signal_error ("`let' bindings can have only one value-form", elt); I tried to remove this, my temp Emacs works like this in *scratch* buffer. (let ((a 1 2)) a) ; Type C-j 1 This is very strange I think. I still think it's important for Emacs, even as an interpreter, to produce errors. ----Next_Part(Tue_Mar__2_12_09_29_2021_773)-- Content-Type: Text/X-Patch; charset=us-ascii Content-Transfer-Encoding: 7bit Content-Disposition: inline; filename="0001-src-eval.c-Stop-checking-for-nvars-and-use-only-CONS.patch" >From 8e65fea2d95bf7118ecd2b816f3b49292353a431 Mon Sep 17 00:00:00 2001 From: Naoya Yamashita Date: Tue, 2 Mar 2021 10:17:29 +0900 Subject: [PATCH] * src/eval.c: Stop checking for nvars, and use only CONSP * src/eval.c (let): Remove checking nvars (length of arglist), and use only CONSP check. --- src/eval.c | 6 ++---- test/src/eval-tests.el | 33 +++++++++++++++++++++++++++++++++ 2 files changed, 35 insertions(+), 4 deletions(-) diff --git a/src/eval.c b/src/eval.c index 542d7f686e..30783f204a 100644 --- a/src/eval.c +++ b/src/eval.c @@ -1001,11 +1001,10 @@ DEFUN ("let", Flet, Slet, 1, UNEVALLED, 0, /* Make space to hold the values to give the bound variables. */ EMACS_INT varlist_len = list_length (varlist); SAFE_ALLOCA_LISP (temps, varlist_len); - ptrdiff_t nvars = varlist_len; /* Compute the values and store them in `temps'. */ - for (argnum = 0; argnum < nvars && CONSP (varlist); argnum++) + for (argnum = 0; CONSP (varlist); argnum++) { maybe_quit (); elt = XCAR (varlist); @@ -1017,12 +1016,11 @@ DEFUN ("let", Flet, Slet, 1, UNEVALLED, 0, else temps[argnum] = eval_sub (Fcar (Fcdr (elt))); } - nvars = argnum; lexenv = Vinternal_interpreter_environment; varlist = XCAR (args); - for (argnum = 0; argnum < nvars && CONSP (varlist); argnum++) + for (argnum = 0; CONSP (varlist); argnum++) { Lisp_Object var; diff --git a/test/src/eval-tests.el b/test/src/eval-tests.el index b2b7dfefda..85dc2a53ae 100644 --- a/test/src/eval-tests.el +++ b/test/src/eval-tests.el @@ -226,4 +226,37 @@ eval-tests/backtrace-in-batch-mode/demoted-errors (should (equal (string-trim (buffer-string)) "Error: (error \"Boo\")"))))) +(ert-deftest eval-tests/let () + (should (equal (eval '(let (a) + a) + t) + nil)) + + (should (equal (eval '(let (a b) + (list a b)) + t) + '(nil nil))) + + (should (equal (eval '(let ((a 1)) + a) + t) + 1)) + + (should (equal (eval '(let ((a 1) b) + (list a b)) + t) + '(1 nil))) + + ;; (error "`let' bindings can have only one value-form" a 1 2) + (should-error (eval '(let ((a 1 2)) + a) + t) + :type 'error) + + ;; (wrong-type-argument symbolp (a)) + (should-error (eval '(let (((a) 1)) + a) + t) + :type 'wrong-type-argument)) + ;;; eval-tests.el ends here -- 2.30.1 ----Next_Part(Tue_Mar__2_12_09_29_2021_773)----