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

API Docs: thread #29378

Open
steveklabnik opened this Issue Oct 26, 2015 · 24 comments

Comments

Projects
None yet
6 participants
@steveklabnik
Member

steveklabnik commented Oct 26, 2015

Part of #29329

http://doc.rust-lang.org/std/thread/

Here's what needs to be done to close out this issue:

  • the module docs are written a while ago, and contain some questionableish advice. It's not actually bad, it's just not the way I'd orient all of this. A particularly enthusiastic contributor could toss this out and re-write it from scratch. If the rest of this work gets done, we can close the ticket without doing this, but it would be nice.
  • Builder's docs are... okay. Just very uninspired.
  • JoinHandle doesn't go into enough detail, and should show off, for example, the detach behavior.
  • LocalKey could use some links and general cleanup.
  • Thread has very little and very boring docs.
  • panicking could use some more advice on when to use this.
  • park should have its module documentation inlined here, and cleaned up.
  • park_timeout could use links to park
  • spawn needs a lot more docs generally. It's the main interface to this module!
  • yield_now doesn't explain any of the "why".
  • Result should at least show off the "use std::thread; thread::Result" pattern, like all module-specific Result types.
@steveklabnik

This comment has been minimized.

Member

steveklabnik commented Mar 8, 2017

I am happy to mentor anyone who wants to tackle this issue.

@mcarpenter09

This comment has been minimized.

Contributor

mcarpenter09 commented Mar 25, 2017

I would be interested in taking on the documentation improvement of Builder as a starter. I'd take on more, but no need to get ahead of myself.

@steveklabnik

This comment has been minimized.

Member

steveklabnik commented Mar 25, 2017

@oldmanmike awesome! Please let me know if you need anything.

@ghost

This comment has been minimized.

ghost commented Mar 29, 2017

I am not (by any means) an expert on this topic but I'd love to help on this one, as @steveklabnik says: writting docs is the best way to learn something

@ghost

This comment has been minimized.

ghost commented Apr 6, 2017

I believe I will be taking the joinHandle and Thead docs

bors added a commit that referenced this issue May 3, 2017

Auto merge of #41543 - z1mvader:master, r=steveklabnik
Rewrote the thread struct docs

#29378

frewsxcv added a commit to frewsxcv/rust that referenced this issue May 3, 2017

@rap2hpoutre

This comment has been minimized.

Contributor

rap2hpoutre commented May 4, 2017

I would like to tackle this issue too and add an example on Result. Feel free to stop me if you are already working on it! (but it seems nobody took it right now)

@rap2hpoutre

This comment has been minimized.

Contributor

rap2hpoutre commented May 4, 2017

Ok so I still need some mentoring! Here is a (working) example I quickly wrote for std::thread::Result.

use std::thread;

fn main() {
    let child = thread::spawn(move || panic!("Oups"));

    let res: thread::Result<()> = child.join();

    match res {
        Ok(_) => println!("this is fine"),
        Err(_) => println!("thread panicked"),
    }
}

I think (but not sure) this example could help because type is explicit, so the user know what he can do with this std::thread::Result.

@steveklabnik asked:

Result should at least show off the "use std::thread; thread::Result" pattern, like all module-specific Result types.

But I'm not sure to understand what does "use std::thread; thread::Result" means. I read an other example in std on io::Result: https://doc.rust-lang.org/std/io/type.Result.html but it seems different, I preferred wrote something more specific to threads.

Anyway, I'm not sure my example follows best practices. And I'm not sure it's useful. So, once again @steveklabnik please help! :)

@steveklabnik

This comment has been minimized.

Member

steveklabnik commented May 4, 2017

But I'm not sure to understand what does "use std::thread; thread::Result" means. I read an other example in std on io::Result: https://doc.rust-lang.org/std/io/type.Result.html but it seems different, I preferred wrote something more specific to threads.

It's what you wrote, specifically

use std::thread;

let res: thread::Result<()> ...

that is, you use this style instead of use std::thread::Result; which would be more conventional generally speaking. The reason why is so you don't shadow std::result::Result from the prelude.

Anyway, I'm not sure my example follows best practices. And I'm not sure it's useful. So, once again @steveklabnik please help! :)

