bug#28489: Acknowledgement (27.0.50; eieio-persistent slot type validation should be a bit smarter)

[Eric Abrahamsen]

Noam Postavsky <npostavs@users.sourceforge.net> writes:

[...]

>> This whole section of code, `eieio-persistent-validate/fix-slot-value'
>> and its neighbors, feels very seat-of-the-pants to me. I wish `cl-typep'
>> could handle more of this work. But in the meantime this patch (or
>> something like it) would at least address the actual bug.
>
> Hmm, to be honest I can't quite make out what this function is actually
> being used for.

It's basically a first pass at slot validation that is specific to the
way eieio-persistent writes objects to disk. It can't write actual
objects, obviously, so it writes object construction forms. Before the
"real" validation happens in `eieio--perform-slot-validation', the
persistent read process needs to make sure the construction forms will
produce objects of the right class, then eval the forms.

So if you've got a slot that's supposed to be of type (list-of foo), the
persistence pre-validation looks at the value read from file, sees that
it's (list (foo ...) (foo ...)), evals those construction forms, and
then what the "real" validation ends up with is (cl-typep (list #<foo>
#<foo>) '(list-of 'foo)), which passes.

Essentially it is validating twice, both before and after the actual
objects are created. I don't have a very firm grasp of all the code
involved, but in principle I would prefer just to eval all object
construction forms regardless, and then let it blow up at "real"
validation time -- it was going to blow up anyway.

Another option would be to make use of `eieio-skip-typecheck', which, if
non-nil, could be interpreted as saying to skip all slot-related type
checking, including in persistent object read.

But again, my patch or something like it would be enough to get
everything working as advertised.

>> I don't think the tabs were my fault! What's Emacs policy on this?
>
> I believe the policy is that new code should use spaces (although
> sometimes people ignore this, it's not a big deal), but don't touch
> lines just for the sake of changing the whitespace.

Good to know, thanks. In lines of code I've added, indentation will be
done with spaces, but I suppose that's okay?