rust-lang / rust

Empowering everyone to build reliable and efficient software.
https://www.rust-lang.org
Other
97.78k stars 12.65k forks source link

Defaulted unit types no longer error out (regression?) #51125

Open Manishearth opened 6 years ago

Manishearth commented 6 years ago

This currently compiles (on stable and nightly). Till 1.25, it would trigger a lint because it has inference default to () instead of throwing an error.

fn main() {}

struct Err;
fn load<T: Default>() -> Result<T, Err> {
    Ok(T::default())
}
fn foo() -> Result<(), Err> {
    let val = load()?; // defaults to ()
    Ok(())
}

(playpen)

That lint indicates that it would become a hard error in the future, but it's not erroring. It seems like a bunch of this was changed when we stabilized !.

That issue says

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:

Though this doesn't really make sense, this looks like a safe way to produce nevers, which should, in short, never happen. It seems like this is related to https://github.com/rust-lang/rust/issues/40801 -- but that was closed as it seems to be a more drastic change.

Also, if you print val, it's clear that the compiler thought it was a unit type. This seems like one of those cases where attempting to observe the situation changes it.

We should have a hard error here, this looks like a footgun, and as @SimonSapin mentioned has broken some unsafe code already. We had an upgrade lint for the hard error and we'll need to reintroduce it for a cycle or two since we've had a release without it. AFAICT this is a less drastic change than #40801, and we seem to have intended for this to be a hard error before so it probably is minor enough that it's fine.

cc @nikomatsakis @eddyb

h/t @spacekookie for finding this

Enselic commented 11 months ago

Triage: Even though the issue is old, it seems serious enough to be formally marked as a regression. So I added a regression label.

apiraino commented 11 months ago

Well, yes this is technically a regression (albeit very old) but it is tracked in https://github.com/rust-lang/rust/issues/39216 and recently added to a 2024 edition milestone. So if I understand the context this issue is not "forgotten" but doesn't require immediate attention.

@rustbot label -I-prioritize

WaffleLapkin commented 7 months ago

So the issue is how desugating of ? interacts with ! (ahaha).

The code in the issue description gets desugared to something like this (heavily simplified) (I've renamed types/variables for clarity):

struct E;

fn load<T: Default>() -> Result<T, E> { ... }

fn foo() -> Result<(), E> {
    let v = match load() {
        Ok(val) => val,
        Err(err) => return Err(err),
    };
    Ok(())
}

In this desugared form it's clear that val's type if affected by ?/return. The match requires ?M <: ?V, ?M <: ?D where ?M type of the match/v, ?V is the type of val/load's generic, ?D is a diverging inference variable that comes from never-to-any coercion. Then we fill ?M = ?V = ?D = () because of the spontaneous never decay / diverging fallback.

? should not do that, this code should cause an error like "can't inference blah blah blah". I'll try to change the desugaring to something like this:

fn foo() -> Result<(), E> {
    let v = {
        let ret;
        match load() {
            Ok(val) => ret = val,
            Err(err) => return Err(err),
        };
        ret
    };
    Ok(())
}

To eliminate the return influnce on val's type.

Upd: this desugaring does not work, because it changes the way ? interacts with temporaries :( Upd: I found a couple desugarings that actually work and opened #122412 with the least intrusive one