Skip to content

Inference regression in 1.26 #49944

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
pietroalbini opened this issue Apr 13, 2018 · 8 comments
Closed

Inference regression in 1.26 #49944

pietroalbini opened this issue Apr 13, 2018 · 8 comments
Labels
A-inference Area: Type inference C-bug Category: This is a bug. P-high High priority regression-from-stable-to-beta Performance or correctness regression from stable to beta. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Milestone

Comments

@pietroalbini
Copy link
Member

There is an inference regression for a rust project.

@pietroalbini pietroalbini added I-nominated T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. regression-from-stable-to-beta Performance or correctness regression from stable to beta. A-inference Area: Type inference C-bug Category: This is a bug. labels Apr 13, 2018
@pietroalbini pietroalbini added this to the 1.26 milestone Apr 13, 2018
@Mark-Simulacrum
Copy link
Member

cc @nikomatsakis -- I think you wanted to at least be aware of these regressions. We'll probably be closing this though as it looks minimally impactful.

@nikomatsakis
Copy link
Contributor

Hmm. Thanks. I'm not sure what would cause that off top of my head.

@pnkfelix
Copy link
Member

visiting for triage.

@pietroalbini are you in a position to provide a standalone, minimized test case to speed up our ability to jump on this?

@pnkfelix
Copy link
Member

triage: P-high

(for now, since its a stable-to-beta regression, though @Mark-Simulacrum commented above that the impact may be "minimal" so who knows, it might get downgraded real soon...)

@pnkfelix pnkfelix added the P-high High priority label Apr 19, 2018
@pietroalbini
Copy link
Member Author

Bisect points at #49163 (wut?), cc @SimonSapin

@SimonSapin
Copy link
Contributor

Apr 12 11:35:07.538 INFO kablam!    Compiling sunrise v0.1.0 (file:///source)
Apr 12 11:35:17.931 INFO kablam! error[E0282]: type annotations needed
Apr 12 11:35:17.931 INFO kablam!    --> src/main.rs:270:23
Apr 12 11:35:17.932 INFO kablam!     |
Apr 12 11:35:17.932 INFO kablam! 270 |     let closest_to = |d| {
Apr 12 11:35:17.932 INFO kablam!     |                       ^ consider giving this closure parameter a type
Apr 12 11:35:17.932 INFO kablam! 
Apr 12 11:35:18.055 INFO kablam! error: aborting due to previous error

https://github.com/jonhoo/rust-at-sunrise/blob/52fcbf516f1f051d8cacad852f4a800b264194fc/src/main.rs#L270-L272

    let closest_to = |d| {
        let gt = perfs.range(d..).next();
        let lt = perfs.range(..d).next_back();

This closure passes RangeFrom<D> values to BTreeMap::range whose signature involves R: RangeBounds<T> (formerly R: RangeArgument<T>). Before #49163, the only corresponding impl were RangeFrom<T>: RangeArgument<T> so this was enough to deduce T = D and infer the type of d. #49163 added RangeFrom<&T>: RangeArgument<T>, making &T = D another possibility and the type of d now ambiguous.

@SimonSapin
Copy link
Contributor

I think this is business as usual when adding new impls. It is a minor change per https://github.com/rust-lang/rfcs/blob/master/text/1105-api-evolution.md#minor-change-implementing-any-non-fundamental-trait since the closure can be changed to |d| {…} to |d: DateTime<Utc>| {…} to fix this and still work in older versions.

jonhoo added a commit to jonhoo/rust-at-sunrise that referenced this issue Apr 19, 2018
@pietroalbini
Copy link
Member Author

Since the crate is now fixed and the breakage was minor and expected, I'm closing this. Thanks to everyone involved on this!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-inference Area: Type inference C-bug Category: This is a bug. P-high High priority regression-from-stable-to-beta Performance or correctness regression from stable to beta. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

5 participants