Skip to content

Conversation

matthiaskrgr
Copy link
Member

Successful merges:

Failed merges:

r? @ghost
@rustbot modify labels: rollup

Create a similar rollup

AronParker and others added 21 commits June 9, 2022 15:32
also use nice new format string syntax in interpret/error.rs
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
Implement ExitCodeExt for Windows

Fixes rust-lang#97914

### Motivation:

On Windows it is common for applications to return `HRESULT` (`i32`) or `DWORD` (`u32`) values. These stem from COM based components ([HRESULTS](https://docs.microsoft.com/en-us/windows/win32/api/objbase/nf-objbase-coinitialize)), Win32 errors ([GetLastError](https://docs.microsoft.com/en-us/windows/win32/api/errhandlingapi/nf-errhandlingapi-getlasterror)), GUI applications ([WM_QUIT](https://docs.microsoft.com/en-us/windows/win32/winmsg/wm-quit)) and more. The newly stabilized `ExitCode` provides an excellent fit for propagating these values, because `std::process::exit` does not run deconstructors which can result in errors. However, `ExitCode` currently only implements `From<u8> for ExitCode`, which disallows the full range of `i32`/`u32` values. This pull requests attempts to address that shortcoming by providing windows specific extensions that accept a `u32` value (which covers all possible `HRESULTS` and Win32 errors) analog to [ExitStatusExt::from_raw](https://doc.rust-lang.org/std/os/windows/process/trait.ExitStatusExt.html#tymethod.from_raw).

This was also intended by the original Stabilization rust-lang#93840 (comment)  as pointed out by ``@eggyal`` in rust-lang#97914 (comment):

> Issues around platform specific representations: We resolved this issue by changing the return type of report from i32 to the opaque type ExitCode. __That way we can change the underlying representation without affecting the API, letting us offer full support for platform specific exit code APIs in the future.__

[Emphasis added]

### API

```rust
/// Windows-specific extensions to [`process::ExitCode`].
///
/// This trait is sealed: it cannot be implemented outside the standard library.
/// This is so that future additional methods are not breaking changes.
#[stable(feature = "windows_process_exit_code_from", since = "1.63.0")]
pub trait ExitCodeExt: Sealed {
    /// Creates a new `ExitCode` from the raw underlying `u32` return value of
    /// a process.
    #[stable(feature = "windows_process_exit_code_from", since = "1.63.0")]
    fn from_raw(raw: u32) -> Self;
}

#[stable(feature = "windows_process_exit_code_from", since = "1.63.0")]
impl ExitCodeExt for process::ExitCode {
    fn from_raw(raw: u32) -> Self {
        process::ExitCode::from_inner(From::from(raw))
    }
}
```

### Misc

I apologize in advance if I misplaced any attributes regarding stabilzation, as far as I learned traits are insta-stable so I chose to make them stable. If this is an error, please let me know and I'll correct it. I also added some additional machinery to make it work, analog to [ExitStatus](https://doc.rust-lang.org/std/process/struct.ExitStatus.html#).

EDIT: Proposal: rust-lang/libs-team#48
interpret: use AllocRange in UninitByteAccess

also use nice new format string syntax in `interpret/error.rs`, and use the `#` flag to add `0x` prefixes where applicable.

r? ``@oli-obk``
replace process exit with more detailed exit in src/bootstrap/*.rs

Fixes [rust-lang#98830](rust-lang#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));
    }
}
```

<details>
<summary>% rg "exit\(" src/bootstrap/*.rs</summary>

```
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);
```
</details>
…chaelwoerister

suggest adding a derive for #[default] applied to variants

cc ``@TaKO8Ki`` as followup to rust-lang#98873.
… r=notriddle

Replace boolean argument for print_where_clause with an enum to make code more clear

As you suggested ``@notriddle.`` Just not sure if the naming seems good to you?

r? ``@notriddle``
@rustbot rustbot added T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-libs Relevant to the library team, which will review and decide on the PR/issue. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. rollup A PR which is a rollup labels Jul 7, 2022
@matthiaskrgr
Copy link
Member Author

@bors r+ rollup=never p=10

@bors
Copy link
Collaborator

bors commented Jul 7, 2022

📌 Commit ec0c156 has been approved by matthiaskrgr

It is now in the queue for this repository.

@bors bors added the S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. label Jul 7, 2022
@bors
Copy link
Collaborator

bors commented Jul 7, 2022

⌛ Testing commit ec0c156 with merge 1517f5d...

@bors
Copy link
Collaborator

bors commented Jul 7, 2022

☀️ Test successful - checks-actions
Approved by: matthiaskrgr
Pushing 1517f5d to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Jul 7, 2022
@bors bors merged commit 1517f5d into rust-lang:master Jul 7, 2022
@rustbot rustbot added this to the 1.64.0 milestone Jul 7, 2022
@rust-timer
Copy link
Collaborator

Finished benchmarking commit (1517f5d): comparison url.

Instruction count

  • Primary benchmarks: 🎉 relevant improvements found
  • Secondary benchmarks: 🎉 relevant improvements found
mean1 max count2
Regressions 😿
(primary)
N/A N/A 0
Regressions 😿
(secondary)
N/A N/A 0
Improvements 🎉
(primary)
-0.8% -0.9% 5
Improvements 🎉
(secondary)
-1.5% -2.3% 11
All 😿🎉 (primary) -0.8% -0.9% 5

Max RSS (memory usage)

This benchmark run did not return any relevant results for this metric.

Cycles

Results
  • Primary benchmarks: no relevant changes found
  • Secondary benchmarks: mixed results
mean1 max count2
Regressions 😿
(primary)
N/A N/A 0
Regressions 😿
(secondary)
11.0% 11.0% 1
Improvements 🎉
(primary)
N/A N/A 0
Improvements 🎉
(secondary)
-3.0% -3.8% 7
All 😿🎉 (primary) N/A N/A 0

If you disagree with this performance assessment, please file an issue in rust-lang/rustc-perf.

@rustbot label: -perf-regression

Footnotes

  1. the arithmetic mean of the percent change 2

  2. number of relevant changes 2

@matthiaskrgr matthiaskrgr deleted the rollup-8ygpcpg branch July 30, 2022 10:32
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. rollup A PR which is a rollup S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-libs Relevant to the library team, which will review and decide on the PR/issue. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue.

Projects

None yet

Development

Successfully merging this pull request may close these issues.