Better docs and associated SUCCESS/FAILURE for process::ExitCode#48618

scottmcm · 2018-02-28T19:32:41Z

Follow-up to #48497 (comment), since that PR was the minimal thing to unblock #48453 (comment).

scottmcm · 2018-02-28T21:08:44Z

src/libstd/process.rs

+/// the [`SUCCESS`] and [`FAILURE`] associated items.
+///
+/// [`SUCCESS`]: #constant.SUCCESS
+/// [`FAILURE`]: #constant.FAILURE


[01:29:50] std/process/struct.ExitCode.html:68: broken link fragment `#constant.SUCCESS` pointing to `std/process/struct.ExitCode.html` [01:29:50] std/process/struct.ExitCode.html:68: broken link fragment `#constant.FAILURE` pointing to `std/process/struct.ExitCode.html` [01:29:56] thread 'main' panicked at 'found some broken links', tools/linkchecker/main.rs:49:9

nikomatsakis · 2018-03-01T19:01:53Z

src/libstd/process.rs

+#[cfg(target_arch = "wasm32")]
+mod rawexit {
+    pub const SUCCESS: i32 = 0;
+    pub const FAILURE: i32 = 1;


do we still need this?

cc @alexcrichton

Er I'm not sure I added this... This actually should be rejected by the tidy lint as well...

In any case seems fine to have if it's needed, we can clean it up later

This private mod is originally from https://github.com/rust-lang/rust/pull/46479/files#diff-7dbed9a9f4aeab33f13e792758ac1ed5R13

Did you mean not have the wasm check? Or not have the mod? I can do either or both...

Er sorry what I mean is that we have lint which forbids #[cfg] in most of the standard library in an attempt to centralize all platform-specific code to src/libstd/sys rather than having it sprinkled throughout the standard library itself.

It's ok to not fix this up here, but if you're feeling enterprising this would just involve moving the modules to src/libstd/sys and using constants from there rather than this location.

I think the main question I had was "does wasm offer the standard constants" -- note though that this code is pre-existing, though it's been moved around, so the lint must not work that well...

nikomatsakis

r=me, but i'd like to know whether we still need the "wasm exception"

nikomatsakis · 2018-03-01T19:02:26Z

src/libstd/process.rs

+///
+/// **Warning**: While various forms of this were discussed in [RFC #1937],
+/// it was ultimately cut from that RFC, and thus this type is more subject
+/// to change even than the usual unstable item churn.


as an aside, I just ran into a place where I really wanted this today. I was already calling exit(1) -- so it seems silly not to provide a way to "return" that...

nikomatsakis · 2018-03-02T22:02:37Z

@scottmcm do you want to make the changes @alexcrichton suggested?

Now begins the saga of fixing compilation errors on other platforms...

scottmcm · 2018-03-04T06:55:14Z

Ok, hopefully this is good now. Made a new tracking issue for the type, #48711

Travis job to see if I typo'd anything in wasm: https://travis-ci.org/rust-lang/rust/builds/348817793

nikomatsakis · 2018-03-05T21:02:53Z

I can't really comment on the imp module business.

r? @alexcrichton

alexcrichton · 2018-03-05T21:44:41Z

@bors: r+

bors · 2018-03-05T21:44:42Z

📌 Commit 74c5c6e has been approved by alexcrichton

@nikomatsakis

…crichton Better docs and associated SUCCESS/FAILURE for process::ExitCode Follow-up to rust-lang#48497 (comment), since that PR was the minimal thing to unblock rust-lang#48453 (comment). r? @nikomatsakis

Rollup of 9 pull requests - Successful merges: #48511, #48549, #48618, #48624, #48651, #48698, #48778, #48787, #48802 - Failed merges: #48669, #48710

…atrieb `pal::unsupported::process::ExitCode`: use an `u8` instead of a `bool` `ExitCode` should “represents the status code the current process can return to its parent under normal termination”, but is currently represented as a `bool` on unsupported platforms, making the `impl From<u8> for ExitCode` lossy. Fixes rust-lang#130532. History: [IRLO thread](https://internals.rust-lang.org/t/mini-pre-rfc-redesigning-process-exitstatus/5426) (`ExitCode` as a `main` return), rust-lang#48618 (initial impl), rust-lang#93445 (`From<u8>` impl).

Rollup merge of rust-lang#130554 - ShE3py:unsupported-exitcode, r=Noratrieb `pal::unsupported::process::ExitCode`: use an `u8` instead of a `bool` `ExitCode` should “represents the status code the current process can return to its parent under normal termination”, but is currently represented as a `bool` on unsupported platforms, making the `impl From<u8> for ExitCode` lossy. Fixes rust-lang#130532. History: [IRLO thread](https://internals.rust-lang.org/t/mini-pre-rfc-redesigning-process-exitstatus/5426) (`ExitCode` as a `main` return), rust-lang#48618 (initial impl), rust-lang#93445 (`From<u8>` impl).

Put some thought and documentation effort into process::ExitCode

fc2e4e7

rust-highfive assigned nikomatsakis Feb 28, 2018

rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Feb 28, 2018

scottmcm commented Feb 28, 2018

View reviewed changes

Fix linkchecker

2ce2b40

nikomatsakis reviewed Mar 1, 2018

View reviewed changes

nikomatsakis approved these changes Mar 1, 2018

View reviewed changes

Move process::ExitCode internals to sys

74c5c6e

Now begins the saga of fixing compilation errors on other platforms...

scottmcm force-pushed the elaborate-exitcode branch from 61cc4f0 to 74c5c6e Compare March 4, 2018 02:45

scottmcm mentioned this pull request Mar 4, 2018

Tracking issue for std::process::ExitCode (feature process_exitcode_placeholder) #48711

Closed

1 task

scottmcm changed the title ~~Better docs and associated SUCCESS/FAILURE for process::ExitCode~~ [DO NOT MERGE] Better docs and associated SUCCESS/FAILURE for process::ExitCode Mar 4, 2018

scottmcm force-pushed the elaborate-exitcode branch from 8610f07 to 74c5c6e Compare March 4, 2018 06:53

scottmcm changed the title ~~[DO NOT MERGE] Better docs and associated SUCCESS/FAILURE for process::ExitCode~~ Better docs and associated SUCCESS/FAILURE for process::ExitCode Mar 4, 2018

nikomatsakis approved these changes Mar 5, 2018

View reviewed changes

rust-highfive assigned alexcrichton and unassigned nikomatsakis Mar 5, 2018

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 Mar 5, 2018

alexcrichton mentioned this pull request Mar 7, 2018

Rollup of 9 pull requests #48806

Merged

bors added a commit that referenced this pull request Mar 7, 2018

Auto merge of #48806 - alexcrichton:rollup, r=alexcrichton

6675913

Rollup of 9 pull requests - Successful merges: #48511, #48549, #48618, #48624, #48651, #48698, #48778, #48787, #48802 - Failed merges: #48669, #48710

bors added a commit that referenced this pull request Mar 7, 2018

Auto merge of #48806 - alexcrichton:rollup, r=alexcrichton

29d7fc1

Rollup of 9 pull requests - Successful merges: #48511, #48549, #48618, #48624, #48651, #48698, #48778, #48787, #48802 - Failed merges: #48669, #48710

bors added a commit that referenced this pull request Mar 7, 2018

Auto merge of #48806 - alexcrichton:rollup, r=alexcrichton

5430c0c

Rollup of 9 pull requests - Successful merges: #48511, #48549, #48618, #48624, #48651, #48698, #48778, #48787, #48802 - Failed merges: #48669, #48710

alexcrichton merged commit 74c5c6e into rust-lang:master Mar 8, 2018

scottmcm deleted the elaborate-exitcode branch March 8, 2018 01:54

ShE3py mentioned this pull request Sep 19, 2024

pal::unsupported::process::ExitCode: use an u8 instead of a bool #130554

Merged

Uh oh!

Conversation

scottmcm commented Feb 28, 2018

Uh oh!

scottmcm Feb 28, 2018

Choose a reason for hiding this comment

Uh oh!

nikomatsakis Mar 1, 2018

Choose a reason for hiding this comment

Uh oh!

alexcrichton Mar 1, 2018

Choose a reason for hiding this comment

Uh oh!

scottmcm Mar 1, 2018

Choose a reason for hiding this comment

Uh oh!

alexcrichton Mar 1, 2018

Choose a reason for hiding this comment

Uh oh!

nikomatsakis Mar 2, 2018

Choose a reason for hiding this comment

Uh oh!

nikomatsakis left a comment

Choose a reason for hiding this comment

Uh oh!

nikomatsakis Mar 1, 2018

Choose a reason for hiding this comment

Uh oh!

nikomatsakis commented Mar 2, 2018

Uh oh!

scottmcm commented Mar 4, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

nikomatsakis commented Mar 5, 2018

Uh oh!

alexcrichton commented Mar 5, 2018

Uh oh!

bors commented Mar 5, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

scottmcm commented Mar 4, 2018 •

edited

Loading