-
Notifications
You must be signed in to change notification settings - Fork 13.1k
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
Cleanup opaque_type_cycle_error
#135307
base: master
Are you sure you want to change the base?
Cleanup opaque_type_cycle_error
#135307
Conversation
This comment has been minimized.
This comment has been minimized.
LGTM. The CI failure is unrelated. r? compiler-errors @bors r+ rollup |
🌲 The tree is currently closed for pull requests below priority 1000. This pull request will be tested once the tree is reopened. |
…e-error, r=compiler-errors Cleanup `opaque_type_cycle_error` Small cleanup opportunity found while reading the code: the `label: bool` var isn't really needed since there's only one execution path that sets is to `true`. Also, tried to reduce right-ward drift. Best reviewed by hiding whitespace
Gonna close/reopen to bump CI, I think this caused the rollup to fail #135317 (comment) |
@bors r- |
This comment has been minimized.
This comment has been minimized.
Ahh jeez, sorry about that @compiler-errors , I was accidentally running the wrong tests locally before pushing 😅 I dived a little deeper into this function and it seems that this rust/compiler/rustc_hir_analysis/src/check/check.rs Lines 1722 to 1726 in b44e14f
i.e., I replaced its body with a panic!() and ran ./x suggest --run , and all the tests passed.So either it's unreachable, and return paths never resolve to a ty::Never , or there's no test exercising this. So I tried to but couldn't come up with a test that does, see futile attempt here: https://godbolt.org/z/91fW8Ea5bAny idea what's going on here? |
ad0e8dd
to
b931b4b
Compare
Fixed for now, but couldn't find a decent way to not duplicate the |
Small cleanup opportunity found while reading the code: the
label: bool
var isn't really needed since there's only one execution path that sets is totrue
. Also, tried to reduce right-ward drift.Best reviewed by hiding whitespace