I think this example is okay, but we might be able to make it better! What about a function that does the spawn and join, and returns the result? then, you wouldn't be adding the extra type for no reason, as the function signature would use it. This is similar to how the Write trait has methods that return their Results.

The question is what that function should do...

@rap2hpoutre

This comment has been minimized.

Contributor

rap2hpoutre commented May 4, 2017

What about a function that does the spawn and join, and returns the result?

Okay, I think you mean:

use std::thread;

fn spawn_and_join() -> thread::Result<()> {
    let child = thread::spawn(move || panic!("Oups"));
    child.join()
}

fn main() {
    match spawn_and_join() {
        Ok(_) => println!("this is fine"),
        Err(_) => println!("thread panicked"),
    }
}

But...

The question is what that function should do...

From there I'm a bit lost: my example has still no sense (what's that spawn_and_join function and why?), but I don't know how to create something with sense in a few lines. Maybe I should call something that could panic? Something like:

fn open_file_in_thread() -> thread::Result<()> {
    let child = thread::spawn(move || {
        File::open("foo.txt").unwrap();
        // etc.
    });
    child.join()
}

But it's a "not real" example either. Should I give up?

EDIT: Maybe a file copy in a thread could be more realistic: fs::copy("foo.txt", "bar.txt") with a copy_in_thread function

use std::thread;
use std::fs;

fn copy_in_thread() -> thread::Result<()> {
    thread::spawn(move || { fs::copy("foo.txt", "bar.txt").unwrap(); }).join()
}

fn main() {
    match copy_in_thread() {
        Ok(_) => println!("this is fine"),
        Err(_) => println!("thread panicked"),
    }
}

frewsxcv added a commit to frewsxcv/rust that referenced this issue May 4, 2017

Rollup merge of rust-lang#41741 - rap2hpoutre:patch-3, r=steveklabnik
join method returns a thread::Result

Join method returns a std::thread::Result, not a std::result::Result: https://doc.rust-lang.org/std/thread/struct.JoinHandle.html#method.join Maybe I misunderstood something.

I have seen this mistake(?) because I wanted to tackle this issue rust-lang#29378 (about Result). It's still one of my first PR. Sorry if I missed something.

frewsxcv added a commit to frewsxcv/rust that referenced this issue May 5, 2017

Rollup merge of rust-lang#41741 - rap2hpoutre:patch-3, r=steveklabnik
join method returns a thread::Result

Join method returns a std::thread::Result, not a std::result::Result: https://doc.rust-lang.org/std/thread/struct.JoinHandle.html#method.join Maybe I misunderstood something.

I have seen this mistake(?) because I wanted to tackle this issue rust-lang#29378 (about Result). It's still one of my first PR. Sorry if I missed something.

frewsxcv added a commit to frewsxcv/rust that referenced this issue May 5, 2017

Rollup merge of rust-lang#41741 - rap2hpoutre:patch-3, r=steveklabnik
join method returns a thread::Result

Join method returns a std::thread::Result, not a std::result::Result: https://doc.rust-lang.org/std/thread/struct.JoinHandle.html#method.join Maybe I misunderstood something.

I have seen this mistake(?) because I wanted to tackle this issue rust-lang#29378 (about Result). It's still one of my first PR. Sorry if I missed something.
@gamazeps

This comment has been minimized.

Contributor

gamazeps commented May 5, 2017

@steveklabnik Hey ! I'm a bit rusty (haven't rusted in a year or so), so I'm looking to get back to it by helping with the docs, which item still need documentation in the list ?

Cheers

@steveklabnik

This comment has been minimized.

Member

steveklabnik commented May 5, 2017

@gamazeps wonderful! Everything except the last one is still open, and that's because there's a PR open, but not merged, hence no checkmark.

@gamazeps

This comment has been minimized.

Contributor

gamazeps commented May 5, 2017

@steveklabnik Ok, sign me in for:

  • Thread has very little and very boring docs.
  • panicking could use some more advice on when to use this.
  • park should have its module documentation inlined here, and cleaned up.
  • park_timeout could use links to park
  • spawn needs a lot more docs generally. It's the main interface to this module!

