unofficial mirror of emacs-devel@gnu.org 
 help / color / mirror / code / Atom feed
From: Stefan Monnier <monnier@iro.umontreal.ca>
To: "Eric M. Ludlam" <eric@siege-engine.com>
Cc: "Eric M. Ludlam" <zappo@gnu.org>, emacs-devel@gnu.org
Subject: Re: EIEIO with lexical scoping
Date: Mon, 13 May 2013 22:13:57 -0400	[thread overview]
Message-ID: <jwvobces4o8.fsf-monnier+emacs@gnu.org> (raw)
In-Reply-To: <51917EA1.50403@siege-engine.com> (Eric M. Ludlam's message of "Mon, 13 May 2013 20:00:33 -0400")

> 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



  parent reply	other threads:[~2013-05-14  2:13 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-05-13 21:25 EIEIO with lexical scoping Stefan Monnier
2013-05-14  0:00 ` Eric M. Ludlam
2013-05-14  1:57   ` Eric M. Ludlam
2013-05-14  2:13   ` Stefan Monnier [this message]
2013-05-14 12:42     ` Stefan Monnier
2013-05-14 20:25     ` David Engster
2013-05-14 22:32       ` Stefan Monnier
2013-06-02 16:45       ` David Engster
2013-06-02 20:47         ` Stefan Monnier
2013-06-03 15:23           ` David Engster
2013-06-03 18:35             ` Glenn Morris
2013-06-03 19:35               ` Stefan Monnier

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: https://www.gnu.org/software/emacs/

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=jwvobces4o8.fsf-monnier+emacs@gnu.org \
    --to=monnier@iro.umontreal.ca \
    --cc=emacs-devel@gnu.org \
    --cc=eric@siege-engine.com \
    --cc=zappo@gnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/emacs.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).