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

Regression in asynchronous-0.4.5, Rust 1.17 #40953

Closed
brson opened this issue Mar 31, 2017 · 20 comments
Closed

Regression in asynchronous-0.4.5, Rust 1.17 #40953

brson opened this issue Mar 31, 2017 · 20 comments
Labels
regression-from-stable-to-beta Performance or correctness regression from stable to beta.

Comments

@brson
Copy link
Contributor

brson commented Mar 31, 2017

https://github.com/bcndanos/asynchronous

101 ec2-user@ip-10-145-56-122:~/triage/asynchronous$ git log -1
commit 5423c7e714e46117a75f8b89086b12f8e31da170
Author: bcndanos <rinnegatobcn@gmail.com>
Date:   Fri May 22 19:38:07 2015 +0200

    Update Readme

ec2-user@ip-10-145-56-122:~/triage/aligner$ rustc +beta -Vv
rustc 1.17.0-beta.2 (b7c276653 2017-03-20)
binary: rustc
commit-hash: b7c27665307704a9b158fe242e88e83914029415
commit-date: 2017-03-20
host: x86_64-unknown-linux-gnu
release: 1.17.0-beta.2
LLVM version: 3.9
        error[E0282]: type annotations needed
 --> <anon>:3:1
  |
3 | asynchronous::Deferred::new(|| {
  | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ cannot infer type for `T`

error: aborting due to previous error(s)

thread 'rustc' panicked at 'Box<Any>', /checkout/src/librustc/session/mod.rs:203

---- src/lib.rs - Deferred<T, E>::then (line 175) stdout ----
        error[E0282]: type annotations needed
 --> <anon>:8:4
  |
8 |    Ok(34)
  |    ^^ cannot infer type for `E`

error: aborting due to previous error(s)

thread 'rustc' panicked at 'Box<Any>', /checkout/src/librustc/session/mod.rs:203

---- src/lib.rs - Promise<T, E>::chain (line 628) stdout ----
        error[E0282]: type annotations needed
  --> <anon>:12:4
   |
12 |    Ok(34)
   |    ^^ cannot infer type for `E`

error: aborting due to previous error(s)

thread 'rustc' panicked at 'Box<Any>', /checkout/src/librustc/session/mod.rs:203

---- src/lib.rs - Promise<T, E>::fail (line 685) stdout ----
        error[E0282]: type annotations needed
 --> <anon>:3:1
  |
3 | asynchronous::Promise::new(|| {
  | ^^^^^^^^^^^^^^^^^^^^^^^^^^ cannot infer type for `T`

error: aborting due to previous error(s)

thread 'rustc' panicked at 'Box<Any>', /checkout/src/librustc/session/mod.rs:203

---- src/lib.rs - Promise<T, E>::then (line 598) stdout ----
        error[E0282]: type annotations needed
 --> <anon>:8:4
  |
8 |    Ok(34)
  |    ^^ cannot infer type for `E`

error: aborting due to previous error(s)

thread 'rustc' panicked at 'Box<Any>', /checkout/src/librustc/session/mod.rs:203


failures:
    src/lib.rs - Deferred<T, E>::chain (line 214)
    src/lib.rs - Deferred<T, E>::fail (line 289)
    src/lib.rs - Deferred<T, E>::then (line 175)
    src/lib.rs - Promise<T, E>::chain (line 628)
    src/lib.rs - Promise<T, E>::fail (line 685)
    src/lib.rs - Promise<T, E>::then (line 598)

test result: FAILED. 22 passed; 6 failed; 0 ignored; 0 measured

error: test failed

Not on nightly.

cc @bcndanos

@brson brson added the regression-from-stable-to-beta Performance or correctness regression from stable to beta. label Mar 31, 2017
@alexcrichton
Copy link
Member

Looks like it also affects dotfiler

cc @matthunz

@brson
Copy link
Contributor Author

brson commented Mar 31, 2017

Also affects error-chain cc @Yamakaky

@alexcrichton
Copy link
Member

Like also affects error_defs

cc @canndrew

@brson
Copy link
Contributor Author

brson commented Mar 31, 2017

Also fiber cc @TileHalo

@brson
Copy link
Contributor Author

brson commented Mar 31, 2017

Also gimli cc @fitzgen

@alexcrichton
Copy link
Member

Also futures 0.1.0

@alexcrichton
Copy link
Member

Also hematite_server

cc @bvssvni

@alexcrichton
Copy link
Member

alexcrichton commented Mar 31, 2017

Also libimagstore

cc @matthiasbeyer

@fitzgen
Copy link
Member

fitzgen commented Mar 31, 2017

@brson:

Also gimli cc @fitzgen

I pinged @aturon about this a few weeks ago. Then I landed a commit to work around the regression within gimli and a new version published to crates.io. Are you saying that the most recent version on crates.io is still affected (or that perhaps this is a different-but-similar regression)? Are the crates listed here the results of a crater (or its new replacement whose name I forgot)?

@brson brson changed the title Regression in asynchronous-0.4.5, Rust 1.16 Regression in asynchronous-0.4.5, Rust 1.17 Mar 31, 2017
@alexcrichton
Copy link
Member

@fitzgen these results are from cargobomb (a crater equivalent-ish) and the full results are posted online with the two regression logs for gimli, downloaded from crates.io.

Do you remember why this fix was needed? It'd be good to track down why inference started failing to see if we want to revert that change.

@Yamakaky
Copy link
Contributor

Yamakaky commented Mar 31, 2017

I modified the broken test from error-chain so that it doesn't fail anymore, BTW. But thanks for the ping.

@alexcrichton
Copy link
Member

@Yamakaky can you point to the fix? It's not clear to me at least currently why this regression happened and affected so many crates.

@nikomatsakis
Copy link
Contributor

This may be fixed by #40636 -- which has yet to be backported to beta.

@fitzgen
Copy link
Member

fitzgen commented Mar 31, 2017

Here is the gimli fix I had made: gimli-rs/gimli@dbec032

@nikomatsakis
Copy link
Contributor

This does work on nightly (I just checked) -- nightly includes my PR that changes how we handle dead-code type-checking (which subsumes and improves upon #40636). I haven't gotten a beta build that includes #40636 yet to test, but I suspect it will be fixed by that.

@Yamakaky
Copy link
Contributor

@alexcrichton
Copy link
Member

Thanks for looking into this Niko! Thanks also for the links to fixes, let's land the backport and go from there

bors added a commit that referenced this issue Apr 1, 2017
…chton

Backport #40636 to beta

I suspect that this will help with #40953 but have not been able to verify for...reasons.

r? @alexcrichton
cc @brson
@nikomatsakis
Copy link
Contributor

OK, so, the backport landed. @alexcrichton, when do you think we'll spin off a new beta so we can most easily test if this is fixed? (That is still a manual process, right?)

@alexcrichton
Copy link
Member

@nikomatsakis we'll try to get a new beta scheduled today

@alexcrichton
Copy link
Member

New beta is here and bug appears fixed! Closing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
regression-from-stable-to-beta Performance or correctness regression from stable to beta.
Projects
None yet
Development

No branches or pull requests

5 participants