Skip to content

Conversation

@samueltardieu
Copy link
Member

@samueltardieu samueltardieu commented Jul 3, 2025

Fix several issues with manual_is_multiple_of

  • &a % &b == 0 compiles, but requires dereferencing b when replacing with a.is_multiple_of(b).
  • In a % b == 0, if type of a is not certain, a.is_multiple_of(b) might not be typable.
  • In a % b == 0, a and b must be unsigned integers, not any arbitrary types implementing Rem and outputing an integer.

Those fixes have required increasing the precision of type certainty determination in the two first patches.

Fixes #15203
Fixes #15204

changelog: [manual_is_multiple_of]: fix various false positive

However, if they appear within the context of a function argument, they
will be marked as certain, as they might eventually be considered `i32`
by default if the context is not more specific.

Also, in the case of binary expressions, if one of the side is
uncertain, then the certainty will come from the other side.
`Node::Param(…)` can represent function parameters as well as closure
parameters. In the second case, a type is not always given. When no
type is given, consider it uncertain.
- `&a % &b == 0` compiles, but requires dereferencing `b` when
  replacing with `a.is_multiple_of(b)`.
- In `a % b == 0`, if type of `a` is not certain,
  `a.is_multiple_of(b)` might not be typable.
- In `a % b == 0`, `a` and `b` must be unsigned integers, not any
  arbitrary types implementing `Rem` and outputing an integer.
@rustbot
Copy link
Collaborator

rustbot commented Jul 3, 2025

r? @Jarcho

rustbot has assigned @Jarcho.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties label Jul 3, 2025
@samueltardieu

This comment was marked as outdated.

@rustbot rustbot added the beta-nominated Nominated for backporting to the compiler in the beta channel. label Jul 3, 2025
@samueltardieu
Copy link
Member Author

@rustbot note remove Beta-nomination
@rustbot label -beta-nominated

Despite what is in the code, this lint has been introduced in 1.90 only. Those fixes only need to be merged before the next beta is cut from master.

@rustbot rustbot removed the beta-nominated Nominated for backporting to the compiler in the beta channel. label Jul 3, 2025
Copy link
Contributor

@Jarcho Jarcho left a comment

Choose a reason for hiding this comment

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

Thank you.

@Jarcho Jarcho added this pull request to the merge queue Jul 14, 2025
Merged via the queue into rust-lang:master with commit 1c64211 Jul 14, 2025
15 of 17 checks passed
@rustbot rustbot removed the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties label Jul 14, 2025
@samueltardieu samueltardieu deleted the fix-manual-is-multiple-of branch July 14, 2025 14:45
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.

manual_is_multiple_of triggers on arbitrary types implementing Rem and outputing an integer Smarter clippy::manual_is_multiple_of ?

3 participants