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

Improve wording of "cannot multiply" type error #78063

Merged
merged 1 commit into from Oct 21, 2020

Conversation

camelid
Copy link
Member

@camelid camelid commented Oct 18, 2020

For example, if you had this code:

fn foo(x: i32, y: f32) -> f32 {
    x * y
}

You would get this error:

error[E0277]: cannot multiply `f32` to `i32`
 --> src/lib.rs:2:7
  |
2 |     x * y
  |       ^ no implementation for `i32 * f32`
  |
  = help: the trait `Mul<f32>` is not implemented for `i32`

However, that's not usually how people describe multiplication. People
usually describe multiplication like how the division error words it:

error[E0277]: cannot divide `i32` by `f32`
 --> src/lib.rs:2:7
  |
2 |     x / y
  |       ^ no implementation for `i32 / f32`
  |
  = help: the trait `Div<f32>` is not implemented for `i32`

So that's what this change does. It changes this:

error[E0277]: cannot multiply `f32` to `i32`
 --> src/lib.rs:2:7
  |
2 |     x * y
  |       ^ no implementation for `i32 * f32`
  |
  = help: the trait `Mul<f32>` is not implemented for `i32`

To this:

error[E0277]: cannot multiply `i32` by `f32`
 --> src/lib.rs:2:7
  |
2 |     x * y
  |       ^ no implementation for `i32 * f32`
  |
  = help: the trait `Mul<f32>` is not implemented for `i32`

@camelid camelid added A-diagnostics Area: Messages for errors, warnings, and lints D-inconsistent Diagnostics: Inconsistency in formatting, grammar or style between diagnostic messages. D-papercut Diagnostics: An error or lint that needs small tweaks. labels Oct 18, 2020
@rust-highfive
Copy link
Collaborator

r? @oli-obk

(rust_highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Oct 18, 2020
@camelid camelid force-pushed the improve-cannot-multiply-error branch from cc9712a to 9e1656e Compare October 18, 2020 03:08
@camelid
Copy link
Member Author

camelid commented Oct 18, 2020

For some reason it only seems to be working for some types – the old error persists for others, even though I can't find any other spot in the compiler that generates this error. Maybe my build cache is corrupted or something...

@jyn514 jyn514 added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Oct 18, 2020
Comment on lines +1 to +5
fn foo(x: i32, y: f32) -> f32 {
x * y //~ ERROR cannot multiply `i32` by `f32`
}

fn main() {}
Copy link
Member

Choose a reason for hiding this comment

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

Do you need this new test case? This is tested in plenty of other places.

Copy link
Member Author

Choose a reason for hiding this comment

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

I couldn't find a place where a simple case of the error was tested, and I wanted a sanity check since this is behaving weirdly. But if you think I should remove it once we figure out what's going awry, I guess I could. I just wanted extra coverage while I figured it out :)

Copy link
Member Author

Choose a reason for hiding this comment

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

I think I'm going to keep this test unless there are significant objections since this helped catch a spot that I didn't know to update.

@camelid
Copy link
Member Author

camelid commented Oct 18, 2020

@estebank What could explain this test failure from CI? I've been scratching my head and I also asked @jyn514 but neither of us could think of what this could be.

error: /checkout/src/test/ui/binop/binop-mul-i32-f32.rs:2: unexpected error: '2:7: 2:8: cannot multiply `f32` to `i32` [E0277]'

error: /checkout/src/test/ui/binop/binop-mul-i32-f32.rs:2: expected error not found: cannot multiply `i32` by `f32`

It seems the old error is being reported in some cases, and the new one in others.

@camelid
Copy link
Member Author

camelid commented Oct 18, 2020

I grepped through the whole compiler, but couldn't find anything even close to this error other than the spot I changed:

$ grep 'cannot multiply' -r compiler
compiler/rustc_typeck/src/check/op.rs:                                format!("cannot multiply `{}` by `{}`", lhs_ty, rhs_ty),

@SNCPlay42
Copy link
Contributor

It's from the rustc_on_unimplemented attribute on the Mul trait itself:

#[rustc_on_unimplemented(
message = "cannot multiply `{Rhs}` to `{Self}`",
label = "no implementation for `{Self} * {Rhs}`"
)]
#[doc(alias = "*")]
pub trait Mul<Rhs = Self> {

@qigangxu
Copy link

qigangxu commented Oct 18, 2020

I grepped through the whole compiler, but couldn't find anything even close to this error other than the spot I changed:

$ grep 'cannot multiply' -r compiler
compiler/rustc_typeck/src/check/op.rs:                                format!("cannot multiply `{}` by `{}`", lhs_ty, rhs_ty),

There are two lines of code similar in the library directory:

library/core/src/ops/arith.rs:305:    message = "cannot multiply `{Rhs}` to `{Self}`",
library/core/src/ops/arith.rs:829:    message = "cannot multiply-assign `{Rhs}` to `{Self}`",

@camelid
Copy link
Member Author

camelid commented Oct 18, 2020

Wow, thank you! I didn't think to look in library/.

@camelid
Copy link
Member Author

camelid commented Oct 18, 2020

Why would the error be produced in the compiler and the standard library?

For example, if you had this code:

