GNU bug report logs - #29541
27.0.50; Use generic function when restoring eieio-persistent, remove validation

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 29541 in the body.
You can then email your comments to 29541 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Eric Abrahamsen <eric <at> ericabrahamsen.net>
To: bug-gnu-emacs <at> gnu.org
Subject: 27.0.50; Use generic function when restoring eieio-persistent, remove
 validation
Date: Sat, 02 Dec 2017 16:10:07 -0800

[Message part 1 (text/plain, inline)]

The attached patch is a proposal that would replace the function
`eieio-persistent-convert-list-to-object' with a generic method
`eieio-persistent-make-instance'. It also removes all slot validation in
the read process.

Considerations:

1. `object-write' is already a method. That means the writing process
can be overridden and customized, but the reading process can't, which
removes most of the benefit of being able to override the write. This
would allow "matching" methods for writing and reading.

2. This version checks *all* slot values, including all elements inside
lists, cons-cells, etc. The safety consideration is mostly removing text
properties (specifically the display property), and this version checks
exhaustively.

3. I removed slot validation because it complicates the restoration
process, and because all the validation will be done again, and done
better, by `cl-typep' in the `initialize-instance' process. There's not
much reason to have a "pre-check", as errors are raised in both cases.

4. This version goes slightly faster than the old one (admittedly,
probably just because it omits type-checking).

Persistence format isn't changed, so this should be backwards
compatible.

I suppose the removal of validation might not be welcome, but at least I
hope the function->generic change is acceptable.



In GNU Emacs 27.0.50 (build 13, x86_64-pc-linux-gnu, GTK+ Version 3.22.26)
 of 2017-11-30 built on slip
Repository revision: 3f3d98ee5851840228786390ee7dbf851d144eb8

[0001-Replace-eieio-persistent-convert-list-to-object-with.patch (text/x-diff, attachment)]

Message #10 received at 29541 <at> debbugs.gnu.org (full text, mbox):

From: Lars Ingebrigtsen <larsi <at> gnus.org>
To: Eric Abrahamsen <eric <at> ericabrahamsen.net>
Cc: 29541 <at> debbugs.gnu.org
Subject: Re: bug#29541: 27.0.50;
 Use generic function when restoring eieio-persistent, remove
 validation
Date: Mon, 24 Jun 2019 22:28:44 +0200

Eric Abrahamsen <eric <at> ericabrahamsen.net> writes:

> The attached patch is a proposal that would replace the function
> `eieio-persistent-convert-list-to-object' with a generic method
> `eieio-persistent-make-instance'. It also removes all slot validation in
> the read process.
>
> Considerations:
>
> 1. `object-write' is already a method. That means the writing process
> can be overridden and customized, but the reading process can't, which
> removes most of the benefit of being able to override the write. This
> would allow "matching" methods for writing and reading.

This makes sense, and having a method instead of a function here
certainly seems cleaner...

Did anybody with a deeper knowledge of the eieio machinery provide any
feedback?  It's a quite large patch...

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

Message #13 received at 29541 <at> debbugs.gnu.org (full text, mbox):

From: Lars Ingebrigtsen <larsi <at> gnus.org>
To: Eric Abrahamsen <eric <at> ericabrahamsen.net>
Cc: 29541 <at> debbugs.gnu.org
Subject: Re: bug#29541: 27.0.50; Use generic function when restoring
 eieio-persistent, remove validation
Date: Mon, 10 Aug 2020 16:05:04 +0200

Lars Ingebrigtsen <larsi <at> gnus.org> writes:

>> 1. `object-write' is already a method. That means the writing process
>> can be overridden and customized, but the reading process can't, which
>> removes most of the benefit of being able to override the write. This
>> would allow "matching" methods for writing and reading.
>
> This makes sense, and having a method instead of a function here
> certainly seems cleaner...
>
> Did anybody with a deeper knowledge of the eieio machinery provide any
> feedback?  It's a quite large patch...

There wasn't any feedback here, so I think that you should go ahead and
apply the patch, Eric.  I think it makes sense conceptually, at least.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

Message #16 received at 29541 <at> debbugs.gnu.org (full text, mbox):

From: Eric Abrahamsen <eric <at> ericabrahamsen.net>
To: Lars Ingebrigtsen <larsi <at> gnus.org>
Cc: 29541 <at> debbugs.gnu.org
Subject: Re: bug#29541: 27.0.50; Use generic function when restoring
 eieio-persistent, remove validation
Date: Thu, 27 Aug 2020 18:40:11 -0700

[Message part 1 (text/plain, inline)]

Lars Ingebrigtsen <larsi <at> gnus.org> writes:

