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

rustc: Disallow autoderef for bottom type ! #14574

Closed
wants to merge 1 commit into from

Conversation

mdup
Copy link

@mdup mdup commented Jun 1, 2014

Fix issue #13847.

@alexcrichton
Copy link
Member

Thanks for looking into this! While this does indeed fix the ICE from appearing, it may not be the desired behavior that we want. In most other use cases the bottom type will unify and work with all other types. For example, if one branch of an if expression has the bottom type, the other branch can have whatever type it wants.

The desired semantics here may be to "just work" and continue to propagate the bottom type outwards, but this is definitely another alternative! It may not matter too much in the long run because this seems pretty rare.

@lilyball
Copy link
Contributor

lilyball commented Jun 1, 2014

I agree, propagating bottom seems like the more correct approach.

@nikomatsakis
Copy link
Contributor

cc me

@nikomatsakis
Copy link
Contributor

I am not sure if propagating bottom is good in this case. It makes sense in (some) other cases, because the semantics of what code would execute are not dependent on what type the ! value winds up being. This is not the case for method calls or (similarly) generic functions.

@nikomatsakis
Copy link
Contributor

To be honest, I actually wonder if we should include bottom in the type system at all. it is not particularly like other types. I think I would be happier overall if we just assigned the "type" ! to a fresh inference variable (but still used the information about what must fail when doing dataflow). Hmm.

@mdup
Copy link
Author

mdup commented Jun 2, 2014

On a short term basis, my patch fixes the ICE, so that might be desirable. On the other hand, it doesn't explain why the assumptions in middle/typeck/check/method.rs:confirm_candidate() are not satisfied (hence the bug() call). It may be a smell of some other failure not discovered yet, so I guess it needs to be addressed for the future.

@lilyball
Copy link
Contributor

lilyball commented Jun 2, 2014

@nikomatsakis By "a fresh inference variable" do you mean that an expression typed as ! would just become an unconstrained variable that can end up unifying with another type (e.g. so ! could end up being inferred as int)?

@nikomatsakis
Copy link
Contributor

On Mon, Jun 02, 2014 at 01:05:44PM -0700, Kevin Ballard wrote:

@nikomatsakis By "a fresh inference variable" do you mean that an expression typed as ! would just become an unconstrained variable that can end up unifying with another type (e.g. so ! could end up being inferred as int)?

Yes. This is what ML compilers do, afaik.

@alexcrichton
Copy link
Member

Closing due to inactivity, but feel free to reopen with a rebase!

bors added a commit to rust-lang-ci/rust that referenced this pull request Jun 5, 2023
internal: Make block_def_map infallible
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants