Merged
Conversation
rust-highfive
added
the
S-waiting-on-review
Contributor
Author
ollie27
added
the
T-rustdoc
oli-obk
reviewed
Aug 4, 2020
oli-obk
reviewed
Aug 4, 2020
oli-obk
reviewed
Aug 4, 2020
rustdoc's `main()` immediately spawns a thread, M, with a large stack (16MiB or 32MiB) on which it runs `main_args()`. `main_args()` does a small amount of options processing and then calls `setup_callbacks_and_run_in_default_thread_pool_with_globals()`, which spawns it own thread, and M is not used further. So, thread M seems unnecessary. However, it does serve a purpose: if the options processing in `main_args()` panics, that panic is caught when M is joined. So M can't simply be removed. However, `main_options()`, which is called by `main_args()`, has a `catch_fatal_errors()` call within it. We can move that call to `main()` and change it to the very similar `catch_with_exit_code()`. With that in place, M can be removed, and panics from options processing will still be caught appropriately. Even better, this makes rustdoc's `main()` match rustc's `main()`, which also uses `catch_with_exit_code()`. (Also note that the use of a 16MiB/32MiB stack was eliminated from rustc in rust-lang#55617.)
It's a very thin wrapper around `setup_callbacks_and_run_in_thread_pool_with_globals()` and it has a single call site.
`run()` returns `Result<(), String>`. But on failure it always returns an empty string, and then `wrap_return()` treats an empty string specially, by not reporting the error. It turns out we already have the `ErrorReported` type for this sort of behaviour. This commit changes `run()` to use it.
nnethercote
force-pushed
the
clean-up-rustdoc-main
branch
from
fabf953 to
5f8a112
Compare
Contributor
Author
|
I have addressed all the comments. |
Contributor
|
@bors r+ |
Collaborator
|
📌 Commit 5f8a112 has been approved by |
bors
added
S-waiting-on-bors
Collaborator
Collaborator
|
☀️ Test successful - checks-actions, checks-azure |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
6 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
It can be simplified and made more similar to rustc's
main().r? @oli-obk