Skip to content

lint suboptimal_flops' mul_add in const context - #17631

Open
Gri-ffin wants to merge 2 commits into
rust-lang:masterfrom
Gri-ffin:imprecise_flops/mul_add
Open

lint suboptimal_flops' mul_add in const context#17631
Gri-ffin wants to merge 2 commits into
rust-lang:masterfrom
Gri-ffin:imprecise_flops/mul_add

Conversation

@Gri-ffin

Copy link
Copy Markdown
Contributor

changelog: [suboptimal_flops]: lint mul_add in const context

@rustbot rustbot added the S-waiting-on-community-reviews Status: This is awaiting for positive reviews from the community before a maintainer is assigned. label Aug 25, 2026
@rustbot

rustbot commented Aug 25, 2026

Copy link
Copy Markdown
Collaborator

Thanks for the pull request. A reviewer will take a look after it receives 2 community reviews.

In the meantime, we would highly appreciate if you could try to review any of PRs waiting on community reviews.

@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties label Aug 25, 2026

@CommanderStorm CommanderStorm left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I mean, yes your impl would work for the moment.
Could we make it work with less maintenance required? For example via Msrv::is_stable_or_const_stable?
The fn (or its siblings) look like the thing we likely want 🤔

The added nesting by is_in_const feels a bit improperly, not sure why..

View changes since this review

@Gri-ffin

Gri-ffin commented Aug 26, 2026

Copy link
Copy Markdown
Contributor Author

is_stable_or_const_stable needs a DefId , but we are triggering here on basic arithmetic, mul_add is the suggestion we are emitting so its not in the HIR, and AFAIK, hardcoding the version is the standard clippy pattern to check the MSRV.

The added nesting by is_in_const feels a bit improperly, not sure why..

It was either centralizing the check in mod.rs file, or sprinkling the checks in every file (which doesn't seem to me a good idea to do for only a single case).

@CommanderStorm CommanderStorm left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Fair, I still get confused with hir vs the rest.

View changes since this review

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

S-waiting-on-community-reviews Status: This is awaiting for positive reviews from the community before a maintainer is assigned. S-waiting-on-review Status: Awaiting review from the assignee but also interested parties

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants