Subject: Re: bug#29220: 26.0.90; eieio-persistent-read fail to restore saved object.

From: Eric Abrahamsen <[email-masked]>
Date: Tue, 05 Dec 2017 09:47:26 -0800

On 12/04/17 21:08 PM, Stefan Monnier wrote:
>> --- a/lisp/emacs-lisp/eieio-base.el
>> +++ b/lisp/emacs-lisp/eieio-base.el
>> @@ -354,15 +354,34 @@ eieio-persistent-validate/fix-slot-value
>>  		   proposed-value))
>>  		 (t
>>  		  proposed-value))))
>> +	((stringp proposed-value)
>> +	 ;; Remove string properties.
>> +	 (substring-no-properties proposed-value))
>> +
>> +        ;; For hash-tables and vectors, the top-level `read' will not
>> +        ;; "look inside" member values, so we need to do that
>> +        ;; explicitly.
>> +        ((hash-table-p proposed-value)
>> +         (maphash
>> +          (lambda (key value)
>> +            (when (class-p (car-safe value))
>> +              (setf (gethash key proposed-value)
>> +                    (eieio-persistent-convert-list-to-object
>> +                     value))))
>> +          proposed-value)
>> +         proposed-value)
>> +
>> +        ((vectorp proposed-value)
>> +         (dotimes (i (length proposed-value))
>> +           (when (class-p (car-safe (aref proposed-value i)))
>> +             (aset proposed-value i
>> +                   (eieio-persistent-convert-list-to-object
>> +                    (aref proposed-value i)))))
>> +         proposed-value)
>  
>> -	 ((stringp proposed-value)
>> -	  ;; Else, check for strings, remove properties.
>> -	  (substring-no-properties proposed-value))
>> -
>> -	 (t
>> -	  ;; Else, just return whatever the constant was.
>> -	  proposed-value))
>> -  )
>> +	(t
>> +	 ;; Else, just return whatever the constant was.
>> +	 proposed-value)))
>
> Not sure why the `stringp` part was reindented, and an empty line was
> lost (which de-synchronized the diff), but other than that, it looks fine.

Dunno about the reindentation, either, I guess I can discard those
changes.

>> +	((hash-table-p thing)
>> +	 (maphash
>> +	  (lambda (key val)
>> +	    (setf (gethash key thing)
>> +		  (read
>> +		   (with-output-to-string
>> +		     (temp-eieio-override-prin1 val)))))
>> +	  thing)
>> +	 (prin1 thing))
>> +	((vectorp thing)
>> +	 (dotimes (i (length thing))
>> +	   (aset thing i
>> +		 (read
>> +		  (with-output-to-string
>> +		    (temp-eieio-override-prin1
>> +		     (aref thing i))))))
>> +	 (prin1 thing))
>
> This looks wrong, OTOH:
> - temp-eieio-override-prin1 does not exist in my copy of Emacs
> `master`.

That was a temporary name to prevent these changes from wrecking my Gnus
and EBDB. I guess I should be working in a separate Emacs.

> - you modify the object you received, so while the print part will
>   presumably work OK, the object left after that is unusable.

I should have realized that...

> Maybe a better option is to print something of the form
>
>    (vector <exp1> <exp2> ...)
>
> i.e. not something that looks like a vector.  Or otherwise, print the
> "standard" syntax of a vector/hash-table, but do it by hand rather than
> rely on prin1 to do it for us.

Would it be too "heavy" to just copy the object and modify the copy? It
seems like writing a list-like form will result in just as much
consing...?

Eric