> Lars Ingebrigtsen <larsi <at> gnus.org> writes:
>
>>> 1. `object-write' is already a method. That means the writing process
>>> can be overridden and customized, but the reading process can't, which
>>> removes most of the benefit of being able to override the write. This
>>> would allow "matching" methods for writing and reading.
>>
>> This makes sense, and having a method instead of a function here
>> certainly seems cleaner...
>>
>> Did anybody with a deeper knowledge of the eieio machinery provide any
>> feedback?  It's a quite large patch...
>
> There wasn't any feedback here, so I think that you should go ahead and
> apply the patch, Eric.  I think it makes sense conceptually, at least.

This was quite a while ago, and the original patch no longer applies.
I've done up the same changes as two separate patches: one removing
extra validation, and the other creating a generic function for
constructing eieio-persistent objects. All the tests pass.

[0002-New-eieio-persistent-make-instance-generic-function.patch (text/x-patch, attachment)]

[0001-Remove-redundant-slot-validation-in-eieio-persistent.patch (text/x-patch, attachment)]

Message #19 received at 29541 <at> debbugs.gnu.org (full text, mbox):

From: Eric Abrahamsen <eric <at> ericabrahamsen.net>
To: Lars Ingebrigtsen <larsi <at> gnus.org>
Cc: 29541 <at> debbugs.gnu.org
Subject: Re: bug#29541: 27.0.50; Use generic function when restoring
 eieio-persistent, remove validation
Date: Thu, 27 Aug 2020 18:41:29 -0700

[Message part 1 (text/plain, inline)]

Lars Ingebrigtsen <larsi <at> gnus.org> writes:

> Lars Ingebrigtsen <larsi <at> gnus.org> writes:
>
>>> 1. `object-write' is already a method. That means the writing process
>>> can be overridden and customized, but the reading process can't, which
>>> removes most of the benefit of being able to override the write. This
>>> would allow "matching" methods for writing and reading.
>>
>> This makes sense, and having a method instead of a function here
>> certainly seems cleaner...
>>
>> Did anybody with a deeper knowledge of the eieio machinery provide any
>> feedback?  It's a quite large patch...
>
> There wasn't any feedback here, so I think that you should go ahead and
> apply the patch, Eric.  I think it makes sense conceptually, at least.

This was quite a while ago, and the original patch no longer applies.
I've done up the same changes as two separate patches: one removing
extra validation, and the other creating a generic function for
constructing eieio-persistent objects. All the tests pass.

[0002-New-eieio-persistent-make-instance-generic-function.patch (text/x-patch, attachment)]

[0001-Remove-redundant-slot-validation-in-eieio-persistent.patch (text/x-patch, attachment)]

Message #22 received at 29541 <at> debbugs.gnu.org (full text, mbox):

From: Lars Ingebrigtsen <larsi <at> gnus.org>
To: Eric Abrahamsen <eric <at> ericabrahamsen.net>
Cc: 29541 <at> debbugs.gnu.org
Subject: Re: bug#29541: 27.0.50; Use generic function when restoring
 eieio-persistent, remove validation
Date: Fri, 28 Aug 2020 16:18:04 +0200

Eric Abrahamsen <eric <at> ericabrahamsen.net> writes:

> This was quite a while ago, and the original patch no longer applies.
> I've done up the same changes as two separate patches: one removing
> extra validation, and the other creating a generic function for
> constructing eieio-persistent objects. All the tests pass.

I've just skimmed the code, but it looks good to me.  I'd say just apply
it and then see whether there is any fallout (but I don't expect there
to be).

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

Message #25 received at 29541 <at> debbugs.gnu.org (full text, mbox):

From: Eric Abrahamsen <eric <at> ericabrahamsen.net>
To: Lars Ingebrigtsen <larsi <at> gnus.org>
Cc: 29541 <at> debbugs.gnu.org, 29541-done <at> debbugs.gnu.org
Subject: Re: bug#29541: 27.0.50; Use generic function when restoring
 eieio-persistent, remove validation
Date: Fri, 28 Aug 2020 08:24:10 -0700

On 08/28/20 16:18 PM, Lars Ingebrigtsen wrote:
> Eric Abrahamsen <eric <at> ericabrahamsen.net> writes:
>
>> This was quite a while ago, and the original patch no longer applies.
>> I've done up the same changes as two separate patches: one removing
>> extra validation, and the other creating a generic function for
>> constructing eieio-persistent objects. All the tests pass.
>
> I've just skimmed the code, but it looks good to me.  I'd say just apply
> it and then see whether there is any fallout (but I don't expect there
> to be).

I've also used this with the gnus registry and EBDB, which should
exercise it fairly well. I'll push now.

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.