Skip to content

replace process exit with more detailed exit in src/bootstrap/*.rs - #98994

Merged
bors merged 1 commit into
rust-lang:masterfrom
kons-9:dont_use_process_exit
Jul 7, 2022
Merged

replace process exit with more detailed exit in src/bootstrap/*.rs#98994
bors merged 1 commit into
rust-lang:masterfrom
kons-9:dont_use_process_exit

Conversation

@kons-9

Copy link
Copy Markdown
Contributor

Fixes#98830

I implemeted "detail_exit.rs" in lib.rs, and replace all of std::process::exit.
So, error code should panic in test code.

// lib.rs
pub fn detail_exit(code: i32) -> ! {
// Successful exit
if code == 0 {
std::process::exit(0);
}
if cfg!(test) {
panic!("status code: {}", code);
} else {
std::panic::resume_unwind(Box::new(code));
}
}
% rg "exit\(" src/bootstrap/*.rs
builder.rs
351: crate::detail_exit(1);
1000: crate::detail_exit(1);
1429: crate::detail_exit(1);
compile.rs
1331: crate::detail_exit(1);
config.rs
818: crate::detail_exit(2);
1488: crate::detail_exit(1);
flags.rs
263: crate::detail_exit(exit_code);
349: crate::detail_exit(exit_code);
381: crate::detail_exit(1);
602: crate::detail_exit(1);
616: crate::detail_exit(1);
807: crate::detail_exit(1);
format.rs
35: crate::detail_exit(1);
117: crate::detail_exit(1);
lib.rs
714: detail_exit(1);
1620: detail_exit(1);
1651:pub fn detail_exit(code: i32) -> ! {
1654: std::process::exit(0);
sanity.rs
107: crate::detail_exit(1);
setup.rs
97: crate::detail_exit(1);
290: crate::detail_exit(1);
test.rs
676: crate::detail_exit(1);
1024: crate::detail_exit(1);
1254: crate::detail_exit(1);
tool.rs
207: crate::detail_exit(1);
toolstate.rs
96: crate::detail_exit(3);
111: crate::detail_exit(1);
182: crate::detail_exit(1);
228: crate::detail_exit(1);
util.rs
339: crate::detail_exit(1);
378: crate::detail_exit(1);
468: crate::detail_exit(1);

@rust-highfive

Copy link
Copy Markdown
Contributor

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @jyn514 (or someone else) soon.

Please see the contribution instructions for more information.

@rust-highfiverust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jul 7, 2022

@jyn514jyn514 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks, this is great! After you address the comments, can you squash the commits? There are instructions at https://rustc-dev-guide.rust-lang.org/git.html#advanced-rebasing.

Comment threadsrc/bootstrap/lib.rs Outdated
Comment threadsrc/bootstrap/lib.rs Outdated
@jyn514jyn514 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 Jul 7, 2022
@jyn514

Copy link
Copy Markdown
Member

@kons-9 I see you added a new "squash the commits" commit but that didn't actually squash them. After squashing you should have exactly one commit in the PR. Make sure you use git push --force instead of merging.

implement detail_exit but I'm not sure it is right.
not create new file and write detail exit in lib.rs
replace std::process::exit to detail_exit
that is not related to code runnning.
remove pub
@kons-9
kons-9force-pushed the dont_use_process_exit branch from 93615e4 to d6de276CompareJuly 7, 2022 04:43
@kons-9

Copy link
Copy Markdown
ContributorAuthor

Sorry for the mishandling of the operation. I think this is probably done.

@jyn514

Copy link
Copy Markdown
Member

@bors r+

Thanks for the PR!

@bors

bors commented Jul 7, 2022

Copy link
Copy Markdown
Collaborator

📌 Commit d6de276 has been approved by jyn514

@borsbors 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 Jul 7, 2022
bors added a commit to rust-lang-ci/rust that referenced this pull request Jul 7, 2022
…askrgr
Rollup of 9 pull requests
Successful merges:
- rust-lang#97917 (Implement ExitCodeExt for Windows)
- rust-lang#98844 (Reword comments and rename HIR visiting methods.)
- rust-lang#98979 (interpret: use AllocRange in UninitByteAccess)
- rust-lang#98986 (Fix missing word in comment)
- rust-lang#98994 (replace process exit with more detailed exit in src/bootstrap/*.rs)
- rust-lang#98995 (Add a test for rust-lang#80471)
- rust-lang#99002 (suggest adding a derive for #[default] applied to variants)
- rust-lang#99004 (Add a test for rust-lang#70408)
- rust-lang#99017 (Replace boolean argument for print_where_clause with an enum to make code more clear)
Failed merges:
r? `@ghost`
`@rustbot` modify labels: rollup
@bors
bors merged commit 6742dc4 into rust-lang:masterJul 7, 2022
@rustbotrustbot added this to the 1.64.0 milestone Jul 7, 2022
compiler-errors added a commit to compiler-errors/rust that referenced this pull request Aug 10, 2022
Fixes bootstrap panic when running x fmt --check
closesrust-lang#100258 wherein bootstrap panics when running x fmt --check. Fixed by replacing resume_unwind in rust-lang#98994. with process::exit.
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

S-waiting-on-borsStatus: 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.

Don't use process::exit in bootstrap tests

5 participants

@kons-9@rust-highfive@jyn514@bors@rustbot