-
Notifications
You must be signed in to change notification settings - Fork 506
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
Document method-call expressions in pseudo-code as well as prose #1432
Conversation
0ddce00
to
4d80b79
Compare
|
||
// Then, for each candidate `T`, add `&T` and `&mut T` to the list | ||
// immediately after `T`. | ||
let candidate_receiver_types = candidate_receiver_types.map(|T| [T, &T, &mut T]).flatten(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
flat_map
? I know this is just pseudo-Rust!
@rustbot labels -S-waiting-on-review +S-wating-on-author We discussed this in the rustdocs call today. On review, we're just not happy with the pseudocode here. It feels like it takes more effort to understand the pseudocode than the value that it's providing. Maybe another way to approach this would be more of an English pseudocode similar to what @ehuss wrote here: It might also be interesting if this pseudocode could be made non-pseudo. In doing that, it would be similar in spirit to:
We're still working out how exactly to tie those things into the Reference/Specification. But this, expanded to be runnnable, would seem to fall in the same category. |
Yeah, that's fair, I wasn't really happy with it myself, which is why I hadn't pursued this further. Thanks for taking the time to review it, I might still try to fix the linked issues, but will do so in another PR in any case. |
This section of the reference has been oversimplistic for some time (rust-lang#1018 and rust-lang#1534) and various rewrites have been attempted (e.g. rust-lang#1394, rust-lang#1432). Here's another attempt! My approach here is: * Stop trying to keep it short and concise * Document what actually happens in the code, step by step This does result in a long explanation, because we're trying to document nearly 2400 lines of code in `probe.rs`, but doing otherwise feels as though we'll continue to run into criticisms of oversimplification. This rewrite documents the post-arbitrary-self-types v2 situation, i.e. it assumes rust-lang/rust#135881 has landed. We should not merge this until or unless that lands. This PR was inspired by discussion in rust-lang#1699. If we go ahead with this approach, rust-lang#1699 becomes irrelevant. There was also discussion at rust-lang/cargo#15117 (comment)
I personally find the documentation on method-call expressions very hard to grok, and I suspect I'm not the only one, so I thought I would attempt to rewrite the section in pseudo-code, to hopefully make the algorithm clearer.
This is done as a precursor to fixing #718 and #1018, which is important for RFC 3519 where we're trying to change the algorithm.