-
Notifications
You must be signed in to change notification settings - Fork 110
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
What is the reason for C-RW-VALUE
?
#174
Comments
I see a reason against it, for every such function down the call stack, you'll add one reference to the reader/writer object. This is inefficient if the compiler doesn't notice that and can rewrite function signatures (which is only the case if it knows all call sites). E.g. just having two levels of such functions results in |
Performance was not a motivation for the guideline. It is chiefly about clarity and simplicity. Compare the amount of noise in these signatures: use std::io::Write;
fn write_by_value<W: Write>(_w: W) {}
fn write_by_mut_ref<W: ?Sized + Write>(_w: &mut W) {} Counting everything but the function names, the second one is 60% more characters and all of that is noise. It's not even more usable -- the noise amounts to making the function usable with strictly fewer types. Compare also the call site: let mut buffer = [0u8; 256];
write_by_value(&mut buffer[..]);
// oh my god
write_by_mut_ref(&mut &mut buffer[..]); The mut ref one is hard to use because Write implementations are built around the expectation that the implementor is passed by value. By value is also more suitable for writer adapters: write_by_value(Adapter::new(&mut underlying));
// :(
write_by_mut_ref(&mut Adapter::new(&mut underlying)); |
Hm. Can we point out the disadvantages in this approach in the docs? |
Performance caveats are a bit of a cross-cutting concern for API design that probably impact a number of guidelines, not just this one. I'll go ahead and close this one, but but I think there's room for another resource on performance-sympathetic tactics. |
https://rust-lang-nursery.github.io/api-guidelines/interoperability.html#c-rw-value
https://github.com/rust-lang-nursery/api-guidelines/blob/05a1d5e5568606442c770919d167513dbbf78024/src/interoperability.md#generic-readerwriter-functions-take-r-read-and-w-write-by-value-c-rw-value
The text was updated successfully, but these errors were encountered: