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

Summary issue - stabilization of ! #48950

Closed
canndrew opened this issue Mar 12, 2018 · 16 comments
Closed

Summary issue - stabilization of ! #48950

canndrew opened this issue Mar 12, 2018 · 16 comments
Labels
A-inference Area: Type inference A-typesystem Area: The type system C-tracking-issue Category: A tracking issue for an RFC or an unstable feature. T-lang Relevant to the language team, which will review and decide on the PR/issue.

Comments

@canndrew
Copy link
Contributor

What is being stabilized

  • ! is now a full-fledged type and can now be used in any type position (eg. RFC 1216). The ! type can coerce into any other type, see https://github.com/rust-lang/rust/tree/master/src/test/run-fail/adjust_never.rs for an example.

  • Type inference will now default unconstrained type variables to ! instead of (). The resolve_trait_on_defaulted_unit lint has been retired. An example of where this comes up is if you have something like:

    // We didn't specify the type of `x`. Under some circumstances, type inference
    // will pick a type for us rather than erroring
    let x = Deserialize::deserialize(data);

    Under the old rules this would deserialize a (), whereas under the new rules it will deserialize a !.

  • The never_type feature gate is stable, although some of the behaviours it used to gate now live behind the new exhaustive_patterns feature gate (see below).

What is not being stabilized

@nikomatsakis
Copy link
Contributor

@scottmcm
Copy link
Member

scottmcm commented Mar 12, 2018

I had a thought that I think needs to be resolved in the same release as this stabilization: Is there an existing stable uninhabited type in core/std that we want to make an alias for !? (AFAIK we could only do this for one of them, since coherence.)

The one that comes to mind for me is std::string::ParseError...

@nikomatsakis
Copy link
Contributor

@scottmcm I don't think there's a ton of motivation to do that -- I believe that, when all is said and done, empty enums and ! will behave quite similarly in most ways, apart from coercions. Notably in matches.

@nikomatsakis
Copy link
Contributor

Still, interesting thought. Maybe nicer just for simplicity?

cc @rust-lang/libs -- see @scottmcm's suggestion above about making uninhabited types like string::ParseError an alias for !

@sfackler
Copy link
Member

I had always assumed that all of the void errors would turn into type aliases for ! but I hadn't thought of the coherence issue. How many do we actually have? It seems like it may not be worth changing just one of many.

@alexcrichton
Copy link
Member

Yeah I was also personally under the impression that we'd change the types to be aliases and run crater to see if it actually had any fallout (as at the time I don't think we expected much)

@nagisa nagisa added C-tracking-issue Category: A tracking issue for an RFC or an unstable feature. T-lang Relevant to the language team, which will review and decide on the PR/issue. and removed C-tracking-issue Category: A tracking issue for an RFC or an unstable feature. T-lang Relevant to the language team, which will review and decide on the PR/issue. labels Mar 14, 2018
@nikomatsakis
Copy link
Contributor

Well, I just r+'d the ! stabilization PR, so we have until the next release. =)

@scottmcm
Copy link
Member

Well I only found 3 (ParseError, unstable Infallible, and new-for-1.26 ParsePathError) so PR up: #49039

bors added a commit that referenced this issue Mar 15, 2018
Replace uninhabited error enums in std with never

Luckily I only found two, and one of them isn't in beta yet, so can disappear completely 😎