Should be done by wednesdy in the worst case scenario :)

bors added a commit that referenced this issue May 6, 2017

Auto merge of #41768 - rap2hpoutre:patch-4, r=frewsxcv
Add an example to std::thread::Result type

This PR is a part of #29378. I submit this PR with the help (mentoring) of @steveklabnik. I'm still not sure my request is good enough but I don't want to spoil the issue with too much questions so I continue here. r? @steveklabnik

gamazeps added a commit to gamazeps/rust that referenced this issue May 7, 2017

Mark-Simulacrum added a commit to Mark-Simulacrum/rust that referenced this issue May 14, 2017

Rollup merge of rust-lang#41994 - gamazeps:thread-builder, r=Guillaum…
…eGomez

[Doc] Implrove `thread::Builder`'s doc.

Part of rust-lang#29378 .

- Explains *why* we would use the builder instead ofthe free function.
- Changes the parent-child explanation for a spawned-caller in `thread::Builder::spawn`
- Adds a link to `io::Result` in `thread::Builder`
- Corrects the return type doc in `thread::Builder::spawn`

r? @rust-lang/docs

gamazeps added a commit to gamazeps/rust that referenced this issue May 15, 2017

gamazeps added a commit to gamazeps/rust that referenced this issue May 15, 2017

Expands `detach` documentation in `thread::JoinHande`.
Part of rust-lang#29378 .

- Adds an example of a thread detaching.
- Expands what `detaching` means.

gamazeps added a commit to gamazeps/rust that referenced this issue May 15, 2017

Expands `detach` documentation in `thread::JoinHande`.
Part of rust-lang#29378 .

- Adds an example of a thread detaching.
- Expands what `detaching` means.

frewsxcv added a commit to frewsxcv/rust that referenced this issue May 15, 2017

Rollup merge of rust-lang#41982 - gamazeps:thread-yield-now, r=Guilla…
…umeGomez

[Doc] Explain why `thread::yield_now` could be used.

Part of rust-lang#29378.

r? @steveklabnik

frewsxcv added a commit to frewsxcv/rust that referenced this issue May 15, 2017

Rollup merge of rust-lang#41994 - gamazeps:thread-builder, r=Guillaum…
…eGomez

[Doc] Implrove `thread::Builder`'s doc.

Part of rust-lang#29378 .

- Explains *why* we would use the builder instead ofthe free function.
- Changes the parent-child explanation for a spawned-caller in `thread::Builder::spawn`
- Adds a link to `io::Result` in `thread::Builder`
- Corrects the return type doc in `thread::Builder::spawn`

r? @rust-lang/docs

frewsxcv added a commit to frewsxcv/rust that referenced this issue May 15, 2017

Rollup merge of rust-lang#41995 - gamazeps:thread-localkey, r=frewsxcv
[Doc] Add links to the `thread::LocalKey` doc.

Part of rust-lang#29378 .

I do not know exactly what should be done for the `cleanup` part, if you have any idea I'll gladly do it.

r? @rust-lang/docs

frewsxcv added a commit to frewsxcv/rust that referenced this issue May 15, 2017

Rollup merge of rust-lang#41982 - gamazeps:thread-yield-now, r=Guilla…
…umeGomez

[Doc] Explain why `thread::yield_now` could be used.

Part of rust-lang#29378.

r? @steveklabnik

frewsxcv added a commit to frewsxcv/rust that referenced this issue May 15, 2017

Rollup merge of rust-lang#41994 - gamazeps:thread-builder, r=Guillaum…
…eGomez

[Doc] Implrove `thread::Builder`'s doc.

Part of rust-lang#29378 .

- Explains *why* we would use the builder instead ofthe free function.
- Changes the parent-child explanation for a spawned-caller in `thread::Builder::spawn`
- Adds a link to `io::Result` in `thread::Builder`
- Corrects the return type doc in `thread::Builder::spawn`

r? @rust-lang/docs

frewsxcv added a commit to frewsxcv/rust that referenced this issue May 15, 2017

Rollup merge of rust-lang#41995 - gamazeps:thread-localkey, r=frewsxcv
[Doc] Add links to the `thread::LocalKey` doc.

