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

Stabilize TryFrom and TryInto with a convert::Infallible empty enum #58302

Merged
merged 5 commits into from
Feb 25, 2019

Conversation

SimonSapin
Copy link
Contributor

This is the plan proposed in #33417 (comment)

@SimonSapin SimonSapin added the T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. label Feb 8, 2019
@rust-highfive
Copy link
Collaborator

r? @rkruppe

(rust_highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Feb 8, 2019
@SimonSapin
Copy link
Contributor Author

@rfcbot fcp merge

@rfcbot
Copy link

rfcbot commented Feb 8, 2019

Team member @SimonSapin has proposed to merge this. The next step is review by the rest of the tagged team members:

No concerns currently listed.

Once a majority of reviewers approve (and none object), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up!

See this document for info about what commands tagged team members can give me.

@rfcbot rfcbot added proposed-final-comment-period Proposed to merge/close by relevant subteam, see T-<team> label. Will enter FCP once signed off. disposition-merge This issue / PR is in PFCP or FCP with a disposition to merge it. labels Feb 8, 2019
@rust-highfive
Copy link
Collaborator

The job x86_64-gnu-llvm-6.0 of your PR failed on Travis (raw log). Through arcane magic we have determined that the following fragments from the build log may contain information about the problem.

Click to expand the log.
travis_time:end:04d689f0:start=1549634997151285181,finish=1549635146244413755,duration=149093128574
$ git checkout -qf FETCH_HEAD
travis_fold:end:git.checkout

Encrypted environment variables have been removed for security reasons.
See https://docs.travis-ci.com/user/pull-requests/#pull-requests-and-security-restrictions
$ export SCCACHE_BUCKET=rust-lang-ci-sccache2
$ export SCCACHE_REGION=us-west-1
Setting environment variables from .travis.yml
$ export IMAGE=x86_64-gnu-llvm-6.0
---
[00:58:49]     Checking compiler_builtins v0.1.5
[00:58:50]  Documenting alloc v0.0.0 (/checkout/src/liballoc)
[00:58:55]     Finished release [optimized] target(s) in 35.92s
[00:58:56]  Documenting core v0.0.0 (/checkout/src/libcore)
[00:59:12] warning: `[never]` cannot be resolved, ignoring it...
[00:59:12]     |
[00:59:12]     |
[00:59:12] 529 | /// This enum has the same role as [the `!` “never” type][never],
[00:59:12]     |
[00:59:12] note: lint level defined here
[00:59:12]    --> src/libcore/lib.rs:63:9
[00:59:12]     |
[00:59:12]     |
[00:59:12] 63  | #![warn(intra_doc_link_resolution_failure)]
[00:59:12]     |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
[00:59:12]     = help: to escape `[` and `]` characters, just add '\' before them like `\[` or `\]`
[00:59:12] 
[00:59:12] warning: `[never]` cannot be resolved, ignoring it...
[00:59:12]     |
[00:59:12]     |
[00:59:12] 529 | /// This enum has the same role as [the `!` “never” type][never],
[00:59:12]     |
[00:59:12]     = help: to escape `[` and `]` characters, just add '\' before them like `\[` or `\]`
[00:59:12] 
[00:59:18]     Finished release [optimized] target(s) in 23.19s
---
[00:59:24]     Checking rustc_tsan v0.0.0 (/checkout/src/librustc_tsan)
[00:59:24]     Checking rustc_msan v0.0.0 (/checkout/src/librustc_msan)
[00:59:24]     Checking rustc_asan v0.0.0 (/checkout/src/librustc_asan)
[00:59:24]  Documenting std v0.0.0 (/checkout/src/libstd)
[00:59:31] error: `[never]` cannot be resolved, ignoring it...
[00:59:31]     |
[00:59:31]     |
[00:59:31] 529 | /// This enum has the same role as [the `!` “never” type][never],
[00:59:31]     |
[00:59:31] note: lint level defined here
[00:59:31]    --> src/libstd/lib.rs:209:9
[00:59:31]     |
[00:59:31]     |
[00:59:31] 209 | #![deny(intra_doc_link_resolution_failure)]
[00:59:31]     |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
[00:59:31]     = help: to escape `[` and `]` characters, just add '\' before them like `\[` or `\]`
[00:59:31] 
[00:59:31] error: `[never]` cannot be resolved, ignoring it...
[00:59:31]     |
[00:59:31]     |
[00:59:31] 529 | /// This enum has the same role as [the `!` “never” type][never],
[00:59:31]     |
[00:59:31]     = help: to escape `[` and `]` characters, just add '\' before them like `\[` or `\]`
[00:59:31] 
[00:59:31] error: Could not document `std`.
[00:59:31] error: Could not document `std`.
[00:59:31] 
[00:59:31] Caused by:
[00:59:31]   process didn't exit successfully: `/checkout/obj/build/bootstrap/debug/rustdoc --crate-name std src/libstd/lib.rs --color always --target x86_64-unknown-linux-gnu -o /checkout/obj/build/x86_64-unknown-linux-gnu/stage1-std/x86_64-unknown-linux-gnu/doc --cfg 'feature="backtrace"' --cfg 'feature="backtrace-sys"' --cfg 'feature="compiler_builtins"' --cfg 'feature="compiler_builtins_c"' --cfg 'feature="default"' --cfg 'feature="panic-unwind"' --cfg 'feature="panic_unwind"' --markdown-css rust.css --markdown-no-toc --index-page /checkout/src/doc/index.md -L dependency=/checkout/obj/build/x86_64-unknown-linux-gnu/stage1-std/x86_64-unknown-linux-gnu/release/deps -L dependency=/checkout/obj/build/x86_64-unknown-linux-gnu/stage1-std/release/deps --extern alloc=/checkout/obj/build/x86_64-unknown-linux-gnu/stage1-std/x86_64-unknown-linux-gnu/release/deps/liballoc-9e6c0311b71511c6.rmeta --extern backtrace_sys=/checkout/obj/build/x86_64-unknown-linux-gnu/stage1-std/x86_64-unknown-linux-gnu/release/deps/libbacktrace_sys-f13b165ed9b4dd57.rmeta --extern compiler_builtins=/checkout/obj/build/x86_64-unknown-linux-gnu/stage1-std/x86_64-unknown-linux-gnu/release/deps/libcompiler_builtins-d54fe968dea87029.rmeta --extern core=/checkout/obj/build/x86_64-unknown-linux-gnu/stage1-std/x86_64-unknown-linux-gnu/release/deps/libcore-84e5b9599b1b7754.rmeta --extern libc=/checkout/obj/build/x86_64-unknown-linux-gnu/stage1-std/x86_64-unknown-linux-gnu/release/deps/liblibc-a69fda92b07aedd5.rmeta --extern panic_abort=/checkout/obj/build/x86_64-unknown-linux-gnu/stage1-std/x86_64-unknown-linux-gnu/release/deps/libpanic_abort-349a3e5cce9f18ee.rmeta --extern panic_unwind=/checkout/obj/build/x86_64-unknown-linux-gnu/stage1-std/x86_64-unknown-linux-gnu/release/deps/libpanic_unwind-d7c48504ff1056b6.rmeta --extern rustc_demangle=/checkout/obj/build/x86_64-unknown-linux-gnu/stage1-std/x86_64-unknown-linux-gnu/release/deps/librustc_demangle-20814bca47e9a554.rmeta --extern rustc_asan=/checkout/obj/build/x86_64-unknown-linux-gnu/stage1-std/x86_64-unknown-linux-gnu/release/deps/librustc_asan-8bc4c4bf1d4ec8f0.rmeta --extern rustc_lsan=/checkout/obj/build/x86_64-unknown-linux-gnu/stage1-std/x86_64-unknown-linux-gnu/release/deps/librustc_lsan-0814373332ff833c.rmeta --extern rustc_msan=/checkout/obj/build/x86_64-unknown-linux-gnu/stage1-std/x86_64-unknown-linux-gnu/release/deps/librustc_msan-655667bc8a1522d0.rmeta --extern rustc_tsan=/checkout/obj/build/x86_64-unknown-linux-gnu/stage1-std/x86_64-unknown-linux-gnu/release/deps/librustc_tsan-5756fbda854daecf.rmeta --extern unwind=/checkout/obj/build/x86_64-unknown-linux-gnu/stage1-std/x86_64-unknown-linux-gnu/release/deps/libunwind-5e31c590860d0940.rmeta` (exit code: 1)
[00:59:31] 
[00:59:31] 
[00:59:31] command did not execute successfully: "/checkout/obj/build/x86_64-unknown-linux-gnu/stage0/bin/cargo" "rustdoc" "--target" "x86_64-unknown-linux-gnu" "-j" "4" "--release" "--locked" "--color" "always" "--features" "panic-unwind backtrace" "--manifest-path" "/checkout/src/libstd/Cargo.toml" "-Z" "unstable-options" "-p" "std" "--" "--markdown-css" "rust.css" "--markdown-no-toc" "--index-page" "/checkout/src/doc/index.md"
[00:59:31] 
[00:59:31] 
[00:59:31] failed to run: /checkout/obj/build/bootstrap/debug/bootstrap doc
[00:59:31] Build completed unsuccessfully in 0:06:12
[00:59:31] Build completed unsuccessfully in 0:06:12
[00:59:31] Makefile:18: recipe for target 'all' failed
[00:59:31] make: *** [all] Error 1
The command "stamp sh -x -c "$RUN_SCRIPT"" exited with 2.
travis_time:start:0e0d8a6d
$ date && (curl -fs --head https://google.com | grep ^Date: | sed 's/Date: //g' || true)
Fri Feb  8 15:12:07 UTC 2019
---
travis_time:end:0b378bfe:start=1549638728629181498,finish=1549638728633904520,duration=4723022
travis_fold:end:after_failure.3
travis_fold:start:after_failure.4
travis_time:start:0103429f
$ ln -s . checkout && for CORE in obj/cores/core.*; do EXE=$(echo $CORE | sed 's|obj/cores/core\.[0-9]*\.!checkout!\(.*\)|\1|;y|!|/|'); if [ -f "$EXE" ]; then printf travis_fold":start:crashlog\n\033[31;1m%s\033[0m\n" "$CORE"; gdb --batch -q -c "$CORE" "$EXE" -iex 'set auto-load off' -iex 'dir src/' -iex 'set sysroot .' -ex bt -ex q; echo travis_fold":"end:crashlog; fi; done || true
travis_fold:end:after_failure.4
travis_fold:start:after_failure.5
travis_time:start:0a0f6eff
travis_time:start:0a0f6eff
$ cat ./obj/build/x86_64-unknown-linux-gnu/native/asan/build/lib/asan/clang_rt.asan-dynamic-i386.vers || true
cat: ./obj/build/x86_64-unknown-linux-gnu/native/asan/build/lib/asan/clang_rt.asan-dynamic-i386.vers: No such file or directory
travis_fold:end:after_failure.5
travis_fold:start:after_failure.6
travis_time:start:0503f1e2
$ dmesg | grep -i kill

I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact @TimNN. (Feature Requests)

/// With `Infallible` being an enum, this code is valid.
/// However when `Infallible` becomes an alias for the never type,
/// the two `impl`s will start to overlap
/// and therefore will be disallowed by the language’s trait coherence rules.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think we want to add something here that says “so, uh, don’t do this?”. Wording suggestions welcome.

Copy link
Member

Choose a reason for hiding this comment

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

Is using one over the other preferable? Either way, maybe the answer to that should be in the docs.

Copy link
Member

@alexcrichton alexcrichton left a comment

Choose a reason for hiding this comment

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

r=me on the code changes, thanks @SimonSapin!

}
}

use fmt;
Copy link
Member

Choose a reason for hiding this comment

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

Could this be moved up to the top of the file?

}

#[stable(feature = "convert_infallible", since = "1.34.0")]
impl Eq for Infallible {}
Copy link
Member

Choose a reason for hiding this comment

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

With PartialEq and Eq perhaps the Ord-style traits could be included as well?

@Centril Centril added the relnotes Marks issues that should be documented in the release notes of the next release. label Feb 11, 2019
@scottmcm
Copy link
Member

scottmcm commented Feb 11, 2019

🎉 Huzzah! 🎉

Hopefully it sticks this time.

(Aside: #33417 (comment))

@rfcbot rfcbot added final-comment-period In the final comment period and will be merged soon unless new substantive objections are raised. and removed proposed-final-comment-period Proposed to merge/close by relevant subteam, see T-<team> label. Will enter FCP once signed off. labels Feb 12, 2019
@rfcbot
Copy link

rfcbot commented Feb 12, 2019

🔔 This is now entering its final comment period, as per the review above. 🔔

@bors
Copy link
Contributor

bors commented Feb 12, 2019

☔ The latest upstream changes (presumably #58389) made this pull request unmergeable. Please resolve the merge conflicts.

///
/// # Future compatibility
///
/// This enum has the same role as [the `!` “never” type][never],
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe using regular quotes instead? "never"

/// pub type Infallible = !;
/// ```
///
/// … and eventually deprecate `Infallible`.
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe using three plain dots

///
///
/// However there is one case where `!` syntax can be used
/// before `!` is stabilized as a full-fleged type: in the position of a function’s return type.
Copy link
Contributor

Choose a reason for hiding this comment

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

'

/// With `Infallible` being an enum, this code is valid.
/// However when `Infallible` becomes an alias for the never type,
/// the two `impl`s will start to overlap
/// and therefore will be disallowed by the language’s trait coherence rules.
Copy link
Contributor

Choose a reason for hiding this comment

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

'

@SimonSapin
Copy link
Contributor Author

@rust-lang/docs, any opinion on the use of non-ASCII v.s. ASCII punctuation in docs? I use the former out of habit.

@GuillaumeGomez
Copy link
Member

We use it everywhere else in the doc so please use the ascii version of it. :)

@frewsxcv
Copy link
Member

frewsxcv commented Feb 13, 2019

To my knowledge, the docs team has no agreed upon preference for smart quotes or ASCII quotes, so either is fine! I personally use smart quotes.

///
///
/// However there is one case where `!` syntax can be used
/// before `!` is stabilized as a full-fleged type: in the position of a function’s return type.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
/// before `!` is stabilized as a full-fleged type: in the position of a function’s return type.
/// before `!` is stabilized as a full-fledged type: in the position of a function’s return type.

"full-fledged" still looks wrong to me as a NZer, but Merriam-Webster seems to imply that "fully-fledged" is only a British thing.

@jethrogb
Copy link
Contributor

jethrogb commented Feb 13, 2019

It seems to me stabilizing the never type is close (#57012/#58184). Is it really necessary to do this now or can we wait just a little bit longer?

@bors 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 Feb 25, 2019
@Centril
Copy link
Contributor

Centril commented Feb 25, 2019

Since this is a stabilization PR, we want to get it in before master=>beta promotion, so: @bors p=50

@bors
Copy link
Contributor

bors commented Feb 25, 2019

⌛ Testing commit cf26754 with merge 2e616be96c6c14265a4f55ca59f90f143e8b36a1...

@bors
Copy link
Contributor

bors commented Feb 25, 2019

💔 Test failed - checks-travis

@rust-highfive
Copy link
Collaborator

Your PR failed on Travis (raw log). Through arcane magic we have determined that the following fragments from the build log may contain information about the problem.

Click to expand the log.
Attempting to download s3://rust-lang-ci-sccache2/docker/1f08d22f2a5e39c33797b6e95be989c2f1c727b72a5f7eeb8d59fa21b0c4483b32db64e100e5a6edf7e52a2b4841266c0b6e400d243c0f6e6db4570618ce4831
[00:00:14] Attempting with retry: curl -y 30 -Y 10 --connect-timeout 30 -f -L -C - -o /tmp/rustci_docker_cache https://s3-us-west-1.amazonaws.com/rust-lang-ci-sccache2/docker/1f08d22f2a5e39c33797b6e95be989c2f1c727b72a5f7eeb8d59fa21b0c4483b32db64e100e5a6edf7e52a2b4841266c0b6e400d243c0f6e6db4570618ce4831
[00:00:14]   % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
[00:00:14]                                  Dload  Upload   Total   Spent    Left  Speed
No output has been received in the last 30m0s, this potentially indicates a stalled build or something wrong with the build itself.
Check the details on how to adjust your build configuration on: https://docs.travis-ci.com/user/common-build-problems/#Build-times-out-because-no-output-was-received
The build has been terminated

I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact @TimNN. (Feature Requests)

@bors bors added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Feb 25, 2019
@SimonSapin
Copy link
Contributor Author

@bors retry

@bors 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 Feb 25, 2019
@bors
Copy link
Contributor

bors commented Feb 25, 2019

⌛ Testing commit cf26754 with merge 00aae71...

bors added a commit that referenced this pull request Feb 25, 2019
Stabilize TryFrom and TryInto with a convert::Infallible empty enum

This is the plan proposed in #33417 (comment)
@bors
Copy link
Contributor

bors commented Feb 25, 2019

☀️ Test successful - checks-travis, status-appveyor
Approved by: alexcrichton
Pushing 00aae71 to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Feb 25, 2019
@bors
Copy link
Contributor

bors commented Feb 25, 2019

☀️ Test successful - checks-travis, status-appveyor
Approved by: alexcrichton
Pushing 00aae71 to master...

@jethrogb
Copy link
Contributor

We're missing impl From<Infallible> for TryFromSliceError, see #60318

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
disposition-merge This issue / PR is in PFCP or FCP with a disposition to merge it. finished-final-comment-period The final comment period is finished for this PR / Issue. merged-by-bors This PR was explicitly merged by bors. relnotes Marks issues that should be documented in the release notes of the next release. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.