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

Stable regression in nightly #26952

Closed
frankmcsherry opened this Issue Jul 10, 2015 · 14 comments

Comments

Projects
None yet
8 participants
@frankmcsherry
Copy link
Contributor

frankmcsherry commented Jul 10, 2015

Type inference broke, or something. Hard to see why, as the file (broadcast.rs) is probably one of the simpler cases.

Easiest repro to is clone the repo (it is also up on crates, though as an earlier version)

% git clone https://github.com/frankmcsherry/timely-dataflow
Cloning into 'timely-dataflow'...
remote: Counting objects: 1959, done.
remote: Compressing objects: 100% (70/70), done.
remote: Total 1959 (delta 32), reused 0 (delta 0), pack-reused 1889
Receiving objects: 100% (1959/1959), 596.67 KiB | 741.00 KiB/s, done.
Resolving deltas: 100% (1393/1393), done.
Checking connectivity... done.
% cd timely-dataflow 
% multirust override stable
multirust: using existing install for 'stable'
multirust: override toolchain for '/Users/mcsherry/timely-dataflow' set to 'stable'
% cargo build
    Updating git repository `https://github.com/frankmcsherry/abomonation`
    Updating registry `https://github.com/rust-lang/crates.io-index`
   Compiling byteorder v0.3.11
   Compiling libc v0.1.8
   Compiling abomonation v0.1.0 (https://github.com/frankmcsherry/abomonation#6bdf392c)
   Compiling log v0.3.1
   Compiling getopts v0.2.11
   Compiling timely v0.0.7 (file:///Users/mcsherry/timely-dataflow)
% multirust override nightly
multirust: using existing install for 'nightly'
multirust: override toolchain for '/Users/mcsherry/timely-dataflow' set to 'nightly'
% cargo build               
   Compiling abomonation v0.1.0 (https://github.com/frankmcsherry/abomonation#6bdf392c)
   Compiling libc v0.1.8
   Compiling byteorder v0.3.11
   Compiling log v0.3.1
   Compiling getopts v0.2.11
   Compiling timely v0.0.7 (file:///Users/mcsherry/timely-dataflow)
src/progress/broadcast.rs:31:51: 31:70 error: the type of this value must be known in this context
src/progress/broadcast.rs:31                 while let Some((update, delta)) = recv_messages.pop() {
                                                                               ^~~~~~~~~~~~~~~~~~~
note: in expansion of while let expansion
src/progress/broadcast.rs:31:17: 33:55 note: expansion site
note: in expansion of while let expansion
src/progress/broadcast.rs:30:13: 37:14 note: expansion site
src/progress/broadcast.rs:34:51: 34:70 error: the type of this value must be known in this context
src/progress/broadcast.rs:34                 while let Some((update, delta)) = recv_internal.pop() {
                                                                               ^~~~~~~~~~~~~~~~~~~
note: in expansion of while let expansion
src/progress/broadcast.rs:34:17: 36:55 note: expansion site
note: in expansion of while let expansion
src/progress/broadcast.rs:30:13: 37:14 note: expansion site
error: aborting due to 2 previous errors
Could not compile `timely`.

To learn more, run the command again with --verbose.
% cargo --version
cargo 0.4.0-nightly (15b497b 2015-07-08) (built 2015-07-08)
% rustc --version
rustc 1.3.0-nightly (16f64c388 2015-07-09)
@alexcrichton

This comment has been minimized.

Copy link
Member

alexcrichton commented Jul 11, 2015

This is likely due to the new implementation of Extend on collections for &T. Do you have some source code that we could poke around with? This may just entail an annotation on a call to collect.

@frankmcsherry

This comment has been minimized.

Copy link
Contributor Author

frankmcsherry commented Jul 11, 2015

Well, you can poke around with a copy of the repo? The file that is causing the issues is https://github.com/frankmcsherry/timely-dataflow/blob/master/src/progress/broadcast.rs, and though it doesn't seem to use Extend or extend directly, it used to (so, the issue might be similar).

If I misunderstand what you mean by "code to poke around with", let me know. I don't have a smaller reproduction, in that keeping things building on stable means not breaking them by ripping out lines, but I'm happy to try and help.

@bluss

This comment has been minimized.

Copy link
Contributor

bluss commented Jul 11, 2015

I didn't understand the reason it didn't infer, looks like it should be quite simple. Smells a bit like a regression in the language, not caused by libstd changes(?) So are you sure the exact same code compiles on stable, but not on nightly?

@alexcrichton

This comment has been minimized.

Copy link
Member

alexcrichton commented Jul 11, 2015

Oh oops sorry I missed the git link in the initial report! Nominating as this does look like a regression

@alexcrichton

This comment has been minimized.

Copy link
Member

alexcrichton commented Jul 11, 2015

cc @rust-lang/lang and @rust-lang/comipler

@alexcrichton

This comment has been minimized.

Copy link
Member

alexcrichton commented Jul 11, 2015

@bkoropoff

This comment has been minimized.

Copy link
Contributor

bkoropoff commented Jul 12, 2015

Note that changes to type inference that require the addition of annotations to existing code are considered "fair game". I'd still like to understand what changed, though.

@jroesch

This comment has been minimized.

Copy link
Member

jroesch commented Jul 12, 2015

I don't think any intentional inference changes have landed, but I agree @bkoropoff we should at least figure out what changed and advise people accordingly.

@frankmcsherry

This comment has been minimized.

Copy link
Contributor Author

frankmcsherry commented Jul 14, 2015

More data on when this broke:

A coworker has a stale nightly, rustc 1.3.0-nightly (faa04a8 2015-06-30), which builds the git pull just fine. I'll see about getting a build against the crates.io version (his project isn't compatible with 0.0.6, but a new project that just loads the timely crate explodes).

My version, 1.3.0-nightly (16f64c3 2015-07-09) explodes on the git pull and crates.io's timely 0.0.6 (which is older than the git pull).

Crater with 1.3.0-nightly (2015-07-10) explodes on crates.io's timely 0.0.6. I suspect it would break on the git drop too.

This may help narrow down when the regression dropped in. I think multirust can pull down specific nightlies, so I may just do a few more of those.

@frankmcsherry

This comment has been minimized.

Copy link
Contributor Author

frankmcsherry commented Jul 14, 2015

courtesy multirust, against crates.io timely 0.0.6, using nightlies from http://static.rust-lang.org/dist/

nightly-2015-06-30: pass
nightly-2015-07-01: pass
nightly-2015-07-07: fail
nightly-2015-07-08: fail
nightly-2015-07-09: fail

I don't know why there is a week gap, but it seems to be somewhere in there.

@nikomatsakis

This comment has been minimized.

Copy link
Contributor

nikomatsakis commented Jul 23, 2015

triage: P-high -- it's a regression

@rust-highfive rust-highfive added P-high and removed I-nominated labels Jul 23, 2015

@nikomatsakis

This comment has been minimized.

Copy link
Contributor

nikomatsakis commented Jul 24, 2015

No one has successfully reduced this, right?

@frankmcsherry

This comment has been minimized.

Copy link
Contributor Author

frankmcsherry commented Jul 24, 2015

I haven't, sorry. The code is currently in a bit of a state, though if I get it all sorted out I should be able to start cutting things out. The file depends on just a few other traits / structs transitively, so I should be able to chop a bit and make some weird non-functional library that may be easier to poke through.

But, currently tied up with accomplishing work.

@nikomatsakis

This comment has been minimized.

Copy link
Contributor

nikomatsakis commented Jul 24, 2015

I just tested against my branch. It is able to build timely-dataflow

@nikomatsakis nikomatsakis self-assigned this Jul 24, 2015

nikomatsakis added a commit to nikomatsakis/rust that referenced this issue Jul 24, 2015

Correct regression in type-inference caused by failing to reconfirm that
the object trait matches the required trait during trait selection.  The
existing code was checking that the object trait WOULD match (in a
probe), but never executing the match outside of a probe.

This corrects various regressions observed in the wild, including
issue rust-lang#26952. Fixes rust-lang#26952.

nikomatsakis added a commit to nikomatsakis/rust that referenced this issue Jul 24, 2015

Correct regression in type-inference caused by failing to reconfirm that
the object trait matches the required trait during trait selection.  The
existing code was checking that the object trait WOULD match (in a
probe), but never executing the match outside of a probe.

This corrects various regressions observed in the wild, including
issue rust-lang#26952. Fixes rust-lang#26952.

Manishearth added a commit to Manishearth/rust that referenced this issue Jul 24, 2015

Rollup merge of rust-lang#27258 - nikomatsakis:issue-26952, r=eddyb
Correct regression in type-inference caused by failing to reconfirm that
the object trait matches the required trait during trait selection.  The
existing code was checking that the object trait WOULD match (in a
probe), but never executing the match outside of a probe.

This corrects various regressions observed in the wild, including
issue rust-lang#26952. Fixes rust-lang#26952.

r? @eddyb 
cc @frankmcsherry

bors added a commit that referenced this issue Jul 25, 2015

Auto merge of #27258 - nikomatsakis:issue-26952, r=eddyb
Correct regression in type-inference caused by failing to reconfirm that
the object trait matches the required trait during trait selection.  The
existing code was checking that the object trait WOULD match (in a
probe), but never executing the match outside of a probe.

This corrects various regressions observed in the wild, including
issue #26952. Fixes #26952.

r? @eddyb 
cc @frankmcsherry

@bors bors closed this in #27258 Jul 25, 2015

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.