Re: Updated Patch for command remapping through keymaps

Re: Updated Patch for command remapping through keymaps

[Kim F. Storm]

storm@cua.dk (Kim F. Storm) writes:

> Richard Stallman <rms@gnu.org> writes:
> 
> >     However, I don't really understand the lack of GCPRO in some of the
> >     functions in keymap.c.  Specifically, I would have expected GCPRO
> >     before the calls to
> >       keymap = get_keymap (..., ..., 1);
> >     in
> >       Fset_keymap_parent  (parent)
> > 
> > You are right.  This was not needed in the past, but now that
> > get_keymap can autoload and thus run Lisp code, the gcpro is needed.
> > Could someone add that?
> 
> I will do that as soon as I have committed my current set of patches.

Well, I have looked at it, and it is more complicated than that.  If
get_keymap can GC, then all the functions which calls it can also GC -
and so on.  For example, functions calling Fkeymap_parent may also GC.

I think fixing this requires a better understanding of the relationships
between these functions, so the maintainer of keymap.c should make the
changes.


-- 
Kim F. Storm <storm@cua.dk> http://www.cua.dk


_______________________________________________
Emacs-devel mailing list
Emacs-devel@gnu.org
http://mail.gnu.org/mailman/listinfo/emacs-devel

Re: Updated Patch for command remapping through keymaps

[Juanma Barranquero]

On 06 Feb 2002 22:23:56 +0100, storm@cua.dk (Kim F. Storm) wrote:

> I will do that as soon as I have committed my current set of patches.

Before your patches:

ELISP> (global-set-key (kbd "C-c a") #'(lambda (arg) (interactive "p") (message "%s" arg)))
(lambda
  (arg)
  (interactive "p")
  (message "%s" arg))

ELISP> (where-is-internal (lookup-key global-map (kbd "C-c a")))
([3 97])

After your patches:

ELISP> (global-set-key (kbd "C-c a") #'(lambda (arg) (interactive "p") (message "%s" arg)))
(lambda
  (arg)
  (interactive "p")
  (message "%s" arg))

ELISP> (where-is-internal (lookup-key global-map (kbd "C-c a")))
*** Eval error ***  Wrong type argument: arrayp, (lambda (arg) (interactive "p") (message "%s" arg))


                                                           /L/e/k/t/u


_______________________________________________
Emacs-devel mailing list
Emacs-devel@gnu.org
http://mail.gnu.org/mailman/listinfo/emacs-devel

Re: Updated Patch for command remapping through keymaps

[Eli Zaretskii]

On 7 Feb 2002, Kim F. Storm wrote:

> Well, I have looked at it, and it is more complicated than that.  If
> get_keymap can GC, then all the functions which calls it can also GC -
> and so on.  For example, functions calling Fkeymap_parent may also GC.
> 
> I think fixing this requires a better understanding of the relationships
> between these functions, so the maintainer of keymap.c should make the
> changes.

The irony of this is that on most modern platforms, including that of 
keymap.c's maintainer, GCPRO is a no-op...

_______________________________________________
Emacs-devel mailing list
Emacs-devel@gnu.org
http://mail.gnu.org/mailman/listinfo/emacs-devel

Re: Updated Patch for command remapping through keymaps

[Kim F. Storm]

Juanma Barranquero <lektu@terra.es> writes:

> After your patches:
> 
> ELISP> (global-set-key (kbd "C-c a") #'(lambda (arg) (interactive "p") (message "%s" arg)))
> (lambda
>   (arg)
>   (interactive "p")
>   (message "%s" arg))
> 
> ELISP> (where-is-internal (lookup-key global-map (kbd "C-c a")))
> *** Eval error ***  Wrong type argument: arrayp, (lambda (arg) (interactive "p") (message "%s" arg))

Thanks.  I've fixed that now.

-- 
Kim F. Storm <storm@cua.dk> http://www.cua.dk


_______________________________________________
Emacs-devel mailing list
Emacs-devel@gnu.org
http://mail.gnu.org/mailman/listinfo/emacs-devel

Re: Updated Patch for command remapping through keymaps

[Stefan Monnier]

> > Well, I have looked at it, and it is more complicated than that.  If
> > get_keymap can GC, then all the functions which calls it can also GC -
> > and so on.  For example, functions calling Fkeymap_parent may also GC.
> > 
> > I think fixing this requires a better understanding of the relationships
> > between these functions, so the maintainer of keymap.c should make the
> > changes.
> 
> The irony of this is that on most modern platforms, including that of 
> keymap.c's maintainer, GCPRO is a no-op...

I for one am running on a platform where GCPROs are no-ops, but I fiddled
the source so as not to use this (neat) feature (well, not for all my
builds, admittedly).


	Stefan


_______________________________________________
Emacs-devel mailing list
Emacs-devel@gnu.org
http://mail.gnu.org/mailman/listinfo/emacs-devel

Re: Updated Patch for command remapping through keymaps

[Richard Stallman]

    I think fixing this requires a better understanding of the relationships
    between these functions, so the maintainer of keymap.c should make the
    changes.

Is there any "the maintainer of keymap.c", other than me?  I can do it
if necessary, but I hope someone else will.

It is not fundamentally hard--you look at all calls to get_keymap and
add gcpro around them; you make a list of the functions they are in,
and look at all calls to them and add gcpro around them; you keep
repeating until there are no more calls in C code.  Not fundamentally
hard, but time-consuming.


_______________________________________________
Emacs-devel mailing list
Emacs-devel@gnu.org
http://mail.gnu.org/mailman/listinfo/emacs-devel

Re: Updated Patch for command remapping through keymaps

[Richard Stallman]

    The irony of this is that on most modern platforms, including that of 
    keymap.c's maintainer, GCPRO is a no-op...

Until this is true on all the platforms we care to support,
the GCPROs remain necessary.


_______________________________________________
Emacs-devel mailing list
Emacs-devel@gnu.org
http://mail.gnu.org/mailman/listinfo/emacs-devel

Re: Updated Patch for command remapping through keymaps

[Stefan Monnier]

>     I think fixing this requires a better understanding of the relationships
>     between these functions, so the maintainer of keymap.c should make the
>     changes.
> 
> Is there any "the maintainer of keymap.c", other than me?  I can do it
> if necessary, but I hope someone else will.

I volunteered as maintainer of keymap.c, so I assumed he was referring
to me.


	Stefan


_______________________________________________
Emacs-devel mailing list
Emacs-devel@gnu.org
http://mail.gnu.org/mailman/listinfo/emacs-devel

Re: Updated Patch for command remapping through keymaps

[Richard Stallman]

    I volunteered as maintainer of keymap.c, so I assumed he was referring
    to me.

Will you take care of adding these GCPROs?

_______________________________________________
Emacs-devel mailing list
Emacs-devel@gnu.org
http://mail.gnu.org/mailman/listinfo/emacs-devel

Re: Updated Patch for command remapping through keymaps

[Stefan Monnier]

>     I volunteered as maintainer of keymap.c, so I assumed he was referring
>     to me.
> 
> Will you take care of adding these GCPROs?

Yes, as soon as I find the time,


	Stefan


_______________________________________________
Emacs-devel mailing list
Emacs-devel@gnu.org
http://mail.gnu.org/mailman/listinfo/emacs-devel

Re: Updated Patch for command remapping through keymaps

[Richard Stallman]

    > Will you take care of adding these GCPROs?

    Yes, as soon as I find the time,

If that won't be soon, then it would be better if KFS tries it.
When do you think it will be?

_______________________________________________
Emacs-devel mailing list
Emacs-devel@gnu.org
http://mail.gnu.org/mailman/listinfo/emacs-devel