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

Handle divergence in type inference for blocks #1945

Merged
merged 1 commit into from
Oct 2, 2019

Conversation

lnicola
Copy link
Member

@lnicola lnicola commented Oct 2, 2019

Fixes #1944.

The infer_basics test is failing, not sure what to do about it.

@flodiebold
Copy link
Member

I think we also need to consider cases like some_function(return) or if some_condition { return; } else { return; }, and check what rustc does with them -- I think it considers them diverging as well, though I don't know whether it gives them type !. (These may seem useless, but some_function(unimplemented!()) or if some_condition { panic!() } else { return; } aren't...)

@flodiebold
Copy link
Member

flodiebold commented Oct 2, 2019

The infer_basics change is probably fine, though as mentioned I'd still want to check what rustc does for this.

@lnicola
Copy link
Member Author

lnicola commented Oct 2, 2019

I'm not sure how to check. rustc says that the function returns (), but that's the declared type, while our test shows the type of the block that makes up the function's body.

@lnicola
Copy link
Member Author

lnicola commented Oct 2, 2019

@lnicola lnicola changed the title [WIP] Handle divergence in type inference for blocks Handle divergence in type inference for blocks Oct 2, 2019
@flodiebold
Copy link
Member

Yeah, but in a more roundabout way: If the block diverges, rustc does not treat the missing tail expression as (). And if there are no returns/breaks in the block and no provided type, that means it gets typed as !.

In general, I think we do need to track divergence separately from the type of the expression, similar to rustc, because e.g. for let x = SomeStruct { field: unimplemented!() } we wouldn't want to say x: !. None the less, I think this change goes in the right direction for that, so let's merge it 🙂

bors r+

bors bot added a commit that referenced this pull request Oct 2, 2019
1945: Handle divergence in type inference for blocks r=flodiebold a=lnicola

Fixes #1944.

The `infer_basics` test is failing, not sure what to do about it.

Co-authored-by: Laurențiu Nicola <lnicola@dend.ro>
@bors
Copy link
Contributor

bors bot commented Oct 2, 2019

Build succeeded

@bors bors bot merged commit f491567 into rust-lang:master Oct 2, 2019
@lnicola
Copy link
Member Author

lnicola commented Oct 2, 2019

e.g. for let x = SomeStruct { field: unimplemented!() } we wouldn't want to say x: !

That actually works because the code is only checking for ! on statements, not bindings.

@flodiebold
Copy link
Member

Well yeah, it still works, but we should actually be checking for divergence inside expressions. For example this type checks:

fn test() -> i128 {
    foo(unimplemented!());
}

fn bar() -> i128 {
    Struct { field: unimplemented!() };
}

So we do need to recognize that Struct { field: unimplemented!() } diverges, but we don't want to give it type !. So we need to track divergence separately from the type.

@lnicola lnicola deleted the block-divergence branch April 5, 2020 15:43
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.

Wrong type inference in if and match with return statement
2 participants