    fn foo(x: i32, y: f32) -> f32 {
        x * y
    }

You would get this error:

    error[E0277]: cannot multiply `f32` to `i32`
     --> src/lib.rs:2:7
      |
    2 |     x * y
      |       ^ no implementation for `i32 * f32`
      |
      = help: the trait `Mul<f32>` is not implemented for `i32`

However, that's not usually how people describe multiplication. People
usually describe multiplication like how the division error words it:

    error[E0277]: cannot divide `i32` by `f32`
     --> src/lib.rs:2:7
      |
    2 |     x / y
      |       ^ no implementation for `i32 / f32`
      |
      = help: the trait `Div<f32>` is not implemented for `i32`

So that's what this change does. It changes this:

    error[E0277]: cannot multiply `f32` to `i32`
     --> src/lib.rs:2:7
      |
    2 |     x * y
      |       ^ no implementation for `i32 * f32`
      |
      = help: the trait `Mul<f32>` is not implemented for `i32`

To this:

    error[E0277]: cannot multiply `i32` by `f32`
     --> src/lib.rs:2:7
      |
    2 |     x * y
      |       ^ no implementation for `i32 * f32`
      |
      = help: the trait `Mul<f32>` is not implemented for `i32`
@camelid camelid force-pushed the improve-cannot-multiply-error branch from 9e1656e to 7b33ae6 Compare October 18, 2020 05:19
@camelid camelid marked this pull request as ready for review October 18, 2020 05:20
@SNCPlay42
Copy link
Contributor

rustc_on_unimplemented is for getting a nicer wording than "the trait std::ops::Mul<f32> is not implemented for i32" on trait errors in general, which might not directly involve actually using the operation, e.g.

use std::ops::Add;
fn incr<T:Add<i32>>(x: T) -> T::Output {
    x + 1 // this is fine
}

fn main() {
    incr(()); //~ ERROR cannot add `i32` to `()`
}

It's an attribute placeable on library traits rather than hardcoded into the compiler because it's used on all sorts of stdlib traits (not just the binops) that might not even be lang items, e.g. Display.

We get that kind of error in your test because the diagnostics in check_overloaded_binop only fire if the LHS type doesn't implement the relevant trait at all, and another code path handles other cases (see also #77304), and obviously i32 does implement Mul<T> for some values of T.

@estebank
Copy link
Contributor

@bors r+

@bors
Copy link
Contributor

bors commented Oct 19, 2020

📌 Commit 7b33ae6 has been approved by estebank

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Oct 19, 2020
JohnTitor added a commit to JohnTitor/rust that referenced this pull request Oct 21, 2020
…or, r=estebank

Improve wording of "cannot multiply" type error

For example, if you had this code:

    fn foo(x: i32, y: f32) -> f32 {
        x * y
    }

You would get this error:

    error[E0277]: cannot multiply `f32` to `i32`
     --> src/lib.rs:2:7
      |
    2 |     x * y
      |       ^ no implementation for `i32 * f32`
      |
      = help: the trait `Mul<f32>` is not implemented for `i32`

However, that's not usually how people describe multiplication. People
usually describe multiplication like how the division error words it:

    error[E0277]: cannot divide `i32` by `f32`
     --> src/lib.rs:2:7
      |
    2 |     x / y
      |       ^ no implementation for `i32 / f32`
      |
      = help: the trait `Div<f32>` is not implemented for `i32`

So that's what this change does. It changes this:

    error[E0277]: cannot multiply `f32` to `i32`
     --> src/lib.rs:2:7
      |
    2 |     x * y
      |       ^ no implementation for `i32 * f32`
      |
      = help: the trait `Mul<f32>` is not implemented for `i32`

To this:

    error[E0277]: cannot multiply `i32` by `f32`
     --> src/lib.rs:2:7
      |
    2 |     x * y
      |       ^ no implementation for `i32 * f32`
      |
      = help: the trait `Mul<f32>` is not implemented for `i32`
bors added a commit to rust-lang-ci/rust that referenced this pull request Oct 21, 2020
Rollup of 7 pull requests

Successful merges:

 - rust-lang#77726 (Add Pin::static_ref, static_mut.)
 - rust-lang#78002 (Tweak "object unsafe" errors)
 - rust-lang#78056 (BTreeMap: split off most code of remove and split_off)
 - rust-lang#78063 (Improve wording of "cannot multiply" type error)
 - rust-lang#78094 (rustdoc: Show the correct source filename in page titles, without `.html`)
 - rust-lang#78101 (fix static_ptr_ty for foreign statics)
 - rust-lang#78118 (Inline const followups)

Failed merges:

r? `@ghost`
@bors bors merged commit 89c98cd into rust-lang:master Oct 21, 2020
@rustbot rustbot added this to the 1.49.0 milestone Oct 21, 2020
@camelid camelid deleted the improve-cannot-multiply-error branch October 21, 2020 18:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-diagnostics Area: Messages for errors, warnings, and lints D-inconsistent Diagnostics: Inconsistency in formatting, grammar or style between diagnostic messages. D-papercut Diagnostics: An error or lint that needs small tweaks. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

9 participants