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 persistent 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 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 `semanti
c-db-table' objects.")) '("Database of file tables."))) nil)
>    #[128 "\301\302\303B\"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 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.")

[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