Part of rust-lang#29378 .

I do not know exactly what should be done for the `cleanup` part, if you have any idea I'll gladly do it.

r? @rust-lang/docs
@steveklabnik

This comment has been minimized.

Member

steveklabnik commented May 15, 2017

@gamazeps thank you so much! ❤️

frewsxcv added a commit to frewsxcv/rust that referenced this issue May 16, 2017

Rollup merge of rust-lang#41982 - gamazeps:thread-yield-now, r=Guilla…
…umeGomez

[Doc] Explain why `thread::yield_now` could be used.

Part of rust-lang#29378.

r? @steveklabnik

frewsxcv added a commit to frewsxcv/rust that referenced this issue May 16, 2017

Rollup merge of rust-lang#41994 - gamazeps:thread-builder, r=Guillaum…
…eGomez

[Doc] Implrove `thread::Builder`'s doc.

Part of rust-lang#29378 .

- Explains *why* we would use the builder instead ofthe free function.
- Changes the parent-child explanation for a spawned-caller in `thread::Builder::spawn`
- Adds a link to `io::Result` in `thread::Builder`
- Corrects the return type doc in `thread::Builder::spawn`

r? @rust-lang/docs

frewsxcv added a commit to frewsxcv/rust that referenced this issue May 16, 2017

Rollup merge of rust-lang#41995 - gamazeps:thread-localkey, r=frewsxcv
[Doc] Add links to the `thread::LocalKey` doc.

Part of rust-lang#29378 .

I do not know exactly what should be done for the `cleanup` part, if you have any idea I'll gladly do it.

r? @rust-lang/docs

Mark-Simulacrum added a commit to Mark-Simulacrum/rust that referenced this issue May 16, 2017

Rollup merge of rust-lang#41982 - gamazeps:thread-yield-now, r=Guilla…
…umeGomez

[Doc] Explain why `thread::yield_now` could be used.

Part of rust-lang#29378.

r? @steveklabnik

Mark-Simulacrum added a commit to Mark-Simulacrum/rust that referenced this issue May 16, 2017

Rollup merge of rust-lang#41994 - gamazeps:thread-builder, r=Guillaum…
…eGomez

[Doc] Implrove `thread::Builder`'s doc.

Part of rust-lang#29378 .

- Explains *why* we would use the builder instead ofthe free function.
- Changes the parent-child explanation for a spawned-caller in `thread::Builder::spawn`
- Adds a link to `io::Result` in `thread::Builder`
- Corrects the return type doc in `thread::Builder::spawn`

r? @rust-lang/docs

Mark-Simulacrum added a commit to Mark-Simulacrum/rust that referenced this issue May 16, 2017

Rollup merge of rust-lang#41995 - gamazeps:thread-localkey, r=frewsxcv
[Doc] Add links to the `thread::LocalKey` doc.

Part of rust-lang#29378 .

I do not know exactly what should be done for the `cleanup` part, if you have any idea I'll gladly do it.

r? @rust-lang/docs

Mark-Simulacrum added a commit to Mark-Simulacrum/rust that referenced this issue May 16, 2017

Rollup merge of rust-lang#41982 - gamazeps:thread-yield-now, r=Guilla…
…umeGomez

[Doc] Explain why `thread::yield_now` could be used.

Part of rust-lang#29378.

r? @steveklabnik

Mark-Simulacrum added a commit to Mark-Simulacrum/rust that referenced this issue May 16, 2017

Rollup merge of rust-lang#41994 - gamazeps:thread-builder, r=Guillaum…
…eGomez

[Doc] Implrove `thread::Builder`'s doc.

Part of rust-lang#29378 .

- Explains *why* we would use the builder instead ofthe free function.
- Changes the parent-child explanation for a spawned-caller in `thread::Builder::spawn`
- Adds a link to `io::Result` in `thread::Builder`
- Corrects the return type doc in `thread::Builder::spawn`

r? @rust-lang/docs

Mark-Simulacrum added a commit to Mark-Simulacrum/rust that referenced this issue May 16, 2017