Are there any others I forgot?  (There are lots in things like liblibc and libstd/sys, but AFAIK those don't matter, nor do things like `btree::node::LeafOrInternal` or `str::pattern::RejectAndMatch`.)

The unstable `convert::Infallible` is being handled by #49038

⚠️ This change may be a 1.26-or-never one.

cc #48950 (comment)
r? @alexcrichton
@Centril Centril added A-typesystem Area: The type system T-lang Relevant to the language team, which will review and decide on the PR/issue. A-inference Area: Type inference C-tracking-issue Category: A tracking issue for an RFC or an unstable feature. labels Mar 16, 2018
@mitsuhiko
Copy link
Contributor

mitsuhiko commented Apr 10, 2018

Not sure if this is a regression you care about but the findshlibs crate now fails compiling some tests on beta and nightly as a result of this stabilization: https://travis-ci.org/gimli-rs/findshlibs/jobs/364806360

The code in question:

        match panic::catch_unwind(|| { TargetSharedLibrary::each(|_| panic!("uh oh")); }) {
            Ok(()) => panic!("Expected a panic, but didn't get one"),
            Err(any) => {
                assert!(any.is::<&'static str>(), "panic value should be a &'static str");
                assert_eq!(*any.downcast_ref::<&'static str>().unwrap(), "uh oh");
            }
        }

Error message:

error[E0277]: the trait bound `IterationControl: std::convert::From<!>` is not satisfied
   --> src/lib.rs:326:40
    |
326 |         match panic::catch_unwind(|| { TargetSharedLibrary::each(|_| panic!("uh oh")); }) {
    |                                        ^^^^^^^^^^^^^^^^^^^^^^^^^ the trait `std::convert::From<!>` is not implemented for `IterationControl`
    |
    = help: the following implementations were found:
              <IterationControl as std::convert::From<()>>
    = note: required because of the requirements on the impl of `std::convert::Into<IterationControl>` for `!`
note: required by `SharedLibrary::each`
   --> src/lib.rs:296:5
    |
296 | /     fn each<F, C>(f: F)
297 | |     where
298 | |         F: FnMut(&Self) -> C,
299 | |         C: Into<IterationControl>;
    | |__________________________________^
error: aborting due to previous error

@nikomatsakis
Copy link
Contributor

@canndrew looking at @mitsuhiko's issue, and just thinking generally -- I was wondering -- what is the classic example of why fallback to () would "have" to be changed? That is, if we continued to fallback to (), but still added !, it would certainly avoid that sort of problem -- and falling back to ! is not optimal, given that there are cases where the fallback occurs to a type variable that winds up influencing "live" code (the intention of course is otherwise, but it's hard to prevent leaks, as we've found).

@nikomatsakis
Copy link
Contributor

Nominating for brief lang-team discussion around this final point

@SimonSapin
Copy link
Contributor

For what it’s worth the fallback change caused some previously-valid unsafe code to go Very Wrong: rust-windowing/winit#428, servo/servo#20474 (comment).

In short: generic FFI bindings in the objc crate end up returning Ok(x: R) where R is an unconstrained type parameter whose inference changed from () to !. So now (to simplify) the Result<!, String>::Ok value is incorrectly assumed to be Err, and on drop we deallocate a String whose pointer is made of padding bytes. jemalloc is not amused. The fix is changing statements like send!(…); to force the return type: let () = send!(…);

@nikomatsakis
Copy link
Contributor

@SimonSapin ah yes thanks for raising that

@nikomatsakis
Copy link
Contributor

Cross-posted my question to the main tracking issue, and added a few details. That seems like a better place to have this conversation.

joliss added a commit to sinopiaolive/package-manager that referenced this issue Jul 3, 2018
warning: lint resolve_trait_on_defaulted_unit has been removed: converted into hard error, see rust-lang/rust#48950
warning: conservative_impl_trait has been stable since 1.26.0. Attribute no longer needed
@SimonSapin
Copy link
Contributor

Should this be closed? The last comment pre-dates #50121 and now #57012.

@pnkfelix
Copy link
Member

closing since we have started a new round of stabilization in #57012 and so we should try to capture all concerns and other threads of conversation there.

SimonSapin added a commit to SimonSapin/rust that referenced this issue Feb 13, 2019
The reverse conversion unfortunately causes unexpected errors like:

```
error[E0277]: the trait bound `!: std::convert::From<()>` is not satisfied
   --> src/librustc_metadata/encoder.rs:105:9
    |
105 |         self.emit_usize(seq.len)?;
    |         ^^^^^^^^^^^^^^^^^^^^^^^^^ the trait `std::convert::From<()>` is not implemented for `!`
    |
    = help: the following implementations were found:
              <! as std::convert::From<std::convert::Infallible>>
    = note: the trait is implemented for `()`. Possibly this error has been caused by changes to Rust's type-inference algorithm (see: rust-lang#48950 for more info). Consider whether you meant to use the type `()` here instead.
    = note: required by `std::convert::From::from`
```

I don’t understand why this error happens.
If I’m reading the code correctly the return types of `emit_usize`
and of the method that contains line 105 are both `Result<(), !>`,
so the expansion of the `?` operator should involve `!: From<!>`,
not `From<()>`.

Is this a type inference bug?
yvt added a commit to yvt/ngspades that referenced this issue Apr 25, 2020
`msg_send!` with an unconstrained return type used to be deduced to have
the return type `()`. This is no longer the case after the stabilization
of the `!` (never) type (rust-lang/rust#48950),
and it'll be deduced to be `!`.

This commit adds explicit return types to preserve the old behavior.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-inference Area: Type inference A-typesystem Area: The type system C-tracking-issue Category: A tracking issue for an RFC or an unstable feature. T-lang Relevant to the language team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

10 participants