EIEIO with lexical scoping

EIEIO with lexical scoping

[Stefan Monnier]

I'm trying to compile eieio.el using lexical-binding but am bumping into
a problem that requires too much internal knowledge of eieio for me.

I'm using the patch appended below for now, which seems to work to some
extent, but when I then try to compile CEDET using it, I get the
following backtrace:

Debugger entered--Lisp error: (invalid-slot-name "#<semanticdb-project-database default-cache-object>" tables)
  signal(invalid-slot-name ("#<semanticdb-project-database default-cache-object>" tables))
  eieio-default-superclass([object semanticdb-project-database default-cache-object unbound semanticdb-table nil nil] tables oref-default)
  apply(eieio-default-superclass ([object semanticdb-project-database default-cache-object unbound semanticdb-table nil nil] tables oref-default))
  eieio-generic-call-primary-only(slot-missing ([object semanticdb-project-database default-cache-object unbound semanticdb-table nil nil] tables oref-default))
  slot-missing([object semanticdb-project-database default-cache-object unbound semanticdb-table nil nil] tables oref-default)
  eieio-oref-default([object semanticdb-project-database default-cache-object unbound semanticdb-table nil nil] tables)
  eieio-set-defaults([object semanticdb-project-database default-cache-object unbound semanticdb-table nil nil] t)
  eieio-defclass(semanticdb-project-database (eieio-instance-tracker) ((tracking-symbol :initform semanticdb-database-list) (reference-directory :type string :documentation "Directory this database refers to.\nWhen a cache directory is specified, then this refers to the directory\nthis database contains symbols for.") (new-table-class :initform semanticdb-table :type class :documentation "New tables created for this database are of this class.") (cache :type list :initform nil :documentation "List of cache information for tools.\nAny particular tool can cache data to a database at runtime\nwith `semanticdb-cache-get'.\n\nUsing a semanticdb cache does not save any information to a file,\nso your cache will need to be recalculated at runtime.\n\nNote: This index will not be saved in a persistent file.") (tables :initarg :tables :type semanticdb-abstract-table-list :accessor semanticdb-get-database-tables :protection :protected :documentation "List of `semantic-db-table' objects.")) ("Database of file tables."))
  (progn (eieio-defclass 'semanticdb-project-database '(eieio-instance-tracker) '((tracking-symbol :initform semanticdb-database-list) (reference-directory :type string :documentation "Directory this database refers to.\nWhen a cache directory is specified, then this refers to the directory\nthis database contains symbols for.") (new-table-class :initform semanticdb-table :type class :documentation "New tables created for this database are of this class.") (cache :type list :initform nil :documentation "List of cache information for tools.\nAny particular tool can cache data to a database at runtime\nwith `semanticdb-cache-get'.\n\nUsing a semanticdb cache does not save any information to a file,\nso your cache will need to be recalculated at runtime.\n\nNote: This index will not be saved in a persistent file.") (tables :initarg :tables :type semanticdb-abstract-table-list :accessor semanticdb-get-database-tables :protection :protected :documentation "List of `semantic-db-table' objects.")) '("Database of file tables.")))
  eval((progn (eieio-defclass 'semanticdb-project-database '(eieio-instance-tracker) '((tracking-symbol :initform semanticdb-database-list) (reference-directory :type string :documentation "Directory this database refers to.\nWhen a cache directory is specified, then this refers to the directory\nthis database contains symbols for.") (new-table-class :initform semanticdb-table :type class :documentation "New tables created for this database are of this class.") (cache :type list :initform nil :documentation "List of cache information for tools.\nAny particular tool can cache data to a database at runtime\nwith `semanticdb-cache-get'.\n\nUsing a semanticdb cache does not save any information to a file,\nso your cache will need to be recalculated at runtime.\n\nNote: This index will not be saved in a persistent file.") (tables :initarg :tables :type semanticdb-abstract-table-list :accessor semanticdb-get-database-tables :protection :protected :documentation "List of `semantic-db-table' objects.")) '("Database of file tables."))) nil)
  #[128 "\301\302\303B\b\"D\207" [lexical-binding quote eval progn] 5 "\n\n(fn &rest BODY)"]((eieio-defclass 'semanticdb-project-database '(eieio-instance-tracker) '((tracking-symbol :initform semanticdb-database-list) (reference-directory :type string :documentation "Directory this database refers to.\nWhen a cache directory is specified, then this refers to the directory\nthis database contains symbols for.") (new-table-class :initform semanticdb-table :type class :documentation "New tables created for this database are of this class.") (cache :type list :initform nil :documentation "List of cache information for tools.\nAny particular tool can cache data to a database at runtime\nwith `semanticdb-cache-get'.\n\nUsing a semanticdb cache does not save any information to a file,\nso your cache will need to be recalculated at runtime.\n\nNote: This index will not be saved in a persistent file.") (tables :initarg :tables :type semanticdb-abstract-table-list :accessor semanticdb-get-database-tables :protection :protected :documentation "List of `semantic-db-table' objects.")) '("Database of file tables.")))
  (eval-and-compile (eieio-defclass 'semanticdb-project-database '(eieio-instance-tracker) '((tracking-symbol :initform semanticdb-database-list) (reference-directory :type string :documentation "Directory this database refers to.\nWhen a cache directory is specified, then this refers to the directory\nthis database contains symbols for.") (new-table-class :initform semanticdb-table :type class :documentation "New tables created for this database are of this class.") (cache :type list :initform nil :documentation "List of cache information for tools.\nAny particular tool can cache data to a database at runtime\nwith `semanticdb-cache-get'.\n\nUsing a semanticdb cache does not save any information to a file,\nso your cache will need to be recalculated at runtime.\n\nNote: This index will not be saved in a persistent file.") (tables :initarg :tables :type semanticdb-abstract-table-list :accessor semanticdb-get-database-tables :protection :protected :documentation "List of `semantic-db-table' objects.")) '("Database of file tables.")))
  (defclass semanticdb-project-database (eieio-instance-tracker) ((tracking-symbol :initform semanticdb-database-list) (reference-directory :type string :documentation "Directory this database refers to.\nWhen a cache directory is specified, then this refers to the directory\nthis database contains symbols for.") (new-table-class :initform semanticdb-table :type class :documentation "New tables created for this database are of this class.") (cache :type list :initform nil :documentation "List of cache information for tools.\nAny particular tool can cache data to a database at runtime\nwith `semanticdb-cache-get'.\n\nUsing a semanticdb cache does not save any information to a file,\nso your cache will need to be recalculated at runtime.\n\nNote: This index will not be saved in a persistent file.") (tables :initarg :tables :type semanticdb-abstract-table-list :accessor semanticdb-get-database-tables :protection :protected :documentation "List of `semantic-db-table' objects.")) "Database of file tables.")
  eval-buffer(#<buffer  *load*-733491> nil "/home/monnier/src/emacs/work/lisp/cedet/semantic/db.el" nil t)  ; Reading at buffer position 12358662
  load-with-code-conversion("/home/monnier/src/emacs/work/lisp/cedet/semantic/db.el" "/home/monnier/src/emacs/work/lisp/cedet/semantic/db.el" nil t)
  require(semantic/db)
  eval-buffer(#<buffer  *load*-379672> nil "/home/monnier/src/emacs/work/lisp/cedet/semantic/db-find.el" nil t)  ; Reading at buffer position 12350271
  load-with-code-conversion("/home/monnier/src/emacs/work/lisp/cedet/semantic/db-find.el" "/home/monnier/src/emacs/work/lisp/cedet/semantic/db-find.el" nil t)
  require(semantic/db-find)
  (progn (require 'semantic/db-find) (require 'semantic/find))
  eval((progn (require 'semantic/db-find) (require 'semantic/find)) nil)
  #[128 "\301\302\303B\b\"D\207" [lexical-binding quote eval progn] 5 "\n\n(fn &rest BODY)"]((require 'semantic/db-find) (require 'semantic/find))
  (eval-when-compile (require 'semantic/db-find) (require 'semantic/find))
  eval-buffer(#<buffer  *load*-7472> nil "/home/monnier/src/emacs/work/lisp/cedet/semantic/util.el" nil t)  ; Reading at buffer position 12346843
  load-with-code-conversion("/home/monnier/src/emacs/work/lisp/cedet/semantic/util.el" "/home/monnier/src/emacs/work/lisp/cedet/semantic/util.el" nil t)
  require(semantic/util)
  eval-buffer(#<buffer  *load*-252040> nil "/home/monnier/src/emacs/work/lisp/cedet/semantic.el" nil t)  ; Reading at buffer position 12395975
  load-with-code-conversion("/home/monnier/src/emacs/work/lisp/cedet/semantic.el" "/home/monnier/src/emacs/work/lisp/cedet/semantic.el" nil t)
  require(semantic)
  eval-buffer(#<buffer  *load*> nil "/home/monnier/src/emacs/work/lisp/cedet/semantic/util.el" nil t)  ; Reading at buffer position 12346651
  load-with-code-conversion("/home/monnier/src/emacs/work/lisp/cedet/semantic/util.el" "/home/monnier/src/emacs/work/lisp/cedet/semantic/util.el" nil t)
  require(semantic/util)
  apply(require semantic/util)
  byte-compile-file-form-require((require 'semantic/util))
  byte-compile-file-form((require 'semantic/util))
  byte-compile-toplevel-file-form((require 'semantic/util))
  #[0 "r\300q\210	\203\0\306	\n\"\210eb\210\307\310\307w\210\311\312!\203\"\313y\210\202\0m\204B`\f\307\x13\314\300!\x13\2039\315\316!\210\317!)\266\202\0\320 \210d\321 )\210	\205Wr\nq\210\322	!)\207" [#<buffer  *Compiler Input*> byte-compile-current-file byte-compile--outbuffer byte-compile-unresolved-functions byte-compile-read-position byte-compile-last-position byte-compile-insert-header nil " 	\n\f" looking-at ";" 1 read byte-compile-warn "!! The file uses old-style backquotes !!\nThis functionality has been obsolete for more than 10 years already\nand will be removed soon.  See (elisp)Backquote in the manual." byte-compile-toplevel-file-form byte-compile-flush-pending byte-compile-warn-about-unresolved-functions byte-compile-fix-header old-style-backquotes] 3 "\n\n(fn)"]()
  byte-compile-from-buffer(#<buffer  *Compiler Input*>)
  byte-compile-file("/home/monnier/src/emacs/work/lisp/cedet/semantic.el")

Can you help me figure out what's going on?


        Stefan


=== modified file 'lisp/emacs-lisp/eieio.el'
--- lisp/emacs-lisp/eieio.el	2013-02-27 04:09:50 +0000
+++ lisp/emacs-lisp/eieio.el	2013-05-13 21:22:02 +0000
@@ -1,4 +1,4 @@
-;;; eieio.el --- Enhanced Implementation of Emacs Interpreted Objects
+;;; eieio.el --- Enhanced Implementation of Emacs Interpreted Objects  -*- lexical-binding:t -*-
 ;;;              or maybe Eric's Implementation of Emacs Interpreted Objects
 
 ;; Copyright (C) 1995-1996, 1998-2013 Free Software Foundation, Inc.
@@ -193,32 +193,31 @@
   ;; No check: If eieio gets this far, it's probably been checked already.
   `(get ,class 'eieio-class-definition))
 
-(defmacro class-p (class)
+(defsubst class-p (class)
   "Return t if CLASS is a valid class vector.
 CLASS is a symbol."
   ;; this new method is faster since it doesn't waste time checking lots of
   ;; things.
-  `(condition-case nil
-       (eq (aref (class-v ,class) 0) 'defclass)
+  (condition-case nil
+      (eq (aref (class-v class) 0) 'defclass)
      (error nil)))
 
-(defmacro eieio-object-p (obj)
+(defsubst eieio-object-p (obj)
   "Return non-nil if OBJ is an EIEIO object."
-  `(let ((tobj ,obj))
-     (and (vectorp tobj)
-          (eq (aref tobj 0) 'object)
-          (class-p (eieio--object-class tobj)))))
+  (and (vectorp obj)
+       (eq (aref obj 0) 'object)
+       (class-p (eieio--object-class obj))))
 (defalias 'object-p 'eieio-object-p)
 
 (defmacro class-constructor (class)
   "Return the symbol representing the constructor of CLASS."
   `(eieio--class-symbol (class-v ,class)))
 
-(defmacro generic-p (method)
+(defsubst generic-p (method)
   "Return t if symbol METHOD is a generic function.
 Only methods have the symbol `eieio-method-obarray' as a property
 \(which contains a list of all bindings to that method type.)"
-  `(and (fboundp ,method) (get ,method 'eieio-method-obarray)))
+  (and (fboundp method) (get method 'eieio-method-obarray)))
 
 (defun generic-primary-only-p (method)
   "Return t if symbol METHOD is a generic function with only primary methods.
@@ -261,10 +260,10 @@
 Return nil if that option doesn't exist."
   `(class-option-assoc (eieio--class-options (class-v ,class)) ',option))
 
-(defmacro class-abstract-p (class)
+(defsubst class-abstract-p (class)
   "Return non-nil if CLASS is abstract.
 Abstract classes cannot be instantiated."
-  `(class-option ,class :abstract))
+  (class-option class :abstract))
 
 (defmacro class-method-invocation-order (class)
   "Return the invocation order of CLASS.
@@ -501,7 +500,7 @@
 		    (setf (eieio--class-children (class-v (car pname)))
 			  (cons cname (eieio--class-children (class-v (car pname))))))
 		  ;; Get custom groups, and store them into our local copy.
-		  (mapc (lambda (g) (add-to-list 'groups g))
+		  (mapc (lambda (g) (pushnew g groups :test #'equal))
 			(class-option (car pname) :custom-groups))
 		  ;; save parent in child
 		  (setf (eieio--class-parent newc) (cons (car pname) (eieio--class-parent newc))))
@@ -680,7 +679,7 @@
 			     prot initarg alloc 'defaultoverride skip-nil)
 
 	;; We need to id the group, and store them in a group list attribute.
-	(mapc (lambda (cg) (add-to-list 'groups cg)) customg)
+	(mapc (lambda (cg) (pushnew cg groups :test #'equal)) customg)
 
 	;; Anyone can have an accessor function.  This creates a function
 	;; of the specified name, and also performs a `defsetf' if applicable
@@ -824,7 +823,7 @@
 
     ;; We have a list of custom groups.  Store them into the options.
     (let ((g (class-option-assoc options :custom-groups)))
-      (mapc (lambda (cg) (add-to-list 'g cg)) groups)
+      (mapc (lambda (cg) (pushnew cg g :test #'equal)) groups)
       (if (memq :custom-groups options)
 	  (setcar (cdr (memq :custom-groups options)) g)
 	(setq options (cons :custom-groups (cons g options)))))
@@ -856,10 +855,10 @@
 (defun eieio-perform-slot-validation-for-default (slot spec value skipnil)
   "For SLOT, signal if SPEC does not match VALUE.
 If SKIPNIL is non-nil, then if VALUE is nil return t instead."
-  (if (and (not (eieio-eval-default-p value))
-	   (not eieio-skip-typecheck)
-	   (not (and skipnil (null value)))
-	   (not (eieio-perform-slot-validation spec value)))
+  (if (not (or (eieio-eval-default-p value)
+	       eieio-skip-typecheck
+	       (and skipnil (null value))
+	       (eieio-perform-slot-validation spec value)))
       (signal 'invalid-slot-type (list slot spec value))))
 
 (defun eieio-add-new-slot (newc a d doc type cust label custg print prot init alloc
@@ -1425,10 +1424,8 @@
 	    (t
 	     (let* ((name (symbol-name type))
 		    (namep (intern (concat name "p"))))
-	       (if (fboundp namep)
-		   (funcall `(lambda () (,namep val)))
-		 (funcall `(lambda ()
-			     (,(intern (concat name "-p")) val)))))))
+	       (funcall (if (fboundp namep) namep (intern (concat name "-p")))
+                        val))))
     (cond ((get (car type) 'cl-deftype-handler)
 	   (eieio--typep val (apply (get (car type) 'cl-deftype-handler)
 				    (cdr type))))
@@ -1450,7 +1447,7 @@
 	  ((memq (car type) '(member member*))
 	   (memql val (cdr type)))
 	  ((eq (car type) 'satisfies)
-	   (funcall `(lambda () (,(cadr type) val))))
+	   (funcall (cadr type) val))
 	  (t (error "Bad type spec: %s" type)))))
 
 (defun eieio-perform-slot-validation (spec value)
@@ -1799,9 +1796,9 @@
   `(car (eieio-class-parents ,class)))
 (define-obsolete-function-alias 'class-parent #'eieio-class-parent "24.4")
 
-(defmacro same-class-fast-p (obj class)
+(defsubst same-class-fast-p (obj class)
   "Return t if OBJ is of class-type CLASS with no error checking."
-  `(eq (eieio--object-class ,obj) ,class))
+  (eq (eieio--object-class obj) class))
 
 (defun same-class-p (obj class) "Return t if OBJ is of class-type CLASS."
   (eieio--check-type class-p class)
@@ -2459,11 +2456,10 @@
 		   buffer-file-name))
 	  loc)
       (when fname
-	(when (string-match "\\.elc$" fname)
+	(when (string-match "\\.elc\\'" fname)
 	  (setq fname (substring fname 0 (1- (length fname)))))
 	(setq loc (get method-name 'method-locations))
-	(add-to-list 'loc
-		     (list class fname))
+	(pushnew (list class fname) loc :test #'equal)
 	(put method-name 'method-locations loc)))
     ;; Now optimize the entire obarray
     (if (< key method-num-lists)

Re: EIEIO with lexical scoping

[Eric M. Ludlam]

On 05/13/2013 05:25 PM, Stefan Monnier wrote:
> I'm trying to compile eieio.el using lexical-binding but am bumping into
> a problem that requires too much internal knowledge of eieio for me.
>
> I'm using the patch appended below for now, which seems to work to some
> extent, but when I then try to compile CEDET using it, I get the
> following backtrace:
>
> Can you help me figure out what's going on?
>

Sure, I'll be glad to help, though I don't know much about the lexical 
binding feature.

I do know someone posted on the cedet-devel mailing list saying that 
EIEIO's use of 'scoped-class' broke lexical-binding.  I'd been thinking 
about converting to use a stack instead of locally rebinding the value 
but hadn't gotten that far.

> Debugger entered--Lisp error: (invalid-slot-name "#<semanticdb-project-database default-cache-object>" tables)
>    signal(invalid-slot-name ("#<semanticdb-project-database default-cache-object>" tables))
>    eieio-default-superclass([object semanticdb-project-database default-cache-object unbound semanticdb-table nil nil] tables oref-default)
>    apply(eieio-default-superclass ([object semanticdb-project-database default-cache-object unbound semanticdb-table nil nil] tables oref-default))
>    eieio-generic-call-primary-only(slot-missing ([object semanticdb-project-database default-cache-object unbound semanticdb-table nil nil] tables oref-default))
>    slot-missing([object semanticdb-project-database default-cache-object unbound semanticdb-table nil nil] tables oref-default)
>    eieio-oref-default([object semanticdb-project-database default-cache-object unbound semanticdb-table nil nil] tables)
>    eieio-set-defaults([object semanticdb-project-database default-cache-object unbound semanticdb-table nil nil] t)

The code around here looks like this:

       (let ((eieio-skip-typecheck t))
	;; All type-checking has been done to our satisfaction
	;; before this call.  Don't waste our time in this call..
	(eieio-set-defaults cache t))

I'm not that familiar with lexical-binding, but I suspect this won't 
work... right?  I didn't see this in your patch.  Of course, it also 
doesn't have anything to do with the error either.

A quick search shows that the method invocation has a similar pattern 
with 'eieiomt-optimizing-obarray' in 'eieio-generic-form' as well as use 
of 'scoped-class' in 'shared-initialize'.  I would look at these first. 
  I don't have line numbers since I think we are using different flavors 
of EIEIO.  (see below.)

>    eieio-defclass(semanticdb-project-database (eieio-instance-tracker) ((tracking-symbol :initform semanticdb-database-list) (reference-directory :type string :documentation "Directory this database refers to.\nWhen a cache directory is specified, then this refers to the directory\nthis database contains symbols for.") (new-table-class :initform semanticdb-table :type class :documentation "New tables created for this database are of this class.") (cache :type list :initform nil :documentation "List of cache information for tools.\nAny particular tool can cache data to a database at runtime\nwith `semanticdb-cache-get'.\n\nUsing a semanticdb cache does not save any information to a file,\nso your cache will need to be recalculated at runtime.\n\nNote: This index will not be saved in a per
 sistent file.") (tables :initarg :tables :type semanticdb-abstract-table-list :accessor semanticdb-get-database-tables :protection :protected :documentation "List of `semantic-db-table' obje
cts.")) ("Database of file tables."))
>    (progn (eieio-defclass 'semanticdb-project-database '(eieio-instance-tracker) '((tracking-symbol :initform semanticdb-database-list) (reference-directory :type string :documentation "Directory this database refers to.\nWhen a cache directory is specified, then this refers to the directory\nthis database contains symbols for.") (new-table-class :initform semanticdb-table :type class :documentation "New tables created for this database are of this class.") (cache :type list :initform nil :documentation "List of cache information for tools.\nAny particular tool can cache data to a database at runtime\nwith `semanticdb-cache-get'.\n\nUsing a semanticdb cache does not save any information to a file,\nso your cache will need to be recalculated at runtime.\n\nNote: This index will not be sav
 ed in a persistent file.") (tables :initarg :tables :type semanticdb-abstract-table-list :accessor semanticdb-get-database-tables :protection :protected :documentation "List of `semantic-db-
table' objects.")) '("Database of file tables.")))
>    eval((progn (eieio-defclass 'semanticdb-project-database '(eieio-instance-tracker) '((tracking-symbol :initform semanticdb-database-list) (reference-directory :type string :documentation "Directory this database refers to.\nWhen a cache directory is specified, then this refers to the directory\nthis database contains symbols for.") (new-table-class :initform semanticdb-table :type class :documentation "New tables created for this database are of this class.") (cache :type list :initform nil :documentation "List of cache information for tools.\nAny particular tool can cache data to a database at runtime\nwith `semanticdb-cache-get'.\n\nUsing a semanticdb cache does not save any information to a file,\nso your cache will need to be recalculated at runtime.\n\nNote: This index will not b
 e saved in a persistent file.") (tables :initarg :tables :type semanticdb-abstract-table-list :accessor semanticdb-get-database-tables :protection :protected :documentation "List of `semanti
c-db-table' objects.")) '("Database of file tables."))) nil)
>    #[128 "\301\302\303B\b\"D\207" [lexical-binding quote eval progn] 5 "\n\n(fn&rest BODY)"]((eieio-defclass 'semanticdb-project-database '(eieio-instance-tracker) '((tracking-symbol :initform semanticdb-database-list) (reference-directory :type string :documentation "Directory this database refers to.\nWhen a cache directory is specified, then this refers to the directory\nthis database contains symbols for.") (new-table-class :initform semanticdb-table :type class :documentation "New tables created for this database are of this class.") (cache :type list :initform nil :documentation "List of cache information for tools.\nAny particular tool can cache data to a database at runtime\nwith `semanticdb-cache-get'.\n\nUsing a semanticdb cache does not save any information to a file,\nso your 
 cache will need to be recalculated at runtime.\n\nNote: This index will not be saved in a persistent file.") (tables :initarg :tables :type semanticdb-abstract-table-list :accessor semanticd
b-get-database-tables :protection :protected :documentation "List of `semantic-db-table' objects.")) '("Database of file tables.")))
>    (eval-and-compile (eieio-defclass 'semanticdb-project-database '(eieio-instance-tracker) '((tracking-symbol :initform semanticdb-database-list) (reference-directory :type string :documentation "Directory this database refers to.\nWhen a cache directory is specified, then this refers to the directory\nthis database contains symbols for.") (new-table-class :initform semanticdb-table :type class :documentation "New tables created for this database are of this class.") (cache :type list :initform nil :documentation "List of cache information for tools.\nAny particular tool can cache data to a database at runtime\nwith `semanticdb-cache-get'.\n\nUsing a semanticdb cache does not save any information to a file,\nso your cache will need to be recalculated at runtime.\n\nNote: This index will
  not be saved in a persistent file.") (tables :initarg :tables :type semanticdb-abstract-table-list :accessor semanticdb-get-database-tables :protection :protected :documentation "List of `s
emantic-db-table' objects.")) '("Database of file tables.")))

Note here: ^^^^

You are using a version of eieio that still uses eval-and-compile.  I 
flushed the remaining uses of eval-and-compile from EIEIO in March in 
the CEDET repository due to a previous thread.  I thought I had shared 
this change or it was merged upstream, but maybe I forgot?  I apologize 
if you didn't get the update.   The changes were extensive as half of 
EIEIO moved into a second file to help with compilation.  It would be 
better of we both worked from there.  Papers are all up to date if you 
want to pull that change from the CEDET repository.

>    (defclass semanticdb-project-database (eieio-instance-tracker) ((tracking-symbol :initform semanticdb-database-list) (reference-directory :type string :documentation "Directory this database refers to.\nWhen a cache directory is specified, then this refers to the directory\nthis database contains symbols for.") (new-table-class :initform semanticdb-table :type class :documentation "New tables created for this database are of this class.") (cache :type list :initform nil :documentation "List of cache information for tools.\nAny particular tool can cache data to a database at runtime\nwith `semanticdb-cache-get'.\n\nUsing a semanticdb cache does not save any information to a file,\nso your cache will need to be recalculated at runtime.\n\nNote: This index will not be saved in a persiste
 nt file.") (tables :initarg :tables :type semanticdb-abstract-table-list :accessor semanticdb-get-database-tables :protection :protected :documentation "List of `semantic-db-table' objects."
)) "Database of file tables.")

[snip]

I have a two random questions below.

> === modified file 'lisp/emacs-lisp/eieio.el'
> --- lisp/emacs-lisp/eieio.el	2013-02-27 04:09:50 +0000
> +++ lisp/emacs-lisp/eieio.el	2013-05-13 21:22:02 +0000
> @@ -1,4 +1,4 @@
> -;;; eieio.el --- Enhanced Implementation of Emacs Interpreted Objects
> +;;; eieio.el --- Enhanced Implementation of Emacs Interpreted Objects  -*- lexical-binding:t -*-
>   ;;;              or maybe Eric's Implementation of Emacs Interpreted Objects
>
>   ;; Copyright (C) 1995-1996, 1998-2013 Free Software Foundation, Inc.
> @@ -193,32 +193,31 @@
>     ;; No check: If eieio gets this far, it's probably been checked already.
>     `(get ,class 'eieio-class-definition))
>
> -(defmacro class-p (class)
> +(defsubst class-p (class)

Is this generic cleanup, or important for a lexical binding?

> @@ -501,7 +500,7 @@
>   		    (setf (eieio--class-children (class-v (car pname)))
>   			  (cons cname (eieio--class-children (class-v (car pname))))))
>   		  ;; Get custom groups, and store them into our local copy.
> -		  (mapc (lambda (g) (add-to-list 'groups g))
> +		  (mapc (lambda (g) (pushnew g groups :test #'equal))

Similar question.  Is add-to-list bad for lexical binding?

Thanks
Eric

Re: EIEIO with lexical scoping

[Eric M. Ludlam]

To follow up with my previous reply...

I tried out compiling eieio.el, and eieio-core.el with lexical binding. 
  I then went in and removed 'scoped-class' and replaced it with some 
code that avoids the dynamic binding.  I got all this passing my test 
suite, and checked it into CEDET's repository.

I did not test it with lexical-binding set during compilation.  I just 
tried to get it closer to what you probably need.

I hope that helps.
Eric

On 05/13/2013 08:00 PM, Eric M. Ludlam wrote:
> On 05/13/2013 05:25 PM, Stefan Monnier wrote:
>> I'm trying to compile eieio.el using lexical-binding but am bumping into
>> a problem that requires too much internal knowledge of eieio for me.
>>
>> I'm using the patch appended below for now, which seems to work to some
>> extent, but when I then try to compile CEDET using it, I get the
>> following backtrace:
>>
>> Can you help me figure out what's going on?
>>
>
> Sure, I'll be glad to help, though I don't know much about the lexical
> binding feature.
>
> I do know someone posted on the cedet-devel mailing list saying that
> EIEIO's use of 'scoped-class' broke lexical-binding. I'd been thinking
> about converting to use a stack instead of locally rebinding the value
> but hadn't gotten that far.

Re: EIEIO with lexical scoping

[Stefan Monnier]

> The code around here looks like this:

>       (let ((eieio-skip-typecheck t))
> 	;; All type-checking has been done to our satisfaction
> 	;; before this call.  Don't waste our time in this call..
> 	(eieio-set-defaults cache t))

> I'm not that familiar with lexical-binding, but I suspect this won't
> work... right?

I don't see anything problematic here (since eieio-skip-typecheck
was declared with defvar to be dynamically scoped).

> A quick search shows that the method invocation has a similar pattern with
> eieiomt-optimizing-obarray' in 'eieio-generic-form' as well as use of
> scoped-class' in 'shared-initialize'.  I would look at these first.

eieiomt-optimizing-obarray is defvarred as well, so it looks fine.
But `scoped-class' indeed looks like it might be a potential source of
problems, I'll take a closer look, thank you.

> You are using a version of eieio that still uses eval-and-compile.

Yes.

> I flushed the remaining uses of eval-and-compile from EIEIO in March in the
> CEDET repository due to a previous thread.

Yes, I remember.

> I thought I had shared this change or it was merged upstream, but
> maybe I forgot?  I apologize if you didn't get the update.
> The changes were extensive as half of EIEIO moved into a second file
> to help with compilation.

Those changes haven't yet been merged into trunk.  I was hoping someone
else could do it.

> It would be better of we both worked from there.  Papers are all up to
> date if you want to pull that change from the CEDET repository.

Right, I'm not particularly worried.  For now, I'm just trying to make
it work and don't plan on installing the change into trunk yet.
And porting/merging the patch won't be a problem, at least for now.

>> -(defmacro class-p (class)
>> +(defsubst class-p (class)
> Is this generic cleanup, or important for a lexical binding?

It's not indispensable, but enables the subsequent

-	       (if (fboundp namep)
-		   (funcall `(lambda () (,namep val)))
-		 (funcall `(lambda ()
-			     (,(intern (concat name "-p")) val)))))))
+	       (funcall (if (fboundp namep) namep (intern (concat name "-p")))
+                        val))))

simplification, which otherwise would need to be turned into:

	       (funcall `(lambda (val)
                           (,(if (fboundp namep) namep
                               (intern (concat name "-p")))
                            val))
                        val))))

Note how `val' needs to be passed to the newly built lambda, because the
`val' inside the lambda is just a symbol in some data structure (as far
as the compiler is concerned) rather than being a reference to the
external `val' variable.  With dynamic scoping, this worked, but not
with lexical scoping.

`defsubst' tends to be a bit more costly than an "equivalent" defmacro,
but that's less so when compiling with lexical scoping, which is why
I think it's OK to make things like class-p into defsubsts.

>> -		  (mapc (lambda (g) (add-to-list 'groups g))
>> +		  (mapc (lambda (g) (pushnew g groups :test #'equal))
> Similar question.  Is add-to-list bad for lexical binding?

Very much so, yes, because here again "'groups" just returns a symbol
and the compiler has no idea it's related to the "groups" variable.


        Stefan

Re: EIEIO with lexical scoping

[Stefan Monnier]

> eieiomt-optimizing-obarray is defvarred as well, so it looks fine.
> But `scoped-class' indeed looks like it might be a potential source of
> problems, I'll take a closer look, thank you.

It looks like this was the remaining culprit.
I'm using the patch below now, and haven't noticed problems yet.


        Stefan


=== modified file 'lisp/emacs-lisp/eieio.el'
--- lisp/emacs-lisp/eieio.el	2013-02-27 04:09:50 +0000
+++ lisp/emacs-lisp/eieio.el	2013-05-14 02:50:01 +0000
@@ -1,4 +1,4 @@
-;;; eieio.el --- Enhanced Implementation of Emacs Interpreted Objects
+;;; eieio.el --- Enhanced Implementation of Emacs Interpreted Objects  -*- lexical-binding:t -*-
 ;;;              or maybe Eric's Implementation of Emacs Interpreted Objects
 
 ;; Copyright (C) 1995-1996, 1998-2013 Free Software Foundation, Inc.
@@ -54,6 +54,8 @@
   (interactive)
   (message eieio-version))
 
+(defvar scoped-class)
+
 ;; FIXME: Don't wrap all the code inside `eval-and-compile'!
 (eval-and-compile
 ;; About the above.  EIEIO must process its own code when it compiles
@@ -193,32 +195,31 @@
   ;; No check: If eieio gets this far, it's probably been checked already.
   `(get ,class 'eieio-class-definition))
 
-(defmacro class-p (class)
+(defsubst class-p (class)
   "Return t if CLASS is a valid class vector.
 CLASS is a symbol."
   ;; this new method is faster since it doesn't waste time checking lots of
   ;; things.
-  `(condition-case nil
-       (eq (aref (class-v ,class) 0) 'defclass)
+  (condition-case nil
+      (eq (aref (class-v class) 0) 'defclass)
      (error nil)))
 
-(defmacro eieio-object-p (obj)
+(defsubst eieio-object-p (obj)
   "Return non-nil if OBJ is an EIEIO object."
-  `(let ((tobj ,obj))
-     (and (vectorp tobj)
-          (eq (aref tobj 0) 'object)
-          (class-p (eieio--object-class tobj)))))
+  (and (vectorp obj)
+       (eq (aref obj 0) 'object)
+       (class-p (eieio--object-class obj))))
 (defalias 'object-p 'eieio-object-p)
 
 (defmacro class-constructor (class)
   "Return the symbol representing the constructor of CLASS."
   `(eieio--class-symbol (class-v ,class)))
 
-(defmacro generic-p (method)
+(defsubst generic-p (method)
   "Return t if symbol METHOD is a generic function.
 Only methods have the symbol `eieio-method-obarray' as a property
 \(which contains a list of all bindings to that method type.)"
-  `(and (fboundp ,method) (get ,method 'eieio-method-obarray)))
+  (and (fboundp method) (get method 'eieio-method-obarray)))
 
 (defun generic-primary-only-p (method)
   "Return t if symbol METHOD is a generic function with only primary methods.
@@ -261,10 +262,10 @@
 Return nil if that option doesn't exist."
   `(class-option-assoc (eieio--class-options (class-v ,class)) ',option))
 
-(defmacro class-abstract-p (class)
+(defsubst class-abstract-p (class)
   "Return non-nil if CLASS is abstract.
 Abstract classes cannot be instantiated."
-  `(class-option ,class :abstract))
+  (class-option class :abstract))
 
 (defmacro class-method-invocation-order (class)
   "Return the invocation order of CLASS.
@@ -501,7 +502,7 @@
 		    (setf (eieio--class-children (class-v (car pname)))
 			  (cons cname (eieio--class-children (class-v (car pname))))))
 		  ;; Get custom groups, and store them into our local copy.
-		  (mapc (lambda (g) (add-to-list 'groups g))
+		  (mapc (lambda (g) (pushnew g groups :test #'equal))
 			(class-option (car pname) :custom-groups))
 		  ;; save parent in child
 		  (setf (eieio--class-parent newc) (cons (car pname) (eieio--class-parent newc))))
@@ -680,7 +681,7 @@
 			     prot initarg alloc 'defaultoverride skip-nil)
 
 	;; We need to id the group, and store them in a group list attribute.
-	(mapc (lambda (cg) (add-to-list 'groups cg)) customg)
+	(mapc (lambda (cg) (pushnew cg groups :test #'equal)) customg)
 
 	;; Anyone can have an accessor function.  This creates a function
 	;; of the specified name, and also performs a `defsetf' if applicable
@@ -815,8 +816,7 @@
     ;; Save the file location where this class is defined.
     (let ((fname (if load-in-progress
 		     load-file-name
-		   buffer-file-name))
-	  loc)
+		   buffer-file-name)))
       (when fname
 	(when (string-match "\\.elc\\'" fname)
 	  (setq fname (substring fname 0 (1- (length fname)))))
@@ -824,7 +824,7 @@
 
     ;; We have a list of custom groups.  Store them into the options.
     (let ((g (class-option-assoc options :custom-groups)))
-      (mapc (lambda (cg) (add-to-list 'g cg)) groups)
+      (mapc (lambda (cg) (pushnew cg g :test #'equal)) groups)
       (if (memq :custom-groups options)
 	  (setcar (cdr (memq :custom-groups options)) g)
 	(setq options (cons :custom-groups (cons g options)))))
@@ -856,10 +856,10 @@
 (defun eieio-perform-slot-validation-for-default (slot spec value skipnil)
   "For SLOT, signal if SPEC does not match VALUE.
 If SKIPNIL is non-nil, then if VALUE is nil return t instead."
-  (if (and (not (eieio-eval-default-p value))
-	   (not eieio-skip-typecheck)
-	   (not (and skipnil (null value)))
-	   (not (eieio-perform-slot-validation spec value)))
+  (if (not (or (eieio-eval-default-p value)
+	       eieio-skip-typecheck
+	       (and skipnil (null value))
+	       (eieio-perform-slot-validation spec value)))
       (signal 'invalid-slot-type (list slot spec value))))
 
 (defun eieio-add-new-slot (newc a d doc type cust label custg print prot init alloc
@@ -1200,7 +1200,7 @@
                (eq (symbol-function name) body))
     (defalias name body)))
 
-(defmacro defgeneric (method args &optional doc-string)
+(defmacro defgeneric (method _args &optional doc-string)
   "Create a generic function METHOD.
 DOC-STRING is the base documentation for this class.  A generic
 function has no body, as its purpose is to decide which method body
@@ -1287,6 +1287,7 @@
 	      (apply 'no-applicable-method (car local-args)
 		     ',method local-args)
 
+            (defvar scoped-class)
 	    ;; It is ok, do the call.
 	    ;; Fill in inter-call variables then evaluate the method.
 	    (let ((scoped-class ',class)
@@ -1425,10 +1426,8 @@
 	    (t
 	     (let* ((name (symbol-name type))
 		    (namep (intern (concat name "p"))))
-	       (if (fboundp namep)
-		   (funcall `(lambda () (,namep val)))
-		 (funcall `(lambda ()
-			     (,(intern (concat name "-p")) val)))))))
+	       (funcall (if (fboundp namep) namep (intern (concat name "-p")))
+                        val))))
     (cond ((get (car type) 'cl-deftype-handler)
 	   (eieio--typep val (apply (get (car type) 'cl-deftype-handler)
 				    (cdr type))))
@@ -1450,7 +1449,7 @@
 	  ((memq (car type) '(member member*))
 	   (memql val (cdr type)))
 	  ((eq (car type) 'satisfies)
-	   (funcall `(lambda () (,(cadr type) val))))
+	   (funcall (cadr type) val))
 	  (t (error "Bad type spec: %s" type)))))
 
 (defun eieio-perform-slot-validation (spec value)
@@ -1799,9 +1798,9 @@
   `(car (eieio-class-parents ,class)))
 (define-obsolete-function-alias 'class-parent #'eieio-class-parent "24.4")
 
-(defmacro same-class-fast-p (obj class)
+(defsubst same-class-fast-p (obj class)
   "Return t if OBJ is of class-type CLASS with no error checking."
-  `(eq (eieio--object-class ,obj) ,class))
+  (eq (eieio--object-class obj) class))
 
 (defun same-class-p (obj class) "Return t if OBJ is of class-type CLASS."
   (eieio--check-type class-p class)
@@ -2212,7 +2211,8 @@
 
     ;; Now loop through all occurrences forms which we must execute
     ;; (which are happily sorted now) and execute them all!
-    (let ((rval nil) (lastval nil) (rvalever nil) (found nil))
+    (let ((rval nil) (lastval nil) ;; (rvalever nil)
+          (found nil))
       (while lambdas
 	(if (car lambdas)
 	    (let* ((scoped-class (cdr (car lambdas)))
@@ -2230,14 +2230,16 @@
 	      (setq lastval (apply (car (car lambdas)) newargs))
 	      (when has-return-val
 	      	(setq rval lastval
-	      	      rvalever t))
+	      	      ;; rvalever t
+                      ))
 	      ))
 	(setq lambdas (cdr lambdas)
 	      keys (cdr keys)))
       (if (not found)
 	  (if (eieio-object-p (car args))
 	      (setq rval (apply 'no-applicable-method (car args) method args)
-		    rvalever t)
+		    ;; rvalever t
+                    )
 	    (signal
 	     'no-method-definition
 	     (list method args))))
@@ -2292,7 +2294,7 @@
 
     ;; Now loop through all occurrences forms which we must execute
     ;; (which are happily sorted now) and execute them all!
-    (let* ((rval nil) (lastval nil) (rvalever nil)
+    (let* ((rval nil) (lastval nil) ;; (rvalever nil)
 	   (scoped-class (cdr lambdas))
 	   (eieio-generic-call-key method-primary)
 	   ;; Use the cdr, as the first element is the fcn
@@ -2305,7 +2307,8 @@
 	  ;; No methods found for this impl...
 	  (if (eieio-object-p (car args))
 	      (setq rval (apply 'no-applicable-method (car args) method args)
-		    rvalever t)
+		    ;; rvalever t
+                    )
 	    (signal
 	     'no-method-definition
 	     (list method args)))
@@ -2317,7 +2320,8 @@
 
 	(setq lastval (apply (car lambdas) newargs))
 	(setq rval lastval
-	      rvalever t)
+	      ;; rvalever t
+              )
 	)
 
       ;; Right Here... it could be that lastval is returned when
@@ -2459,11 +2463,10 @@
 		   buffer-file-name))
 	  loc)
       (when fname
-	(when (string-match "\\.elc$" fname)
+	(when (string-match "\\.elc\\'" fname)
 	  (setq fname (substring fname 0 (1- (length fname)))))
 	(setq loc (get method-name 'method-locations))
-	(add-to-list 'loc
-		     (list class fname))
+	(pushnew (list class fname) loc :test #'equal)
 	(put method-name 'method-locations loc)))
     ;; Now optimize the entire obarray
     (if (< key method-num-lists)
@@ -2728,7 +2731,7 @@
   "Method invoked when an attempt to access a slot in OBJECT fails.")
 
 (defmethod slot-missing ((object eieio-default-superclass) slot-name
-			 operation &optional new-value)
+			 _operation &optional _new-value)
   "Method invoked when an attempt to access a slot in OBJECT fails.
 SLOT-NAME is the name of the failed slot, OPERATION is the type of access
 that was requested, and optional NEW-VALUE is the value that was desired
@@ -2761,7 +2764,7 @@
   "Called if there are no implementations for OBJECT in METHOD.")
 
 (defmethod no-applicable-method ((object eieio-default-superclass)
-				 method &rest args)
+				 method &rest _args)
   "Called if there are no implementations for OBJECT in METHOD.
 OBJECT is the object which has no method implementation.
 ARGS are the arguments that were passed to METHOD.
@@ -2811,7 +2814,7 @@
 (defgeneric destructor (this &rest params)
   "Destructor for cleaning up any dynamic links to our object.")
 
-(defmethod destructor ((this eieio-default-superclass) &rest params)
+(defmethod destructor ((_this eieio-default-superclass) &rest _params)
   "Destructor for cleaning up any dynamic links to our object.
 Argument THIS is the object being destroyed.  PARAMS are additional
 ignored parameters."

Re: EIEIO with lexical scoping

[David Engster]

Stefan Monnier writes:
>> I thought I had shared this change or it was merged upstream, but
>> maybe I forgot?  I apologize if you didn't get the update.
>> The changes were extensive as half of EIEIO moved into a second file
>> to help with compilation.
>
> Those changes haven't yet been merged into trunk.  I was hoping someone
> else could do it.

I'll do another merge soon.

-David

Re: EIEIO with lexical scoping

[Stefan Monnier]

>>> I thought I had shared this change or it was merged upstream, but
>>> maybe I forgot?  I apologize if you didn't get the update.
>>> The changes were extensive as half of EIEIO moved into a second file
>>> to help with compilation.
>> Those changes haven't yet been merged into trunk.  I was hoping someone
>> else could do it.
> I'll do another merge soon.

Thanks.  I didn't mean to put pressure, tho, do it when convenient.
It's not like I'm waiting for it to install my patch (I just started to
use lexical-binding *everywhere* and am slowly walking through the
resulting breakage, trying to learn about ways/strategies to better
detect/fix them).


        Stefan

Re: EIEIO with lexical scoping

[David Engster]

David Engster writes:
> Stefan Monnier writes:
>>> I thought I had shared this change or it was merged upstream, but
>>> maybe I forgot?  I apologize if you didn't get the update.
>>> The changes were extensive as half of EIEIO moved into a second file
>>> to help with compilation.
>>
>> Those changes haven't yet been merged into trunk.  I was hoping someone
>> else could do it.
>
> I'll do another merge soon.

Done.

Eric's work on EIEIO is now in Emacs. The test suite did run fine, but
it might still be that stuff breaks. For instance, I had to change
registry.el in Gnus, which for some reason had all 'defmethod' calls
wrapped in 'eval-and-compile', which does not compile anymore. I don't
know why, though; you probably do. ;-)

-David

Re: EIEIO with lexical scoping

[Stefan Monnier]

> Eric's work on EIEIO is now in Emacs. The test suite did run fine, but
> it might still be that stuff breaks.

Great, thanks.

> For instance, I had to change registry.el in Gnus, which for some
> reason had all 'defmethod' calls wrapped in 'eval-and-compile', which
> does not compile anymore. I don't know why, though; you probably
> do. ;-)

I don't either, actually.

In a some related vein.  I remember discussions to take EIEIO out of
CEDET, (i.e. use Emacs's built-in eieio for recent Emacsen, and
distribute a separate eieio package for people who want to use CEDET on
older Emacsen).

I'd welcome such a move.  IOW for me, the earlier the better.


        Stefan

Re: EIEIO with lexical scoping

[David Engster]

Stefan Monnier writes:
>> For instance, I had to change registry.el in Gnus, which for some
>> reason had all 'defmethod' calls wrapped in 'eval-and-compile', which
>> does not compile anymore. I don't know why, though; you probably
>> do. ;-)
>
> I don't either, actually.

Well, is this something which needs fixing?

> In a some related vein.  I remember discussions to take EIEIO out of
> CEDET, (i.e. use Emacs's built-in eieio for recent Emacsen, and
> distribute a separate eieio package for people who want to use CEDET on
> older Emacsen).
>
> I'd welcome such a move.  IOW for me, the earlier the better.

I think now would be a good time, but Eric should say if he's OK with
that. Since we bumped the minimum supported version for CEDET to 23.2,
we wouldn't have to ship EIEIO at all.

The only thing that's missing in Emacs is the EIEIO test suite, but I
can fix that.

-David

Re: EIEIO with lexical scoping

[Glenn Morris]

>>> For instance, I had to change registry.el in Gnus, which for some
>>> reason had all 'defmethod' calls wrapped in 'eval-and-compile', which
>>> does not compile anymore. I don't know why, though; you probably
>>> do. ;-)
>>
>> I don't either, actually.

I suspect the following might be related; now `defclass' is opaque to
the byte-compiler. Compiling this:

(require 'eieio)

(defclass foo-class ()
  (())
  "blah.")

(defun foo-func ()
 (if foo-class
  (foo-class 'blah)))

warns:
  
  In foo-func:
  foo.el:8:7:Warning: reference to free variable `foo-class'
  
  In end of data:
  foo.el:9:1:Warning: the function `foo-class' is not known to be defined.

which is a bit annoying. It didn't use to do this. A total guess, maybe
the removal of `eval-and-compile' from defclass?

Re: EIEIO with lexical scoping

[Stefan Monnier]

> which is a bit annoying. It didn't use to do this. A total guess, maybe
> the removal of `eval-and-compile' from defclass?

Could be.  As for the right fix, it's already suggested in the code:

   (defun eieio-defclass (cname superclasses slots options-and-doc)
     ;; FIXME: Most of this should be moved to the `defclass' macro.


        Stefan