Skip to content
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

Add doc for impl From for Waker #53507

Merged
merged 3 commits into from
Oct 25, 2018
Merged

Add doc for impl From for Waker #53507

merged 3 commits into from
Oct 25, 2018

Conversation

phungleson
Copy link
Contributor

As part of issue #51430 (cc @skade).

The impl is very simple, so not sure if we need to go into any details.

@rust-highfive
Copy link
Collaborator

r? @withoutboats

(rust_highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Aug 19, 2018
@rust-highfive

This comment has been minimized.

@dtolnay
Copy link
Member

dtolnay commented Aug 20, 2018

@bors delegate=skade

@bors
Copy link
Contributor

bors commented Aug 20, 2018

✌️ @skade can now approve this pull request

@pietroalbini
Copy link
Member

Ping from triage @skade! This PR needs your review.

@TimNN
Copy link
Contributor

TimNN commented Sep 4, 2018

Ping from triage @skade / @rust-lang/docs: This PR requires your review.

@GuillaumeGomez
Copy link
Member

Please add an example.

@phungleson
Copy link
Contributor Author

Hey, I am currently traveling, will look at this again in around 3 weeks.

@GuillaumeGomez
Copy link
Member

No problem, we'll wait for you!

@TimNN TimNN added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Sep 11, 2018
@TimNN TimNN added A-allocators Area: Custom and system allocators and removed A-allocators Area: Custom and system allocators labels Sep 25, 2018
@phungleson
Copy link
Contributor Author

@GuillaumeGomez writing examples for this seems harder than I expected, e.g. creating LocalWake of some sort of UnsafeWake

Do you have any good examples or good documents about these things?

And also not sure with all movement of futures api, are these struct gonna be necessary?

@GuillaumeGomez
Copy link
Member

Nothing coming to my mind right now... In the worst case, just show a code example demonstrating how the From call works, no need for it to be pertinent if that requires too much code.

@frewsxcv
Copy link
Member

In the worst case, just show a code example demonstrating how the From call works, no need for it to be pertinent if that requires too much code.

@GuillaumeGomez Do you know how to create a LocalWaker? It's not obvious to me and that's necessary for the example you're describing

@GuillaumeGomez
Copy link
Member

Never used it so no. Maybe someone from the @rust-lang/libs or @rust-lang/compiler might know?

@cramertj
Copy link
Member

cramertj commented Oct 1, 2018

You can use LocalWaker::new, local_waker_from_nonlocal, or local_waker.

@frewsxcv
Copy link
Member

frewsxcv commented Oct 7, 2018

Even with cramertj's links, it's not immediately clear how to instantiate these for doc example purposes, so this PR seems good as-is. Thanks for your contribution @phungleson!

@bors r+ rollup

@bors
Copy link
Contributor

bors commented Oct 7, 2018

📌 Commit a84782cb0785b3b3f6cb5af73b5256bffa2a4083 has been approved by frewsxcv

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Oct 7, 2018
@bors
Copy link
Contributor

bors commented Oct 7, 2018

⌛ Testing commit a84782cb0785b3b3f6cb5af73b5256bffa2a4083 with merge 29d791f6c6c7ee2dbbed0b5b132c0fa14387baa1...

@bors
Copy link
Contributor

bors commented Oct 7, 2018

💔 Test failed - status-travis

@bors bors added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Oct 7, 2018
@rust-highfive
Copy link
Collaborator

The job x86_64-gnu-llvm-5.0 of your PR failed on Travis (raw log). Through arcane magic we have determined that the following fragments from the build log may contain information about the problem.

Click to expand the log.

[01:24:28] travis_time:end:stage0-linkchecker:start=1538940267499346160,finish=1538940269785948454,duration=2286602294

[01:24:28] [TIMING] ToolBuild { compiler: Compiler { stage: 0, host: "x86_64-unknown-linux-gnu" }, target: "x86_64-unknown-linux-gnu", tool: "linkchecker", path: "src/tools/linkchecker", mode: ToolBootstrap, is_optional_tool: false, source_type: InTree, extra_features: [] } -- 2.287
[01:26:45] std/convert/trait.From.html:318: broken link - std/convert/struct.LocalWaker.html
[01:26:45] std/convert/trait.From.html:318: broken link - std/convert/struct.Waker.html
[01:27:08] core/convert/trait.From.html:67: broken link - core/convert/struct.LocalWaker.html
[01:27:08] core/convert/trait.From.html:67: broken link - core/convert/struct.Waker.html
[01:28:00] thread 'main' panicked at 'found some broken links', tools/linkchecker/main.rs:49:9
[01:28:00] 
[01:28:00] 
[01:28:00] command did not execute successfully: "/checkout/obj/build/x86_64-unknown-linux-gnu/stage0-tools-bin/linkchecker" "/checkout/obj/build/x86_64-unknown-linux-gnu/doc"
[01:28:00] expected success, got: exit code: 101
[01:28:00] expected success, got: exit code: 101
[01:28:00] 
[01:28:00] 
[01:28:00] failed to run: /checkout/obj/build/bootstrap/debug/bootstrap test
[01:28:00] Build completed unsuccessfully in 0:43:45
[01:28:00] Makefile:58: recipe for target 'check' failed
[01:28:00] make: *** [check] Error 1

The command "stamp sh -x -c "$RUN_SCRIPT"" exited with 2.
travis_time:start:01ea2aad
$ date && (curl -fs --head https://google.com | grep ^Date: | sed 's/Date: //g' || true)
---
travis_time:end:00a51977:start=1538940493862311019,finish=1538940493872628388,duration=10317369
travis_fold:end:after_failure.3
travis_fold:start:after_failure.4
travis_time:start:37e300c6
$ ln -s . checkout && for CORE in obj/cores/core.*; do EXE=$(echo $CORE | sed 's|obj/cores/core\.[0-9]*\.!checkout!\(.*\)|\1|;y|!|/|'); if [ -f "$EXE" ]; then printf travis_fold":start:crashlog\n\033[31;1m%s\033[0m\n" "$CORE"; gdb --batch -q -c "$CORE" "$EXE" -iex 'set auto-load off' -iex 'dir src/' -iex 'set sysroot .' -ex bt -ex q; echo travis_fold":"end:crashlog; fi; done || true
travis_fold:end:after_failure.4
travis_fold:start:after_failure.5
travis_time:start:19af462d
travis_time:start:19af462d
$ cat ./obj/build/x86_64-unknown-linux-gnu/native/asan/build/lib/asan/clang_rt.asan-dynamic-i386.vers || true
cat: ./obj/build/x86_64-unknown-linux-gnu/native/asan/build/lib/asan/clang_rt.asan-dynamic-i386.vers: No such file or directory
travis_fold:end:after_failure.5
travis_fold:start:after_failure.6
travis_time:start:0348026a
$ dmesg | grep -i kill

I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact @TimNN. (Feature Requests)

@pietroalbini
Copy link
Member

@bors r-

@bors bors added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Oct 7, 2018
@TimNN
Copy link
Contributor

TimNN commented Oct 16, 2018

Ping form triage @phungleson: It looks like your PR failed on travis and needs to updated.

@TimNN TimNN added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Oct 23, 2018
@TimNN
Copy link
Contributor

TimNN commented Oct 23, 2018

Ping from triage @withoutboats: It looks like this PR is now ready for your review.

@@ -188,6 +188,10 @@ impl LocalWaker {
}

impl From<LocalWaker> for Waker {
/// Converts a `LocalWaker` into a `Waker`.
///
/// This conversion forgets local waker and allocates a new waker with
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

allocate is misleading here, since no new allocation is occurring. What about "This conversion turns a !Sync LocalWaker into a Sync Waker, allowing a wakeup object to be sent to another thread, but giving up its ability to do specialized thread-local wakeup behavior."

@phungleson
Copy link
Contributor Author

Thanks @cramertj it is updated

@cramertj
Copy link
Member

@bors r+ rollup

@bors
Copy link
Contributor

bors commented Oct 23, 2018

📌 Commit 3539132 has been approved by cramertj

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Oct 23, 2018
kennytm added a commit to kennytm/rust that referenced this pull request Oct 24, 2018
… r=cramertj

Add doc for impl From for Waker

As part of issue rust-lang#51430 (cc @skade).

The impl is very simple, so not sure if we need to go into any details.
pietroalbini added a commit to pietroalbini/rust that referenced this pull request Oct 25, 2018
… r=cramertj

Add doc for impl From for Waker

As part of issue rust-lang#51430 (cc @skade).

The impl is very simple, so not sure if we need to go into any details.
bors added a commit that referenced this pull request Oct 25, 2018
Rollup of 22 pull requests

Successful merges:

 - #53507 (Add doc for impl From for Waker)
 - #53931 (Gradually expanding libstd's keyword documentation)
 - #54965 (update tcp stream documentation)
 - #54977 (Accept `Option<Box<$t:ty>>` in macro argument)
 - #55138 (in which unused-parens suggestions heed what the user actually wrote)
 - #55173 (Suggest appropriate syntax on missing lifetime specifier in return type)
 - #55200 (Documents `From` implementations for `Stdio`)
 - #55245 (submodules: update clippy from 5afdf8b to b1d0343)
 - #55247 (Clarified code example in char primitive doc)
 - #55251 (Fix a typo in the documentation of RangeInclusive)
 - #55253 (only issue "variant of the expected type" suggestion for enums)
 - #55254 (Correct trailing ellipsis in name_from_pat)
 - #55269 (fix typos in various places)
 - #55282 (Remove redundant clone)
 - #55285 (Do some copy editing on the release notes)
 - #55291 (Update stdsimd submodule)
 - #55296 (Set RUST_BACKTRACE=0 for rustdoc-ui/failed-doctest-output.rs)
 - #55306 (Regression test for #54478.)
 - #55328 (Fix doc for new copysign functions)
 - #55340 (Operands no longer appear in places)
 - #55345 (Remove is_null)
 - #55348 (Update RELEASES.md after destabilization of non_modrs_mods)

Failed merges:

r? @ghost
@bors bors merged commit 3539132 into rust-lang:master Oct 25, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

10 participants