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

Change which type variables fall back to `!` #40801

Closed
nikomatsakis opened this Issue Mar 24, 2017 · 9 comments

Comments

Projects
None yet
6 participants
@nikomatsakis
Contributor

nikomatsakis commented Mar 24, 2017

Right now, type variables fallback to ! if they are the result of a return etc. The idea was that this represents a "dead control flow path", but it is not clear that this is the right rule, because those type variables often get 'caught up' in other paths.

In #40224, I outlined another possibility in a comment:

In talking with @pnkfelix, we had an alternative idea that yields a similar result, but does so in a more tailored way. Specifically, we could say that when you instantiate one enum variant (say, Err), any type parameters which do not appear in the variant in question get a diverging fallback. So the type of Err(x) is Result<?T, X>, where ?T falls back to !.

This would allow Err(x)? to work as expected, while making Deserialize::deserialize()? fail as requiring a type annotation.

...

This result is not perfect. One can still write things like:

let mut x = Err(E);
x = Ok(Deserialize::deserialize()?);

in particular, this would mean that Deserialize::deserialize()?; would error out, whereas today it defaults to deserializing the ! type (and hence ... probably ... panics?). This feels like a case where I would expect an "unconstrained type variable" error -- but we don't get one today because one of the arms is dead, and hence generates a diverging fallback when coerced.

There is an obvious backwards compatibility concern here. It's not clear how much we can change these paths. But I suspect we have some leeway, if what we do is tailored enough.

@nikomatsakis

This comment has been minimized.

Contributor

nikomatsakis commented Mar 24, 2017

@arielb1

This comment has been minimized.

Contributor

arielb1 commented Mar 24, 2017

@nikomatsakis

Your change is far more easily visible - it allows you to do mem::size_of_val(&None).

@nikomatsakis

This comment has been minimized.

Contributor

nikomatsakis commented Mar 24, 2017

@arielb1 True, though I would argue that they are both easily visible -- using the ? operator, after all, triggers the current approach.

@dhardy

This comment has been minimized.

Contributor

dhardy commented May 5, 2017

Apparently Option<!> is already a legal type, with size 1: assert_eq!(mem::size_of::<Option<!>>(), 1), but having None default to Option<!> when the type is otherwise unconstrained is a bit weird.

Perhaps here the problem isn't that None is considered Option<!> when its value is not used, but that metadata dependent on that value (i.e. the size of the type) can be tested even though the value type is unconstrained. I'm not convinced trying to prevent checking metadata (mem::size_of_val(&None)) actually makes any sense however, so maybe it's a weirdness we should just live with.

@nikomatsakis

This comment has been minimized.

Contributor

nikomatsakis commented May 5, 2017

@dhardy

Perhaps here the problem isn't that None is considered Option<!> when its value is not used, but that metadata dependent on that value (i.e. the size of the type) can be tested even though the value type is unconstrained.

I see. So, yeah ,basically, the reason that Option<!> feels natural is because, in a way, you can't observe it -- but you're pointing out that, indeed, you can observe it, through things like sizeof. There are other ways too, through traits, though you have to work harder at it. I also feel like ultimately we have to accept that fallback to ! will leak out in some places -- I was unable to find a workable scheme that could avoid that entirely.

@shepmaster shepmaster changed the title from change which type variables fallback to `!` to Change which type variables fall back to `!` Aug 3, 2017

@nikomatsakis

This comment has been minimized.

Contributor

nikomatsakis commented Oct 25, 2017

So @arielb1 and I were talking about this. We decided to close this issue on the following grounds:

  • It would be a backwards incompatible change to alter the fallback rules.
  • The proposed rule is not better enough than the existing rule (it still has surprising cases).

Just for future reference, the relevant examples are:

try!(Err(E)) -- this works under both today's rule and the proposed rule. It produces a type ?T with fallback, though it does so differently. Today, this happens because we desugar to a:

match Err(E) { 
    Ok(v) => v, // yields an unconstrainted type variable `?T`
    Err(e) => return Err(e) // yields an unconstrained type variable `?U` with fallback
}

As a result, ?T and ?U are unified and hence the resulting type has fallback (from ?U). Under the rule proposed here, the fallback arises from the Err(E) expression, which results in a type Result<?U, E> where ?U has fallback.

However, today's rules do something undesirable in a case like this (which occurred in serde)

try!(Deserialize::deserialize(...))

what happens here is that this "desugars" to

try!(<?T as Deserialize>::deserialize(...))

where ?T winds up (by the same mechanism) inheriting a fallback. If this deserialize() invocation results in an error, that's fine, because that type ?T is irrelevant. But otherwise, it is odd. Today, this means ?T ends up as (), but with ! it will end up as ! -- ! is perhaps better since Deserialize may not be implemented for !, or can produce a nicer error if the Ok path winds up executing.

The proposed rule here would result in an "unresolved type variable error" -- which is better, but backwards incompatible.

However, the proposed rule will default in an example like this:

let x = match v {
  Case1 => <?T as Deserialize>::deserialize(),
  Case2 => Err::<?U, E>(e) // `?U` gets fallback here
};

The end-result is that the call to deserialize is <! as Deserialize>::deserialize.

@canndrew

This comment has been minimized.

Contributor

canndrew commented Oct 26, 2017

Sounds fine to me. By the way ! does (or at least should) implement Deserialize, it's just that it will always error at runtime (since it's never possible to produce a ! from the serialized data). So I agree deserializing to ! makes more sense than ().

@cramertj

This comment has been minimized.

Member

cramertj commented Oct 29, 2017

I'm not totally clear on the resolution here-- does this mean that None will fall back to Option<!>? If so, adding a impl<T> PartialEq<T> for ! would allow us to finally resolve rust-lang/rfcs#1239, which would be amazing.

@arielb1

This comment has been minimized.

Contributor

arielb1 commented Oct 31, 2017

@cramertj

No - the final decision is to create ! defaults when you have a return, not an enum variant.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment