Skip to content

Incorrectly typed for expression when into_iter expression diverges. #78240

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

Closed
adlerd opened this issue Oct 22, 2020 · 10 comments
Closed

Incorrectly typed for expression when into_iter expression diverges. #78240

adlerd opened this issue Oct 22, 2020 · 10 comments
Labels
A-type-system Area: Type system C-bug Category: This is a bug.

Comments

@adlerd
Copy link

adlerd commented Oct 22, 2020

I tried this code:

fn test_diverging_into_iter() -> ! {
    for _ in { panic!(); 0.. } {}
}

I expected to see this happen: This should compile successfully. Moreover, the for expression should be assignable to any type and this should work for any function return type. In short, the for expression should be typed at ! not (). As a general rule, for _ in b { c } should be typed like { b; () }, not ().

Instead, this happened:

error[E0308]: mismatched types
 --> src/lib.rs:2:5
  |
1 | fn test_iter_position() -> ! {
  |                            - expected `!` because of return type
2 |     for _ in { panic!(); 0.. } {}
  |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ expected `!`, found `()`
  |
  = note:   expected type `!`
          found unit type `()`

Version: playground, nightly 2020-10-18

See also #77156

@adlerd adlerd added the C-bug Category: This is a bug. label Oct 22, 2020
@adlerd
Copy link
Author

adlerd commented Oct 22, 2020

Also, for the hat trick:

fn test_while() -> ! {
    while { panic!(); false } {}
}

is incorrectly typed. I'll defer an additional bug since it's similar enough to the for issue.

@camelid
Copy link
Member

camelid commented Oct 22, 2020

I don't think this is a bug.

@camelid camelid closed this as completed Oct 22, 2020
@camelid camelid added the A-type-system Area: Type system label Oct 22, 2020
@camelid
Copy link
Member

camelid commented Oct 22, 2020

Here's my reasoning:

This code

for _ in { panic!(); 0.. } {}

is equivalent to

let range: RangeFrom<i32> = { panic!(); 0.. };
for _ in range {}

. The type of range is RangeFrom<i32>; even if it were !, the type of the for expression should not depend on the type of the value it is iterating over.

And the same reasoning for while { panic!(); false } {}.

@varkor
Copy link
Member

varkor commented Oct 22, 2020

I have mixed feelings about this. I think this should work if and only if the following works:

fn foo() -> ! {
    return panic!();
    ()
}

which currently does not work. This is because types must still be valid, even in unreachable code. However, return panic!(); does work, which seems strange, considering statements have type ().

@camelid
Copy link
Member

camelid commented Oct 22, 2020

@varkor Do you want to move this discussion to #t-lang on Zulip or to some kind of T-lang proposal?

@adlerd
Copy link
Author

adlerd commented Oct 22, 2020

It was always my understanding that { a; } is supposed to be the same thing as { a; () } yet now I see a number of simple tests show that when a : !, these are not at all equivalent! Namely, { panic!(); } can be typed ! but { panic!(); () } can't. It's the same situation when you get rid of panic!() altogether and use return 0.

@camelid
Copy link
Member

camelid commented Oct 22, 2020

yet now I see a number of simple tests show that when a : !, these are not at all equivalent! Namely, { panic!(); } can be typed ! but { panic!(); () } can't.

Hmm, that does seem confusing. I still don't think

for _ in { panic!(); 0.. } {}

should be typed as !, but I think the behavior you described should be changed so the types are as follows:

let x: ! = panic!();
let x: ! = { panic!() };
let x: () = { panic!(); };
let x: () = { panic!(); () };

@adlerd
Copy link
Author

adlerd commented Oct 22, 2020

I see where you're coming from, but unless you want to treat function bodies as different than other blocks, you will break every program like this:

fn rand() -> u32 {
    return 4;
}

This is why I propose that all of the expressions you gave should be !. However, I now realize that this is a major change, not merely a small bug with for/while.

@varkor
Copy link
Member

varkor commented Oct 22, 2020

My suspicion is as you say, @adlerd: that people often write return x; and this would break existing code if it was no longer inferred to be !. This must be special-cased somewhere, though I'm not sure where off the top of my head. If there are other inconsistencies, I'd be tempted to change something, but if this is a single special-case, that's probably okay.

@varkor
Copy link
Member

varkor commented Oct 22, 2020

The special case is described here:

// In some cases, blocks have just one exit, but other blocks
// can be targeted by multiple breaks. This can happen both
// with labeled blocks as well as when we desugar
// a `try { ... }` expression.
//
// Example 1:
//
// 'a: { if true { break 'a Err(()); } Ok(()) }
//
// Here we would wind up with two coercions, one from
// `Err(())` and the other from the tail expression
// `Ok(())`. If the tail expression is omitted, that's a
// "forced unit" -- unless the block diverges, in which
// case we can ignore the tail expression (e.g., `'a: {
// break 'a 22; }` would not force the type of the block
// to be `()`).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-type-system Area: Type system C-bug Category: This is a bug.
Projects
None yet
Development

No branches or pull requests

3 participants