Skip to content
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

Document the Termination trait for main() and test functions #1194

Merged
merged 1 commit into from
Apr 22, 2022

Conversation

mattheww
Copy link
Contributor

Closes #1193

There doesn't seem to be a satisfactory way to document when tests are considered to pass: the implementation checks whether ExitCode::to_i32() returns 0, but to_i32() is intentionally hidden and marked as perma-unstable, and nothing in ExitCode's docs provides a notion of a "successful" code.

So I've vaguely said "whether the resulting ExitCode represents successful termination", and listed some specific cases.

@ehuss
Copy link
Contributor

ehuss commented Apr 18, 2022

Thanks!

Although they specifically did not stabilize to_i32, an ExitCode can be created from any arbitrary u8:

use std::process::ExitCode;

fn main() -> ExitCode {
    ExitCode::from(123)
}

#[test]
fn failing() -> ExitCode {
    ExitCode::from(99)
}

So, at least for tests, I think it should be defined as any non-zero exit code (that's how I wrote it in the tests chapter).

Also, something I didn't really think about is whether or not to document the return code of the process for the exit code of main. I think it probably should be documented, but it seems like there could be some platform-specific issues here.

Comment on lines +27 to +30
* Tests that return `()` pass as long as they terminate and do not panic.
* Tests that return a `Result<(), E>` pass as long as they return `Ok(())`.
* Tests that return `ExitCode::SUCCESS` pass, and tests that return `ExitCode::FAILURE` fail.
* Tests that do not terminate neither pass nor fail.
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm concerned this may be over-specifying the behavior. There's a bit of an awkward separation between "Rust the language" and "the standard library". The reference typically tends to focus on just the language behavior, and not delve into the standard library's implementation.

Also, I'm not really comfortable with specifying the non-termination behavior. I would consider that a failure, and it is possible in the future that either libtest or cargo could have some sort of timeout controls which would consider the test to fail.

I think leaving this up to the Termination docs in the standard library should be sufficient.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The "Tests that do not terminate neither pass nor fail." sentence was present already; this PR only moves it.

@mattheww
Copy link
Contributor Author

I've filed #1196 with my suggestion for how what to say about the process exit status.

@mattheww
Copy link
Contributor Author

I don't think writing "non-zero exit code" works, because the standard library docs don't say that all exit codes have a numeric value, and in particular they don't say that SUCCESS is zero and FAILURE isn't (presumably because they want it be possible for them not to be on some platforms).

I would be much happier if the Reference text could simply defer to an ExitCode::is_success() method, but that's evidently not an option for Rust 1.61.

If we want to avoid the Reference having a list like the one in this PR, perhaps an alternative is to make the standard library docs classify exit codes as "successful" or "unsuccessful", even in the absence of a public way to retrieve that classification.

@ehuss
Copy link
Contributor

ehuss commented Apr 21, 2022

Good points! And thanks for opening that issue, it does seem like something that needs some input.

Then I think I'm good with this PR as-is. Did you want to make any edits, or is it ready to go?

@mattheww
Copy link
Contributor Author

I think this is ready to go.

Copy link
Contributor

@ehuss ehuss left a comment

Choose a reason for hiding this comment

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

Thanks!

@ehuss ehuss merged commit 9f0de65 into rust-lang:master Apr 22, 2022
bors added a commit to rust-lang-ci/rust that referenced this pull request May 11, 2022
Update books

## nomicon

9 commits in c7d8467ca9158da58ef295ae65dbf00a308752d9..10d40c59a581c66d8ecd29ad18d410bf97ed524d
2022-04-06 14:26:54 +0900 to 2022-05-07 10:45:07 +0900
- Introducing init/uninit before its use (rust-lang/nomicon#355)
- Change will to would to discuss what don't occur (rust-lang/nomicon#361)
- State that pop for length 1 is an example (rust-lang/nomicon#360)
- Correct a sentence that didn't seem to be proper (rust-lang/nomicon#358)
- Indicate that C reference are C reference (rust-lang/nomicon#357)
- Introduce and avoid dropck (rust-lang/nomicon#353)
- Rephrase improperly reduced borrows introduction (rust-lang/nomicon#352)
- Two lifetime clarification (rust-lang/nomicon#350)
- "UB" vs "Undefined Behavior" (rust-lang/nomicon#349)

## reference

9 commits in b5f6c2362baf932db9440fbfcb509b309237ee85..8e36971959ff238b5aa2575fbc7a2e09e1313e82
2022-04-10 19:19:51 -0700 to 2022-05-09 17:20:59 -0700
- Stop saying that const functions cannot use 'extern' (rust-lang/reference#1207)
- Moved the option variant imports (rust-lang/reference#1208)
- #[must_use] on traits also affects trait objects (rust-lang/reference#1203)
- Don't use PathPattern in RangePattern bounds (rust-lang/reference#1204)
- Inline assembly: Add kreg0 register class (rust-lang/reference#1205)
- Fix crate_type attribute examples (rust-lang/reference#1201)
- Say that that the default function return type is the unit type (rust-lang/reference#1199)
- Clarify guarantees provided by repr(packed) (rust-lang/reference#1163)
- Document the Termination trait for main() and test functions (rust-lang/reference#1194)

## book

43 commits in de0dbffc5812fd885700874e8d258dd334733ac4..d9415b7cbfcb4b24062683f429bd0ff535396362
2022-04-18 19:29:45 -0400 to 2022-05-09 09:10:44 -0400
- Update ch09-02-recoverable-errors-with-result.md
- Added missing be 2
- Added missing be
- Move hardcoded string into status_line to be consistent
- Fix trailing space
- Propagate tech review edits back to src
- Change "semantics" to "mechanics"; when referring to compiler behavior, rather than syntax.
- Propagate some edits to ch4 snapshot
- Suggestions from tech review
- Propagate edits to src
- Propagate edits back to nostarch version
- Clarify sentences about lock types. Fixes rust-lang/book#2937.
- Edits to edits to chapter 16
- Edits from nostarch for chapter 16
- Propagate nostarch edits back to src
- Add words to dictionary
- Propagating edits back to the nostarch snapshot
- Small wording change. Fixes rust-lang/book#3112.
- Clarify the kind of manual cleanup meant here
- Edits to edits to chapter 15
- Edits from nostarch
- Add missing word
- Improve sentence structure
- fix unidiomatic new functions in chapter 15
- Propagate nostarch ch14 to src
- Update a link and the -p publishing instructions
- Actually, I don't think we need to show the command output here
- Edits to edits to chapter 14
- Update manual regeneration instructions
- Reflect the addition of the -p flag in Cargo 1.56 in chapter 14
- Change polarity and names of variables in env var section
- Propagate nostarch edits back to ch 12
- Change environment variable and field name to perhaps be less confusing
- Responses to nostarch edits
- Merge remote-tracking branch 'origin/ch13'
- Fix rust-lang/book#3002 use noplayground with common.rs
- Propagate ch3 edits to src
- Updating chapter 3 to use new println style
- Specify loop label format. Fixes rust-lang/book#3105.
- Clarify function definition must be in an accessible scope. Fixes rust-lang/book#3003
- Addressing tech review comments, propagating other changes
- Comments from tech review
- Chapter 3, section 2 - Add explicit type annotation to example of scalar type char.

## rust-by-example

6 commits in 44a80e8d8bfc5881c9bd69a2cb3a570776ee4181..e9f93cfcf410bc092c9107b8a41a82f144c761f2
2022-04-19 07:46:28 -0300 to 2022-05-08 18:24:06 -0300
- Add empty slice example (rust-lang/rust-by-example#1538)
- Enhancement/print (rust-lang/rust-by-example#1536)
- Update cast.md (rust-lang/rust-by-example#1521)
- Update iter_any.md (rust-lang/rust-by-example#1522)
- Update tuples.md (rust-lang/rust-by-example#1524)
- fix indent in fs.md (rust-lang/rust-by-example#1535)

## rustc-dev-guide

8 commits in 043e60f4f191651e9f8bf52fa32df14defbb23d9..0c02acdb6f48f03907a02ea8e537c3272b4fde9f
2022-04-20 18:57:49 +0900 to 2022-05-10 09:45:31 -0300
- Update overview.md (rust-lang/rustc-dev-guide#1351)
- Update date references on parallel-rustc (rust-lang/rustc-dev-guide#1348)
- mention `WithOptConstParam` (rust-lang/rustc-dev-guide#1346)
- Fix format (rust-lang/rustc-dev-guide#1349)
- correct type of SubstsRef (rust-lang/rustc-dev-guide#1347)
- Document ErrorGuaranteed (rust-lang/rustc-dev-guide#1316)
- Edit "What the compiler does to your code" (rust-lang/rustc-dev-guide#1306)
- Update some date refs
@ehuss ehuss mentioned this pull request Jun 30, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Update for Termination trait
2 participants