From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.org!.POSTED.blaine.gmane.org!not-for-mail From: Stefan Monnier Newsgroups: gmane.emacs.devel Subject: Re: eieio-base patch to support EDE project loading Date: Fri, 04 Oct 2019 16:45:18 -0400 Message-ID: References: <8d7cb453-41c9-f3eb-8395-01cb46079663@siege-engine.com> Mime-Version: 1.0 Content-Type: text/plain Injection-Info: blaine.gmane.org; posting-host="blaine.gmane.org:195.159.176.226"; logging-data="161013"; mail-complaints-to="usenet@blaine.gmane.org" User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/27.0.50 (gnu/linux) Cc: Emacs Development To: Eric Ludlam Original-X-From: emacs-devel-bounces+ged-emacs-devel=m.gmane.org@gnu.org Fri Oct 04 22:45:47 2019 Return-path: Envelope-to: ged-emacs-devel@m.gmane.org Original-Received: from lists.gnu.org ([209.51.188.17]) by blaine.gmane.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.89) (envelope-from ) id 1iGUSQ-000fdz-BW for ged-emacs-devel@m.gmane.org; Fri, 04 Oct 2019 22:45:46 +0200 Original-Received: from localhost ([::1]:53186 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1iGUSO-0006Hz-R9 for ged-emacs-devel@m.gmane.org; Fri, 04 Oct 2019 16:45:44 -0400 Original-Received: from eggs.gnu.org ([2001:470:142:3::10]:35321) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1iGUS8-0006Ec-Si for emacs-devel@gnu.org; Fri, 04 Oct 2019 16:45:30 -0400 Original-Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1iGUS7-0002ag-2u for emacs-devel@gnu.org; Fri, 04 Oct 2019 16:45:28 -0400 Original-Received: from mailscanner.iro.umontreal.ca ([132.204.25.50]:18325) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1iGUS6-0002ZO-RG for emacs-devel@gnu.org; Fri, 04 Oct 2019 16:45:27 -0400 Original-Received: from pmg3.iro.umontreal.ca (localhost [127.0.0.1]) by pmg3.iro.umontreal.ca (Proxmox) with ESMTP id BDECC448A82; Fri, 4 Oct 2019 16:45:25 -0400 (EDT) Original-Received: from mail01.iro.umontreal.ca (unknown [172.31.2.1]) by pmg3.iro.umontreal.ca (Proxmox) with ESMTP id BFE95448A7E; Fri, 4 Oct 2019 16:45:19 -0400 (EDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=iro.umontreal.ca; s=mail; t=1570221919; bh=VMcku/ZxrTXJMT9Ko1xpDSYSxYWF8Z3n2gI8oNecyvk=; h=From:To:Cc:Subject:References:Date:In-Reply-To:From; b=MrpmGL8n4HwpnPxnqvdIYtsulF2+3aXXYRhLBqzcW08u+PZZWK/9aeWa5y3iuPtZo +xxqh8G8Cq6Zte8iXKmXnEERSXLuvA32xrntVLq7drTKnQPPIjbD9Ivk9yO/r9iWy6 U/378ILdIfHn+gdJ5cVkYPQm7pDXdRH1u4CeU9T5339YukfJrNwNkEBAoF4OnGKw+U Kq4mrtCTG4Za4CpalKYGWa/7UYaCugI+Cchf4rjrqcy6ur9s257Edf4AnHaY24sdwb BW8SXxbNiBDj+aom8MQtH0ym8uxxgOqmMudK59d1VnL+L6jdIq2rVuZVvqTCIypaPR MmHyIIs5H7n8w== Original-Received: from alfajor (modemcable157.163-203-24.mc.videotron.ca [24.203.163.157]) by mail01.iro.umontreal.ca (Postfix) with ESMTPSA id 892AE1204BD; Fri, 4 Oct 2019 16:45:19 -0400 (EDT) In-Reply-To: <8d7cb453-41c9-f3eb-8395-01cb46079663@siege-engine.com> (Eric Ludlam's message of "Mon, 30 Sep 2019 22:18:24 -0400") X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] [fuzzy] 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.org@gnu.org Original-Sender: "Emacs-devel" Xref: news.gmane.org gmane.emacs.devel:240585 Archived-At: > Without that list of superclasses, `child-of-class-p' fails. Since having > the superclass list in autoloads is a non-starter, this patch will > un-autoload classes found in the file being read before the call to > `child-of-class-p'. Looks good, but I felt like those un-autoloads scattered around felt a bit haphazard (with tests slightly different each time), so I suggest the patch below instead, WDYT (which also moves the un-autoload to child-of-class-p so it applies not just to eieio-persistent-validate/fix-slot-value but to all users of child-of-class-p). > It would be better to store the superclasses in the autoload declaration so > that extensible class hierarchies can be more dynamically loaded without > internal checks with un-autoload, but I don't know enough about how the new > version of eieio works to provide that patch. It would allow child-of-class-p to be computed without loading the class, hence a bit more laziness, but I'm wondering how often it would make a real difference: how likely are you to use child-of-class-p without either already having an instance of that class or being right about to build such an instance? Can you test the patch below before I push it? Stefan diff --git a/lisp/emacs-lisp/eieio-base.el b/lisp/emacs-lisp/eieio-base.el index 534613811d..fc78b3e098 100644 --- a/lisp/emacs-lisp/eieio-base.el +++ b/lisp/emacs-lisp/eieio-base.el @@ -277,8 +277,7 @@ eieio-persistent-convert-list-to-object (progn ;; If OBJCLASS is an eieio autoload object, then we need to ;; load it. - (eieio-class-un-autoload objclass) - (eieio--class-object objclass)))) + (eieio--full-class-object objclass)))) (while slots (let ((initarg (car slots)) diff --git a/lisp/emacs-lisp/eieio-core.el b/lisp/emacs-lisp/eieio-core.el index 620b47e68d..3d5d4765f1 100644 --- a/lisp/emacs-lisp/eieio-core.el +++ b/lisp/emacs-lisp/eieio-core.el @@ -125,7 +125,8 @@ eieio--object-class-tag (defsubst eieio--class-object (class) "Return the class object." (if (symbolp class) - ;; Keep the symbol if class-v is nil, for better error messages. + ;; Keep the symbol if the class object doesn't exist, + ;; for better error messages. (or (cl--find-class class) class) class)) @@ -217,10 +218,6 @@ eieio-defclass-autoload (make-obsolete-variable cname (format "use \\='%s instead" cname) "25.1")) - ;; Store the new class vector definition into the symbol. We need to - ;; do this first so that we can call defmethod for the accessor. - ;; The vector will be updated by the following while loop and will not - ;; need to be stored a second time. (setf (cl--find-class cname) newc) ;; Create an autoload on top of our constructor function. @@ -230,9 +227,17 @@ eieio-defclass-autoload (autoload (intern (format "%s-child-p" cname)) filename "" nil nil) (autoload (intern (format "%s-list-p" cname)) filename "" nil nil))))) -(defsubst eieio-class-un-autoload (cname) - "If class CNAME is in an autoload state, load its file." - (autoload-do-load (symbol-function cname))) ; cname +(defun eieio--full-class-object (class) + "Like `eieio--class-object' but loads the class if needed." + (let ((c (eieio--class-object class))) + (and (not (symbolp c)) + ;; If the default-object-cache slot is nil, the class object + ;; is still a "dummy" setup by eieio-defclass-autoload. + (not (eieio--class-default-object-cache c)) + ;; FIXME: We rely on the autoload setup for the "standard" + ;; constructor, here! + (autoload-do-load (symbol-function (eieio--class-name c)))) + c)) (cl-deftype list-of (elem-type) `(and list @@ -730,9 +735,7 @@ eieio-oref (cl-check-type obj (or eieio-object class)) (let* ((class (cond ((symbolp obj) (error "eieio-oref called on a class: %s" obj) - (let ((c (cl--find-class obj))) - (if (eieio--class-p c) (eieio-class-un-autoload obj)) - c)) + (eieio--full-class-object obj)) (t (eieio--object-class obj)))) (c (eieio--slot-name-index class slot))) (if (not c) @@ -1013,16 +1016,15 @@ eieio--class-precedence-list method invocation orders of the involved classes." (if (or (null class) (eq class eieio-default-superclass)) nil - (unless (eieio--class-default-object-cache class) - (eieio-class-un-autoload (eieio--class-name class))) - (cl-case (eieio--class-method-invocation-order class) - (:depth-first - (eieio--class-precedence-dfs class)) - (:breadth-first - (eieio--class-precedence-bfs class)) - (:c3 - (eieio--class-precedence-c3 class)))) - ) + (let ((class (eieio--full-class-object class))) + (cl-case (eieio--class-method-invocation-order class) + (:depth-first + (eieio--class-precedence-dfs class)) + (:breadth-first + (eieio--class-precedence-bfs class)) + (:c3 + (eieio--class-precedence-c3 class)))))) + (define-obsolete-function-alias 'class-precedence-list 'eieio--class-precedence-list "24.4") diff --git a/lisp/emacs-lisp/eieio.el b/lisp/emacs-lisp/eieio.el index 4b899cdc64..103c02f795 100644 --- a/lisp/emacs-lisp/eieio.el +++ b/lisp/emacs-lisp/eieio.el @@ -468,7 +468,7 @@ 'obj-of-class-p (defun child-of-class-p (child class) "Return non-nil if CHILD class is a subclass of CLASS." - (setq child (eieio--class-object child)) + (setq child (eieio--full-class-object child)) (cl-check-type child eieio--class) ;; `eieio-default-superclass' is never mentioned in eieio--class-parents, ;; so we have to special case it here.