Clean up std::task docs, make TaskBuilder a real builder

[lilyball]

Delete all the documentation from std::task that references linked
failure.

Tweak TaskBuilder to be more builder-like. .name() is now .named() and
.add_wrapper() is now .with_wrapper(). Remove .watched() and
.unwatched() as they didn't actually do anything.

Closes #6399.

[alexcrichton]

This is a fairly core module, so I'd like to discuss this a bit before merging. I'm in favor of the changes.

Could you also delete #[allow(missing_doc)] and fixup the other related fields? I can fill in any missing documentation if anything is unclear.

[lilyball]

@alexcrichton My understanding based on some of the documentation is that TaskBuilder was always intended to operate in this fashion. There was some documentation referencing e.g. task().supervised().spawn(..). I think that behavior was simply accidentally lost when linked failure was removed.

I couldn't make .future_result() behave the same way, of course, since it returns a Port, and I wasn't sure whether .add_wrapper() should be changed to .with_wrapper() so I left that alone as well.

As for missing documentation, I wasn't sure what to write for the logger, stdout, and stderr fields. The latter two obviously affect the stdout/stderr of that task, but that seems like a fuzzily-defined thing, since I'm not really sure what operations actually use task-local stdout/stderr. io::stdout()/io::stderr() certainly don't. As for logger, I have no idea what that does.

If you want to give me a description for logger, I think I can do the rest (for stdout/stderr I suppose I can just say that it sets the task-local stdout/stderr, and not bother trying to clarify that because io::set_stdout() certainly doesn't).

[lilyball]

Ok, I guess logger simply affects the std::logging module, which I assume means debug!(), etc.

[lilyball]

Pushed new documentation. Running make check now but a cursory look shows all public items are documented.

[huonw]

FWIW, apropos of nothing in particular: something that's bothered me for a while is (with the appropriate imports) task::task() rather than, say, Task::new(), or even task::builder().

[lilyball]

Dammit! Too easy to accidentally hit that button. Sigh.

[alexcrichton]

• stdout - used for println! and friends, equivalent to calling set_stdout in the new task.
• stderr - used for fail!() messages, equivalent to calling set_stderr in the new task.
• logger - used for all logging in the new tasks, equivalent o calling set_logger in the new task.

[alexcrichton]

Taking another look at this, I fear that this builder pattern isn't quite as useful as it used to be. Neither watched nor unwatched are actually used when spawning, so the only "builder method" is named, which seems a bit odd. You can't use the builder to configure anything inside of TaskOpts.

I fear that this api still needs a fairly large overhaul because there's still lots of little gotchas here and there. I think that the builder approach is where we want to converge on, but I'm not 100% sure of that.

I think we should go ahead with this PR, but I think we have aways to go for finishing std::task

[brson]

Glad to see activity to clean up std::task. Onwards!

[flaper87]

I agree with what @alexcrichton's comment, however, I think we should let this PR land. I like the changes in this PR.

Just one nit, though. Although all tasks are watched by default, I'd still like to see a test that explicitly uses watched. There may be usages of that method out there and I'd like to make sure we've a test for it in the code base. Also, this needs rebase.

[lilyball]

I'm not actually entirely sure what watched()/unwatched() do. The only reasonable guess is that a lifecycle notification for a task is delayed until all spawned watched tasks finish, except that the lack of linked failure means that I would be surprised to get an Err() back from the parent task if it did not fail itself.

[alexcrichton]

watch and unwatched are ignored, neither libnative nor libgreen look at them

[lilyball]

@alexcrichton Should they be removed then?

[alexcrichton]

Yes.

[flaper87]

+1 for removing them if they're not actually being used anywhere.

[sfackler]

This closes #6399

[lilyball]

Rebased and removed watched/unwatched.

[alexcrichton]

I believe this is still true and shouldn't be deleted.

[alexcrichton]

r=me with this taken care of

[lilyball]

Oops.