Open HadrienG2 opened 5 years ago
This seems very plausible! I'll check it out.
Investigation suggests that while this is fine for encode
, it could be a bit gross for the internals. The reason being if we accept a mut write: W
and need to use it multiple times, we would need to invoke recursive calls with &mut write
to avoid transferring ownership. This results in a recursive call with the write implementor &mut W
, and each bit of structural recursion layers on a new &mut
.
This seems like it would confound structural recursion through tree-structured data, e.g.
struct Tree {
children: Vec<Tree>,
}
Let's not do this then, as I said it's not a big issue ;)
Wait. Given that &mut W implements Write since W implements Write, what prevents you from sending around an &mut writer while traversing a tree ?
I think that is what happens now, isn't it? We could allow encode
to take a mut writer: W
but we probably (?) want entomb
to continue to take a writer: &mut W
. Maybe I mis-understand what you are proposing though.
To clarify, I meant that because &mut W implements Write just like owned writers, this code is legal:
pub fn takes_write_impl(_: impl Write) {}
pub fn also_takes_write_impl(mut w: impl Write) {
takes_write_impl(&mut w);
takes_write_impl(w);
}
Therefore, there is no problem with accepting "owned" writers, and using them multiple times.
Yup, I agree. However,
pub fn takes_write_impl(mut w: impl std::io::Write) {
takes_write_impl(&mut w)
}
fn main() {
let mut bytes = Vec::new();
takes_write_impl(&mut bytes);
}
trips a recursion limit
error: reached the recursion limit while instantiating `takes_write_impl::<&mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut std::vec::Vec<u8>>`
--> src/main.rs:1:1
|
1 | / pub fn takes_write_impl(mut w: impl std::io::Write) {
2 | | takes_write_impl(&mut w)
3 | | }
| |_^
error: aborting due to previous error
This is the problem I think exists, as we seem to need to use &mut writer
in the body of many of the writing methods (edit: which may recursively call themselves)
Oh, I see. This could cause problems with deep object trees indeed, especially as it also implies that the compiler is generating O(N) copies of take_write_impl when using this pattern, which is a compilation time liability.
Then I agree with you: encode
can take a writer by value, but entomb
shouldn't.
&mut T
whereT: Write
implementsWrite
, so there is no reason to ask for a borrowed writer in the API. But this is unlikely to be a problem in practice because owned writers are rarely used.