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

Rust 1.19 regression, minions 0.2.8, type mismatch #42618

Closed
brson opened this Issue Jun 12, 2017 · 7 comments

Comments

Projects
None yet
4 participants
@brson
Copy link
Contributor

brson commented Jun 12, 2017

https://github.com/dbeck/acto-rs

commit 5311c872dd88d3bf034e023f501971c711dee41c
Author: David Beck <david.beck.priv@gmail.com>
Date:   Mon Aug 29 23:04:00 2016 +0100

    sanitize Observer interface
rustc 1.19.0-beta.1 (a87984118 2017-06-06)
binary: rustc
commit-hash: a879841184d6fe89b819425fde1568b50a54543f
commit-date: 2017-06-06
host: x86_64-unknown-linux-gnu
release: 1.19.0-beta.1
LLVM version: 4.0

brian@ip-10-145-43-250:~/dev/acto-rs⟫ cargo +beta test
   Compiling minions v0.2.7 (file:///mnt2/dev/acto-rs)
error[E0308]: mismatched types
   --> src/main.rs:147:14
    |
147 |       sum += i;
    |              ^ expected i32, found ()
    |
    = note: expected type `i32`
               found type `()`

error: aborting due to previous error(s)

error: Could not compile `minions`.

To learn more, run the command again with --verbose.

cc @dbeck

@Mark-Simulacrum

This comment has been minimized.

Copy link
Member

Mark-Simulacrum commented Jun 13, 2017

The relevant code (which is in a test case, I believe) has been deleted on master of the relevant repo, but bisection shows failure was introduced in #42265 -- cc @eddyb @Zoxc, this is actually somewhat concerning. It's possible that this hints at some more wide-spread problem that patch introduced that just wasn't noticed yet, but I could be wrong.

Here is the code https://github.com/dbeck/acto-rs/blob/5311c872dd88d3bf034e023f501971c711dee41c/src/main.rs#L147.

@eddyb

This comment has been minimized.

Copy link
Member

eddyb commented Jun 13, 2017

I really don't see how that could be happening - does it reproduce on nightly?

@Mark-Simulacrum

This comment has been minimized.

Copy link
Member

Mark-Simulacrum commented Jun 13, 2017

Reproduces on nightly and 5aa3403.

@eddyb

This comment has been minimized.

Copy link
Member

eddyb commented Jun 13, 2017

Reproduced with:

fn main() {
    let mut sum = 0;
    for i in Vec::new() {
        sum += i;
    }
}

@brson brson added the A-typesystem label Jun 15, 2017

@brson

This comment has been minimized.

Copy link
Contributor Author

brson commented Jun 15, 2017

@brson brson added the P-high label Jun 15, 2017

bors added a commit that referenced this issue Jun 16, 2017

Auto merge of #42634 - Zoxc:for-desugar2, r=nikomatsakis
Change the for-loop desugar so the `break` does not affect type inference. Fixes #42618

Rewrite the `for` loop desugaring to avoid contaminating the inference results. Under the older desugaring, `for x in vec![] { .. }` would erroneously type-check, even though the type of `vec![]` is unconstrained. (written by @nikomatsakis)

bors added a commit that referenced this issue Jun 22, 2017

Auto merge of #42634 - Zoxc:for-desugar2, r=nikomatsakis
Change the for-loop desugar so the `break` does not affect type inference. Fixes #42618

Rewrite the `for` loop desugaring to avoid contaminating the inference results. Under the older desugaring, `for x in vec![] { .. }` would erroneously type-check, even though the type of `vec![]` is unconstrained. (written by @nikomatsakis)

@bors bors closed this in dbb655a Jun 22, 2017

@Mark-Simulacrum

This comment has been minimized.

Copy link
Member

Mark-Simulacrum commented Jun 22, 2017

Reopening to track the beta backport of #42634. I don't know if this is the proper procedure but... seems good and not harmful.

@nikomatsakis

This comment has been minimized.

Copy link
Contributor

nikomatsakis commented Jun 29, 2017

@Mark-Simulacrum looks like beta backport landed in #42927, no? Going to close for now, assuming I'm right.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.