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

librustc: Don't ICE when operator traits are not implemented properly. #12638

Merged
merged 1 commit into from
Mar 1, 2014

Conversation

luqmana
Copy link
Member

@luqmana luqmana commented Mar 1, 2014

From my comment on #11450:

The reason for the ICE is because for operators rustc does a little bit of magic. Notice that while you implement the Mul trait for some type &T (i.e a reference to some T), you can simply do Vec2 {..} * 2.0f32. That is, 2.0f32 is f32 and not &f32. This works because rustc will automatically take a reference. So what's happening is that with foo * T, the compiler is expecting the mul method to take some &U and then it can compare to make sure T == U (or more specifically that T coerces to U). But in this case, the argument of the mul method is not a reference and hence the "no ref" error.

I don't think we should ICE in this case since we do catch the mismatched trait/impl method and hence provide a better error message that way.

Fixes #11450

@@ -1710,12 +1710,7 @@ pub fn check_expr_with_unifier(fcx: @FnCtxt,
// For variadic functions, we don't have a declared type for all of
// the arguments hence we only do our usual type checking with
// the arguments who's types we do know.
let t = if variadic {
Copy link
Member

Choose a reason for hiding this comment

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

Why does this have to change as well?

Copy link
Member Author

Choose a reason for hiding this comment

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

Oh right, it's not related to this change but just a nit I forgot to fix way back when :P We don't need the if there since if !variadic then expected_arg_count == supplied_arg_count. Thus we can always just use expected_arg_count there.

Copy link
Member

Choose a reason for hiding this comment

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

I'm not familiar with this code, so I don't know enough to confirm or deny this fact, but if you add an assertion that the two counts are the same then r=me

bors added a commit that referenced this pull request Mar 1, 2014
From my comment on #11450:

The reason for the ICE is because for operators `rustc` does a little bit of magic. Notice that while you implement the `Mul` trait for some type `&T` (i.e a reference to some T), you can simply do `Vec2 {..} * 2.0f32`. That is, `2.0f32` is `f32` and not `&f32`. This works because `rustc` will automatically take a reference. So what's happening is that with `foo * T`, the compiler is expecting the `mul` method to take some `&U` and then it can compare to make sure `T == U` (or more specifically that `T` coerces to `U`). But in this case, the argument of the `mul` method is not a reference and hence the "no ref" error.

I don't think we should ICE in this case since we do catch the mismatched trait/impl method and hence provide a better error message that way.

Fixes #11450
@bors bors closed this Mar 1, 2014
@bors bors merged commit a174941 into rust-lang:master Mar 1, 2014
@luqmana luqmana deleted the op-no-ref branch June 5, 2014 23:18
bors added a commit to rust-lang-ci/rust that referenced this pull request Jul 25, 2022
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.

internal compiler error: no ref
4 participants