Skip to content

Linting shouldn't be run on optimized MIR #6080

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Open
ecstatic-morse opened this issue Sep 24, 2020 · 5 comments
Open

Linting shouldn't be run on optimized MIR #6080

ecstatic-morse opened this issue Sep 24, 2020 · 5 comments
Labels
C-bug Category: Clippy is not doing the correct thing E-hard Call for participation: This a hard problem and requires more experience or effort to work on T-MIR Type: This lint will require working with the MIR

Comments

@ecstatic-morse
Copy link
Contributor

ecstatic-morse commented Sep 24, 2020

For example, const-checking needs to run before optimizations, otherwise it might miss some invalid operations that get optimized away. Specifically, it should run on the result of the mir_const query, but currently it uses optimized_mir. This is because the result of the mir_const query is "stolen" instead of cloned, so it no longer exists when clippy runs. clippy tries to work around this by setting mir-opt-level=0, but this doesn't disable every optimization pass.

Until this is fixed, missing_const_for_fn will have false positives and/or ICE (due to some assertions about the state of the MIR at the point const-checking is run).

@ecstatic-morse ecstatic-morse added C-bug Category: Clippy is not doing the correct thing I-ICE Issue: Clippy panicked, giving an Internal Compilation Error (ICE) ❄️ labels Sep 24, 2020
@ecstatic-morse ecstatic-morse changed the title missing_const_for_fn runs on optimized MIR Linting shouldn't be run on optimized MIR Sep 25, 2020
@camsteffen
Copy link
Contributor

According to @oli-obk on zulip, we can now use mir_for_ctfe.

@camsteffen camsteffen added E-hard Call for participation: This a hard problem and requires more experience or effort to work on T-MIR Type: This lint will require working with the MIR labels Jan 26, 2021
bors added a commit that referenced this issue Mar 5, 2021
Remove a couple MIR usages

changelog: none

We use MIR to get the return type of a closure/function in a couple places. But typeck seems like a better approach.

This is the easy part of #6080.

Also did a tiny cleanup with `typeck` -> `typeck_body`.
@camsteffen
Copy link
Contributor

I tried to switch over to mir_for_ctfe but was harshly denied, "Turn aside, heathen! None shall enter except he be const!". Chatted again with @oli-obk and he suggested we should add tcx.unoptimized_mir to simply bypasses the assertion.

@b-NC
Copy link
Contributor

b-NC commented May 31, 2022

I tried to switch over to mir_for_ctfe but was harshly denied, "Turn aside, heathen! None shall enter except he be const!". Chatted again with @oli-obk and he suggested we should add tcx.unoptimized_mir to simply bypasses the assertion.

@camsteffen is this still something to be fixed ?

@camsteffen
Copy link
Contributor

@InfRandomness yes I believe so

@1c3t3a
Copy link
Member

1c3t3a commented Jan 16, 2025

Checking in once more: Would it be possible to add tcx.unoptimized_mir? I hit this issue again in #14003 and I think this would be a clean solution? Don't know about the performance cost, though.

@Jarcho Jarcho removed the I-ICE Issue: Clippy panicked, giving an Internal Compilation Error (ICE) ❄️ label Mar 9, 2025
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
C-bug Category: Clippy is not doing the correct thing E-hard Call for participation: This a hard problem and requires more experience or effort to work on T-MIR Type: This lint will require working with the MIR
Projects
None yet
Development

No branches or pull requests

5 participants