Rewrite never_loop as a strict reachability pass
#11447
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @Centri3 (or someone else) soon. Please see the contribution instructions for more information. Namely, in order to ensure the minimum review times lag, PR authors and assigned reviewers should ensure that the review label (
|
rustbot
added
the
S-waiting-on-review
bors
added a commit
that referenced
this pull request
Sep 2, 2023
`never_loop` catches `loop { panic!() }`
* Depends on: #11447
This is an outgrowth of #11447 which I felt would best be done as a separate PR because it yields significant new results.
This uses typecheck results to determine divergence, meaning we can now detect cases like `loop { std::process::abort() }` or `loop { panic!() }`. A downside is that `loop { unimplemented!() }` is also being linted, which is arguably a false positive. I'm not really sure how to check this from HIR though, and it seems best to leave this epicycle for a later PR.
changelog: [`never_loop`]: Now lints on `loop { panic!() }` and similar constructs
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes #11004. This is a "more principled" rewrite of
never_loopwhich does a proper reachability analysis ofcontinue 'main_loop.IgnoreUntilEnd(label)which had an unclear (and ultimately incorrect) relationship to reachability of labels and divergence, local label reachability is now being tracked once per label in thelocal_labelslist (previouslyignore_ids)AlwaysBreakandOtherwisehave been renamed toDivergingandNormalrespectivelyMayContinueMainLoop- this was the source of one of the bugs.The constant check from #11005 was removed, because it is no longer necessary to avoid the FP and it is necessarily quite approximate (it only checks for a literal
truein the if statement, which would already be caught by another lint). Since we are using a sound underapproximation now, any if statement will simply be treated as if both branches are reachable, no matter the condition. This ensures no FPs at the cost of some FNs (which IIUC is the desired error direction). But we could put that check back in (or something like it) if desired.changelog: [
never_loop]: Handles more edge cases involving labeled breaks