Skip to content

Correct regression in type-inference #27258

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

Merged
merged 1 commit into from
Jul 25, 2015
Merged

Conversation

nikomatsakis
Copy link
Contributor

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

let upcast_trait_refs: Vec<_> = util::supertraits(self.tcx(), poly_trait_ref).collect();

// Find the first one that matches; this is what we are upcasting to.
let index =
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: I think this iterator salad would actually be clearer as a for-loop

@arielb1
Copy link
Contributor

arielb1 commented Jul 24, 2015

r+ modulo nit

let mut upcast_trait_ref = None;
let mut vtable_base = 0;
// Collect the full set of supertraitrefs.
let upcast_trait_refs: Vec<_> = util::supertraits(self.tcx(), poly_trait_ref).collect();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Couldn't this be done without collecting?

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
Copy link
Contributor Author

@eddyb updated

@@ -1898,7 +1898,7 @@ impl<'tcx> PolyTraitRef<'tcx> {
/// erase, or otherwise "discharge" these bound regions, we change the
/// type from `Binder<T>` to just `T` (see
/// e.g. `liberate_late_bound_regions`).
#[derive(Clone, PartialEq, Eq, Hash, Debug)]
#[derive(Copy, Clone, PartialEq, Eq, Hash, Debug)]
pub struct Binder<T>(pub T);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Time to remove a thousand clone calls :P.
@huonw @Manishearth How about a lint against calling .clone() on a Copy type?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds good to me.

@eddyb
Copy link
Member

eddyb commented Jul 24, 2015

@bors r+

@bors
Copy link
Collaborator

bors commented Jul 24, 2015

📌 Commit 4726bb4 has been approved by eddyb

Manishearth added a commit to Manishearth/rust that referenced this pull request 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.

r? @eddyb 
cc @frankmcsherry
@bors
Copy link
Collaborator

bors commented Jul 25, 2015

⌛ Testing commit 4726bb4 with merge 7276d8b...

bors added a commit that referenced this pull request Jul 25, 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 #26952. Fixes #26952.

r? @eddyb 
cc @frankmcsherry
@bors bors merged commit 4726bb4 into rust-lang:master Jul 25, 2015
@nikomatsakis nikomatsakis deleted the issue-26952 branch March 30, 2016 16:14
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.

5 participants