From: Eric Abrahamsen <eric@ericabrahamsen.net>
To: 28489@debbugs.gnu.org
Subject: bug#28489: Acknowledgement (27.0.50; eieio-persistent slot type validation should be a bit smarter)
Date: Wed, 27 Sep 2017 22:02:58 -0700 [thread overview]
Message-ID: <87zi9fxvnh.fsf@ericabrahamsen.net> (raw)
In-Reply-To: <87lglcn8dt.fsf@ericabrahamsen.net>
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?
next prev parent reply other threads:[~2017-09-28 5:02 UTC|newest]
Thread overview: 13+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-09-18 0:43 bug#28489: 27.0.50; eieio-persistent slot type validation should be a bit smarter Eric Abrahamsen
[not found] ` <handler.28489.B.150569546424990.ack@debbugs.gnu.org>
2017-09-26 20:22 ` bug#28489: Acknowledgement (27.0.50; eieio-persistent slot type validation should be a bit smarter) Eric Abrahamsen
2017-09-27 0:05 ` Noam Postavsky
2017-09-27 16:39 ` Eric Abrahamsen
2017-09-28 2:23 ` Noam Postavsky
2017-09-28 5:02 ` Eric Abrahamsen [this message]
2017-09-29 0:35 ` Noam Postavsky
2017-09-29 20:31 ` Eric Abrahamsen
2017-09-30 0:57 ` Noam Postavsky
2017-09-30 18:05 ` Eric Abrahamsen
2017-09-30 21:58 ` Noam Postavsky
2017-09-30 23:30 ` Eric Abrahamsen
2017-10-14 12:13 ` Eric Abrahamsen
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=87zi9fxvnh.fsf@ericabrahamsen.net \
--to=eric@ericabrahamsen.net \
--cc=28489@debbugs.gnu.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this external index
https://git.savannah.gnu.org/cgit/emacs.git
https://git.savannah.gnu.org/cgit/emacs/org-mode.git
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.