-
Notifications
You must be signed in to change notification settings - Fork 13k
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
Clean up rustdoc's main()
#75124
Merged
Merged
Clean up rustdoc's main()
#75124
Conversation
This file contains 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
rust-highfive
added
the
S-waiting-on-review
Status: Awaiting review from the assignee but also interested parties.
label
Aug 4, 2020
ollie27
added
the
T-rustdoc
Relevant to the rustdoc team, which will review and decide on the PR/issue.
label
Aug 4, 2020
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
August 5, 2020 01:29
fabf953
to
5f8a112
Compare
I have addressed all the comments. |
@bors r+ |
📌 Commit 5f8a112 has been approved by |
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
Aug 5, 2020
☀️ Test successful - checks-actions, checks-azure |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Labels
merged-by-bors
This PR was explicitly merged by bors.
S-waiting-on-bors
Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
T-rustdoc
Relevant to the rustdoc team, which will review and decide on the PR/issue.
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