Rollup merge of rust-lang#41995 - gamazeps:thread-localkey, r=frewsxcv
[Doc] Add links to the `thread::LocalKey` doc.

Part of rust-lang#29378 .

I do not know exactly what should be done for the `cleanup` part, if you have any idea I'll gladly do it.

r? @rust-lang/docs

Mark-Simulacrum added a commit to Mark-Simulacrum/rust that referenced this issue May 24, 2017

Rollup merge of rust-lang#41980 - gamazeps:thread-send, r=steveklabnik
[Doc] Add `'static` and `Send` constraints explanations to `thread::spawn`

Part of rust-lang#29378.

Explains why the constraints on the closure and its return value are `'static` and `Send`.

Allows to tick of `thread::spawn` from the list of things to document in the `thread` module.

r? @steveklabnik

Mark-Simulacrum added a commit to Mark-Simulacrum/rust that referenced this issue May 24, 2017

Rollup merge of rust-lang#41980 - gamazeps:thread-send, r=steveklabnik
[Doc] Add `'static` and `Send` constraints explanations to `thread::spawn`

Part of rust-lang#29378.

Explains why the constraints on the closure and its return value are `'static` and `Send`.

Allows to tick of `thread::spawn` from the list of things to document in the `thread` module.

r? @steveklabnik

Mark-Simulacrum added a commit to Mark-Simulacrum/rust that referenced this issue May 24, 2017

Rollup merge of rust-lang#41980 - gamazeps:thread-send, r=steveklabnik
[Doc] Add `'static` and `Send` constraints explanations to `thread::spawn`

Part of rust-lang#29378.

Explains why the constraints on the closure and its return value are `'static` and `Send`.

Allows to tick of `thread::spawn` from the list of things to document in the `thread` module.

r? @steveklabnik

Mark-Simulacrum added a commit to Mark-Simulacrum/rust that referenced this issue May 25, 2017

Rollup merge of rust-lang#41980 - gamazeps:thread-send, r=steveklabnik
[Doc] Add `'static` and `Send` constraints explanations to `thread::spawn`

Part of rust-lang#29378.

Explains why the constraints on the closure and its return value are `'static` and `Send`.

Allows to tick of `thread::spawn` from the list of things to document in the `thread` module.

r? @steveklabnik

Mark-Simulacrum added a commit to Mark-Simulacrum/rust that referenced this issue May 25, 2017

Rollup merge of rust-lang#41980 - gamazeps:thread-send, r=steveklabnik
[Doc] Add `'static` and `Send` constraints explanations to `thread::spawn`

Part of rust-lang#29378.

Explains why the constraints on the closure and its return value are `'static` and `Send`.

Allows to tick of `thread::spawn` from the list of things to document in the `thread` module.

r? @steveklabnik

gamazeps added a commit to gamazeps/rust that referenced this issue Jun 2, 2017

Expands `detach` documentation in `thread::JoinHande`.
Part of rust-lang#29378 .

- Adds an example of a thread detaching.
- Expands what `detaching` means.

Mark-Simulacrum added a commit to Mark-Simulacrum/rust that referenced this issue Jun 2, 2017

Rollup merge of rust-lang#41981 - gamazeps:thread-detach, r=frewsxcv
[Doc] Expands `detach` documentation in `thread::JoinHande`.

Part of rust-lang#29378 .

- Adds an example of a thread detaching.
- Expands what `detaching` means.

r? @steveklabnik
@SeanPrashad

This comment has been minimized.

Contributor

SeanPrashad commented Dec 8, 2017

Hi @steveklabnik! I'd LOVE to help bring this across the finish line but with the last documentation issue for the module docs. I'd greatly appreciate some guidance with where to start if it's possible 🙂

@steveklabnik

This comment has been minimized.

Member

steveklabnik commented Dec 8, 2017

Hey @SeanPrashad that'd be amazing!

One option is

A particularly enthusiastic contributor could toss this out and re-write it from scratch.

Which would be a bunch of work but might be the way to go.

Basically, I don't think the docs are organized well; they're kind of just a random collection of thoughts. So I think the first task is, how do we want to organize these docs? Generally, good module docs show an overview of the module conceptually, and then highlight key APIs. Maybe we can start there?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment