Subject: Re: bug#20914: eieio/slot/group

From: Pierre Lorenzon <[email-masked]>
Date: Wed, 01 Jul 2015 06:02:28 +0200 (CEST)

From: Stefan Monnier <[email-masked]>
Subject: Re: bug#20914: eieio/slot/group
Date: Tue, 30 Jun 2015 14:09:20 -0400

>> The problem comes from use of `copy-sequence' in
>> `eieio-copy-parents-into-subclass' function of eieio-core. this
>> copy is not reccursive as explained in code comment but it
>> should be ! There's a joined patch that fixes the bug.
> 
> Thanks for the investigation.  I think the fix should be

  Sure it is better to have a true copier for
  cl--slot-descriptor.

> applied deeper.  Can you confirm that the patch below works for you

  unfortunately it does not ! I modified cl-preloaded.el
  recompiled it but once restarting emacs my example classes do
  not havae suitable properties. When doing Ch f
  cl--copy-slot-descriptor I get that it is an alias for
  copy-sequence as it was before and not the function defined
  by the new definition you gave. Moreover if I force
  cl-prelaoded.el to be loaded I get the correct definition. I
  suspect that things are not loaded as they should but why
  ....

> as well?
> 
> 
>         Stefan
> 
> 
> diff --git a/lisp/emacs-lisp/cl-preloaded.el b/lisp/emacs-lisp/cl-preloaded.el
> index ed0639b..2e9bdde 100644
> --- a/lisp/emacs-lisp/cl-preloaded.el
> +++ b/lisp/emacs-lisp/cl-preloaded.el
> @@ -195,7 +195,7 @@
>                 (:constructor nil)
>                 (:constructor cl--make-slot-descriptor
>                  (name &optional initform type props))
> -               (:copier cl--copy-slot-descriptor))
> +               (:copier nil))
>    ;; FIXME: This is actually not used yet, for circularity reasons!
>    "Descriptor of structure slot."
>    name                                  ;Attribute name (symbol).
> @@ -205,6 +205,11 @@
>    ;;  :documentation, :protection, :custom, :label, :group, :printer.
>    (props nil :type alist))
>  
> +(defun cl--copy-slot-descriptor (slot)
> +  (let ((new (copy-sequence slot)))

  Are you sure that a copy-sequence will be enough here ? props
  themselves are sequences hence they will be referenced by
  this copy-sequence not copied. I think that a copy-tree
  should be used here.

> +    (cl-callf copy-sequence (cl--slot-descriptor-props new))
> +    new))
> +
>  (cl-defstruct (cl--class
>                 (:constructor nil)
>                 (:copier nil))

		  Ok I'll try to understand why it does not
		  work. I'll probably learn something in this
		  history and that's good !

Pierre