From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.io!.POSTED.blaine.gmane.org!not-for-mail From: akater Newsgroups: gmane.emacs.devel Subject: Re: [PATCH] Improve detection of local function calls in methods Date: Sun, 29 Aug 2021 11:25:07 +0000 Message-ID: <87zgt0sdbg.fsf@gmail.com> References: <878s0mva4u.fsf@gmail.com> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="=-=-=" Injection-Info: ciao.gmane.io; posting-host="blaine.gmane.org:116.202.254.214"; logging-data="8079"; mail-complaints-to="usenet@ciao.gmane.io" Cc: emacs-devel@gnu.org To: Stefan Monnier Original-X-From: emacs-devel-bounces+ged-emacs-devel=m.gmane-mx.org@gnu.org Sun Aug 29 13:36:59 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 1mKJ7T-0001mO-De for ged-emacs-devel@m.gmane-mx.org; Sun, 29 Aug 2021 13:36:59 +0200 Original-Received: from localhost ([::1]:43924 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1mKJ7R-00075E-Bs for ged-emacs-devel@m.gmane-mx.org; Sun, 29 Aug 2021 07:36:57 -0400 Original-Received: from eggs.gnu.org ([2001:470:142:3::10]:46656) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1mKJ6r-0006Qb-Ty for emacs-devel@gnu.org; Sun, 29 Aug 2021 07:36:21 -0400 Original-Received: from mail-ej1-x62b.google.com ([2a00:1450:4864:20::62b]:39743) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1mKJ6q-0004vK-4c for emacs-devel@gnu.org; Sun, 29 Aug 2021 07:36:21 -0400 Original-Received: by mail-ej1-x62b.google.com with SMTP id a25so24475334ejv.6 for ; Sun, 29 Aug 2021 04:36:19 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=from:to:cc:subject:in-reply-to:references:date:message-id :mime-version; bh=9Op7xhli0NOt7KFiadg3eDPXnLCzatwGlsc2maIQr3w=; b=qN0GZUC0oZBFyA9nqIl3x759cApdzAc17NKOjnS9ZaVWII8AFf0j2+trVsjK6ecA4r jgR3rk6DjvngeZf6mQ9QMPU2yIObPoRgelrMOMtfPV6pPAmekJ4XWpdqNEAtt1Eqs9Rj chZ8ZNk5yQ0wtdaAq77x5oiT+UHQeb9UWhacxmLqKe8k1/FCHyy/4zR1mrIFCoAB2+G6 LR29RCRv66a+NHmczW0h+yLmofc/mQOE8f6BDjdZD/Tpbkc0lEAuUROLaryu9mgFH/ru AykeORbXEHXTelNEztEU/6H94abOn6rHitB+nff+KMUlXwZDtL1EhdhAiYpRSdR9xpZh S5lA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:in-reply-to:references:date :message-id:mime-version; bh=9Op7xhli0NOt7KFiadg3eDPXnLCzatwGlsc2maIQr3w=; b=cs2jv7ECUe1T45HZOaIA80yDITd/HvrVlfE0Z3k1ZJbvbKGFv7V2/8I3SZMYgsGC4c dJJdskzgQ+hzL5sDWmsSyaxAmX3Wz1xT1HDrcb7oVq1+wrZDbk9rARLpSpNJH5DG95Bq fh5W206R4DYyET6d5T+mqkftXCe2s6Q3izaNeNSnUBTG1fDnYtEM1gUq48mVQvHz3S6w qeK3Rfvl5nhFA1Fm7ccUaedtY/lCKiIj1bR49/KTh3RAFxNmJAToYZHDYiNbWfJGRgOw 7JF00zZ9ZehpM2AvxRXTb89twXZ4DdS8HMs/ydoBJ2HRR5qzKcpOHsPWBBgV+ON3UXOP V0MQ== X-Gm-Message-State: AOAM533bBWshxA1QsMNn8rSgmZoiAR3itkV5DJ96dJzND9WSGdQvfc8n f3mq+8Ig52RsPv917PzreZU= X-Google-Smtp-Source: ABdhPJxL5Rr7ajvG5hkDoxPR1t2CmyqlIBsvUSEzSzBf90FCR6k9Q4JVSsrmFOYJMRdiyu85It+zwg== X-Received: by 2002:a17:906:2718:: with SMTP id z24mr19244803ejc.170.1630236978149; Sun, 29 Aug 2021 04:36:18 -0700 (PDT) Original-Received: from localhost (tor-exit-relay-4.anonymizing-proxy.digitalcourage.de. [185.220.102.250]) by smtp.googlemail.com with ESMTPSA id m6sm1004180edi.10.2021.08.29.04.36.15 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Sun, 29 Aug 2021 04:36:16 -0700 (PDT) In-Reply-To: Received-SPF: pass client-ip=2a00:1450:4864:20::62b; envelope-from=nuclearspace@gmail.com; helo=mail-ej1-x62b.google.com X-Spam_score_int: -7 X-Spam_score: -0.8 X-Spam_bar: / X-Spam_report: (-0.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_FROM=0.001, RCVD_IN_BL_SPAMCOP_NET=1.347, RCVD_IN_DNSWL_NONE=-0.0001, SPF_HELO_NONE=0.001, SPF_PASS=-0.001 autolearn=no 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:273403 Archived-At: --=-=-= Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable Stefan Monnier writes: > Hmm... IIUC this fails to account for the case where > #'cl-call-next-method is passed to a function (the most common case (or > more precisely, the only case I've seen so far) being when it's passed > to `apply`). Right. cl-flet should have reminded me that cl-call-next-method is a local function, not a local macro. > [ I'm not completely sure what you mean, but (defvar foo) has an effect I was referring to hypothetical #+begin_example elisp (let* (... cl-generic--uses-crm) (declare (special cl-generic--uses-crm)) ...) #+end_example But it doesn't matter anymore. See the new patches. Notes: - I splitted patches in two because the first diff looks significantly better this way. - Normally I'd put uses-cnm binding/declaration right before where it's used. However, putting it earlier gives a cleaner diff; also, uses-cnm means =E2=80=9Cparsed body uses cnm=E2=80=9D so I think it's OK = to put it right on the same line as (parsed-body ..), and this also utilizes the whitespace better. I don't know if all this is appropriate style; I provide patches this way in the hope it's acceptable. --=-=-= Content-Type: text/x-diff Content-Disposition: attachment; filename=0001-Improve-detection-of-local-function-calls-in-methods.patch Content-Description: cl--generic-lambda fix >From 2fd4d66a93831a63d19a8ab2efb927136f196beb Mon Sep 17 00:00:00 2001 From: akater Date: Thu, 26 Aug 2021 06:09:07 +0000 Subject: [PATCH 1/2] Improve detection of local function calls in methods * lisp/emacs-lisp/cl-generic.el (cl--generic-lambda): Rather than `grep' after the fact, the macroexpansion records directly when cl-call-next-method or cl-next-method-p are used. --- lisp/emacs-lisp/cl-generic.el | 51 ++++++++++++++++++++++++++--------- 1 file changed, 39 insertions(+), 12 deletions(-) diff --git a/lisp/emacs-lisp/cl-generic.el b/lisp/emacs-lisp/cl-generic.el index 4a69df15bc..8fdd905785 100644 --- a/lisp/emacs-lisp/cl-generic.el +++ b/lisp/emacs-lisp/cl-generic.el @@ -369,7 +369,7 @@ defun cl--generic-lambda (args body) (macroenv (cons `(cl-generic-current-method-specializers . ,(lambda () spec-args)) macroexpand-all-environment))) - (require 'cl-lib) ;Needed to expand `cl-flet' and `cl-function'. + (require 'cl-lib) ;Needed to expand `cl-function' and body. (when (interactive-form (cadr fun)) (message "Interactive forms unsupported in generic functions: %S" (interactive-form (cadr fun)))) @@ -377,24 +377,51 @@ defun cl--generic-lambda (args body) ;; destructuring args, `declare' and whatnot). (pcase (macroexpand fun macroenv) (`#'(lambda ,args . ,body) - (let* ((parsed-body (macroexp-parse-body body)) + (let* ((parsed-body (macroexp-parse-body body)) uses-cnm (cnm (make-symbol "cl--cnm")) (nmp (make-symbol "cl--nmp")) - (nbody (macroexpand-all - `(cl-flet ((cl-call-next-method ,cnm) - (cl-next-method-p ,nmp)) - ,@(cdr parsed-body)) - macroenv)) - ;; FIXME: Rather than `grep' after the fact, the - ;; macroexpansion should directly set some flag when cnm - ;; is used. + (nbody + ;; We duplicate the code from `cl-flet' augmenting it + ;; with `cl-pushnew' forms to record the presence of + ;; `cl-call-next-method', `cl-next-method-p'. + ;; It would be better to avoid code duplication + ;; but it's not clear how to do that reasonably enough. + (let ((newenv + (cons `(cl-call-next-method + . + ,(lambda (&rest args) + (cl-pushnew cnm uses-cnm :test #'eq) + (if (eq (car args) cl--labels-magic) + (list cl--labels-magic cnm) + `(funcall ,cnm ,@args)))) + (cons `(cl-next-method-p + . + ,(lambda (&rest args) + (cl-pushnew nmp uses-cnm :test #'eq) + (if (eq (car args) cl--labels-magic) + (list cl--labels-magic nmp) + `(funcall ,nmp ,@args)))) + macroenv)))) + (macroexpand-all + `(progn ,@(cdr parsed-body)) + ;; Don't override lexical-let's macro-expander + (if (assq 'function newenv) newenv + (cons (cons 'function + (lambda (f) + (cl-case f + (cl-call-next-method + (cl-pushnew cnm uses-cnm :test #'eq)) + (cl-next-method-p + (cl-pushnew nmp uses-cnm :test #'eq))) + (cl--labels-convert f))) + newenv))))) ;; FIXME: Also, optimize the case where call-next-method is ;; only called with explicit arguments. - (uses-cnm (macroexp--fgrep `((,cnm) (,nmp)) nbody))) + (uses-cnm uses-cnm)) (cons (not (not uses-cnm)) `#'(lambda (,@(if uses-cnm (list cnm)) ,@args) ,@(car parsed-body) - ,(if (not (assq nmp uses-cnm)) + ,(if (not (memq nmp uses-cnm)) nbody `(let ((,nmp (lambda () (cl--generic-isnot-nnm-p ,cnm)))) -- 2.31.1 --=-=-= Content-Type: text/x-diff Content-Disposition: attachment; filename=0002-lisp-emacs-lisp-cl-generic.el-cl-generic-lambda-Clea.patch Content-Description: cl--generic-lambda cleanup >From 9d506f6bdee01bce3dfe2fed8d159dfd4ce9d0ef Mon Sep 17 00:00:00 2001 From: akater Date: Sun, 29 Aug 2021 10:38:12 +0000 Subject: [PATCH 2/2] ; * lisp/emacs-lisp/cl-generic.el (cl--generic-lambda): Cleanup --- lisp/emacs-lisp/cl-generic.el | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lisp/emacs-lisp/cl-generic.el b/lisp/emacs-lisp/cl-generic.el index 8fdd905785..fd57599b1e 100644 --- a/lisp/emacs-lisp/cl-generic.el +++ b/lisp/emacs-lisp/cl-generic.el @@ -415,9 +415,9 @@ defun cl--generic-lambda (args body) (cl-pushnew nmp uses-cnm :test #'eq))) (cl--labels-convert f))) newenv))))) - ;; FIXME: Also, optimize the case where call-next-method is + ;; FIXME: Optimize the case where call-next-method is ;; only called with explicit arguments. - (uses-cnm uses-cnm)) + ) (cons (not (not uses-cnm)) `#'(lambda (,@(if uses-cnm (list cnm)) ,@args) ,@(car parsed-body) -- 2.31.1 --=-=-=--