Avoid creating references to invalid data #22
Conversation
|
Thanks very much! Let me get some time to walk through this, to make sure I grok what is going on, but the intent sounds very solid. |
|
Apologies for the big rebase, but it should ultimately be a net benefit: commits serve a clearer purpose (and can thus be reviewed one by one easily) and irrelevant changes were pushed out of this PR. |
|
No worries. Sorry for the delay in the review. I'm moving house this week, and employment is moving offices at the same time. |
| // One possibility would be a C++11-style combination of variadic generics and recursion. | ||
| let ($(ref mut $ty,)*) = *self_.as_ptr(); | ||
| $( | ||
| let field_ptr : NonNull<$ty> = From::from($ty); |
There was a problem hiding this comment.
My inner programming language nerd is amazed by the fact that Rust's grammar is so well designed that we can have a type and a variable with the same name and use these at the same time without any issues.
I also wish we didn't need this hack
|
By the way, this PR breaks the API of the (FWIW, #25 also breaks the API, so you may want to batch both into a single release if you choose to merge them) |
|
Ping @frankmcsherry, do you have more time to look into these PRs nowadays? |
This implements the changes discussed in #17 . Areas in the code where references are still necessary are properly annotated in the code, along with a discussion of what language-level fixes are needed to fix the problem in a zero-cost way.
(Non-zero cost ways are also envisionable, of course, for example
Vec<T>len could be dumped alongside the serialized data, but I fear that such unsatisfactory performance compromises go against the spirit of abomonation.)I also moved all tests to
assert_eqso that they produce more exhaustive error messages and engaged in a few formatting tweaks which make the code more readable in my subjective opinion. Since looks are subjective, feel free to disagree and request me to revert any part of that ;)Fixes #17.