Enhance documentation on wake call memory ordering

[xmh0511]

View all comments

Add documentation about memory ordering requirements for wake calls. Try to fix #128920

[rustbot]

r? @Mark-Simulacrum

rustbot has assigned @Mark-Simulacrum.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

Why was this reviewer chosen?

The reviewer was selected based on:

• Owners of files modified in this PR: libs
• libs expanded to 7 candidates

[rustbot]

Reminder, once the PR becomes ready for a review, use @rustbot ready.

[Darksonn]

In general I would say that this documentation makes more sense on the Waker struct or the Future trait. The Wake trait is just a utility trait, and should not be the primary location for these kinds of docs (though it can be mentioned there too).

I would also relax the wording a bit. You use the word 'runtime must' and so on, but I think it would be useful to instead phrase it along the lines of 'to avoid missed wakeups, the runtime must' or similar to make it clear what the consequences of getting it wrong are.

[Darksonn]

Overall looks good to me.

View changes since this review

[Darksonn]

It looks like you intended to have a newline here, but markdown will not treat this as a newline.

Please either reflow without a line break or add an empty line if you want two paragraphs. (I probably would keep one paragraph.)

View changes since the review

[Darksonn]

Saying "returned wake" sounds like a value called wake is being returned, but that is not the case. Can you reword?

View changes since the review

[xmh0511]

Is "returning from the wake happens-before ..." good enough? Or, option B

there is a subsequent call to poll such that the wake() return happens-before

[Darksonn]

It's probably sufficient to say that "the call to wake() happens-before the beginning of the call to poll()".

[xmh0511]

@Darksonn I have closed this PR due to confliction, and created a new one, see #157694

[RalfJung]

It is generally a bad idea to create PRs from your main branch. Always create a new branch in your repo for every PR.

It is however not necessary to make a new PR, you can just force-push to that branch to remove the bad changes.

[rustbot]

This PR changes rustc_public

cc @oli-obk, @celinval, @ouz-a, @makai410

The run-make-support library was changed

cc @jieyouxu

miri is developed in its own repository. If possible, consider making this change to rust-lang/miri instead.

cc @rust-lang/miri

clippy is developed in its own repository. If possible, consider making this change to rust-lang/rust-clippy instead.

cc @rust-lang/clippy

[rustbot]

This PR was rebased onto a different main commit. Here's a range-diff highlighting what actually changed.

Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers.

[rust-log-analyzer]

The job x86_64-gnu-tools failed! Check out the build log: (web) (plain enhanced) (plain)

Click to see the possible cause of the failure (guessed by this bot)

error: encountered diff marker
  --> compiler/rustc_middle/src/queries.rs:61:1
   |
61 | <<<<<<< HEAD
   | ^^^^^^^ between this marker and `=======` is the code that you are merging into
...
64 | =======
   | ------- between this marker and `>>>>>>>` is the incoming code
65 | use rustc_errors::ErrorGuaranteed;
66 | >>>>>>> 665bf7d3863 (revert cargo fmt)
   | ^^^^^^^ this marker concludes the conflict region
   |
   = note: conflict markers indicate that a merge was started but could not be completed due to merge conflicts
           to resolve a conflict, keep only the code you want and then delete the lines containing conflict markers
   = help: if you are in a merge, the top section is the code you already had checked out and the bottom section is the new code
           if you are in a rebase, the top section is the code being rebased onto and the bottom section is the code you had checked out which is being rebased
   = note: for an explanation on these markers from the `git` documentation, visit <https://git-scm.com/book/en/v2/Git-Tools-Advanced-Merging#_checking_out_conflicts>

[RUSTC-TIMING] rustc_middle test:false 0.087
warning: `rustc_middle` (lib) generated 1 warning
error: could not compile `rustc_middle` (lib) due to 1 previous error; 1 warning emitted
warning: build failed, waiting for other jobs to finish...