Join GitHub today
GitHub is home to over 31 million developers working together to host and review code, manage projects, and build software together.
Sign upRegression in #[derive(Eq)] on type with non-Eq members. #36830
Comments
TimNN
added
the
regression-from-stable-to-beta
label
Sep 29, 2016
This comment has been minimized.
This comment has been minimized.
|
Caused by #36384 |
This comment has been minimized.
This comment has been minimized.
|
Explanation: previous ".assert_param_is_eq" used method resolution, finding slice methods on arrays. New code only looks at the type itself (the array). |
This comment has been minimized.
This comment has been minimized.
|
Theoretically, this is a bugfix, because the previous implementation didn't enforced the bounds properly. |
This comment has been minimized.
This comment has been minimized.
|
Without this road block, there would be many more Eq implementations on structs with floats in them. |
This comment has been minimized.
This comment has been minimized.
|
Should maybe crater #36384 to see the "bugfix" impact. |
alexcrichton
added
I-nominated
T-libs
labels
Sep 29, 2016
This comment has been minimized.
This comment has been minimized.
|
Discussed during libs triage, conclusion was that we're likely to let this slide (as it's a bug fix) but we'll wait for the next crater build to ensure that no massive impact is seen. |
alexcrichton
removed
the
I-nominated
label
Oct 3, 2016
brson
assigned
alexcrichton
Oct 6, 2016
This comment has been minimized.
This comment has been minimized.
|
Ok, ran crater, there are two regressions related to this I believe the latter is yours, right @rphmeier? The former presumably hasn't been reported yet. |
This comment has been minimized.
This comment has been minimized.
|
adding back the nominated tag to discuss again. |
alexcrichton
added
the
I-nominated
label
Oct 7, 2016
This comment has been minimized.
This comment has been minimized.
|
@alexcrichton right, I haven't published the fix to crates.io yet though. |
This comment has been minimized.
This comment has been minimized.
|
Ok, the libs team discussed this today and the conclusion was that we can just send PRs to fix these issues. @rphmeier to clarify have you got this fixed locally? If you need any help I can send a PR! |
This comment has been minimized.
This comment has been minimized.
|
@bluss holy cow thanks so much for sending a PR to fix |
This comment has been minimized.
This comment has been minimized.
|
@alexcrichton Yep, fixed in the repo, just haven't pushed the fix to crates.io yet. I'll try and do it tomorrow. |
This comment has been minimized.
This comment has been minimized.
|
Ok, in that case I'm going to close this, thanks for the update though! |
rphmeier commentedSep 29, 2016
Playground: https://play.rust-lang.org/?gist=ca6a481e640668153d66259c0ef1dde6&version=stable&backtrace=0
Succeeds on
rustc 1.11.0 (9b21dcd6a 2016-08-15), but fails onrustc 1.13.0-beta.1 (cbbeba430 2016-09-28)Seems to be related to
std::cmp::AssertParamIsEq.