-
Notifications
You must be signed in to change notification settings - Fork 13.3k
Fixing confusion between mod and remainder #107389
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
Conversation
r? @estebank (rustbot has picked a reviewer for you, use r? to override) |
Hey! It looks like you've submitted a new PR for the library teams! If this PR contains changes to any Examples of
|
compiler/rustc_hir_typeck/src/op.rs
Outdated
@@ -335,7 +335,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { | |||
format!("cannot divide `{lhs_ty}` by `{rhs_ty}`") | |||
} | |||
hir::BinOpKind::Rem => { | |||
format!("cannot mod `{lhs_ty}` by `{rhs_ty}`") | |||
format!("cannot rem `{lhs_ty}` by `{rhs_ty}`") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually, let's expand the wording here to make it even clearer (feel free to change the wording further), here and elsewhere
format!("cannot rem `{lhs_ty}` by `{rhs_ty}`") | |
format!("cannot calculate the remainder of `{lhs_ty}` divided by `{rhs_ty}`") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good idea, fixed it.
@bors r+ |
Like many programming languages, rust too confuses remainder and modulus. The
%
operator and the associatedRem
trait is (as the trait name suggests) the remainder, but since most people are linguistically more familiar with the modulus the documentation sometimes claims otherwise. This PR tries to fix this problem in rustc.