-
Notifications
You must be signed in to change notification settings - Fork 12.8k
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
Use ObligationCtxt
in custom type ops
#111741
Conversation
r? @TaKO8Ki (rustbot has picked a reviewer for you, use r? to override) |
This comment has been minimized.
This comment has been minimized.
be3161d
to
02cb5c4
Compare
This comment has been minimized.
This comment has been minimized.
02cb5c4
to
8b6961d
Compare
r? @lcnr feel free to reassign (maybe to oli) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
r=me after nits
// delay more bugs than accidentally not delay a bug at all. | ||
self.type_checker.tcx().sess.delay_span_bug( | ||
self.locations.span(self.type_checker.body), | ||
format!("errors selecting obligation during MIR typeck"), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
format!("errors selecting obligation during MIR typeck"), | |
"errors selecting obligation during MIR typeck", |
when can we hit that case, given that we previously used unwrap
here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Happens because if we hit an error while processing the region obligations, we return Err(NoSolution)
instead of just Ok
. Shouldn't matter anyways because we delay a bug both here and within scrape, but it's probably better to migrate to returning ErrorGuaranteed
like you said below. I'll do it in a quick follow-up to avoid having re-review 😅
compiler/rustc_trait_selection/src/traits/query/type_op/custom.rs
Outdated
Show resolved
Hide resolved
infcx.tcx.sess.delay_span_bug( | ||
DUMMY_SP, | ||
format!("errors selecting obligation during MIR typeck: {:?}", errors), | ||
); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I feel like we may want to move that delay span bug out of the commit_if_ok
and change scrape_region_constraints
to return ErrorGuaranteed
, think that may be more involved though so it doesn't have to be in this PR
8b6961d
to
521a0bc
Compare
@bors r=lcnr |
…earth Rollup of 5 pull requests Successful merges: - rust-lang#111741 (Use `ObligationCtxt` in custom type ops) - rust-lang#111840 (Expose more information in `get_body_with_borrowck_facts`) - rust-lang#111876 (Roll compiler_builtins to 0.1.92) - rust-lang#111912 (Use `Option::is_some_and` and `Result::is_ok_and` in the compiler ) - rust-lang#111915 (libtest: Improve error when missing `-Zunstable-options`) r? `@ghost` `@rustbot` modify labels: rollup
…r, r=lcnr Use `ErrorGuaranteed` more in MIR type ops Delay bugs more eagerly and pass them through type op infra instead of delaying them at all the usage-sites. Follow up to: rust-lang#111741 (comment) r? `@lcnr`
… r=lcnr Use `ErrorGuaranteed` more in MIR type ops Delay bugs more eagerly and pass them through type op infra instead of delaying them at all the usage-sites. Follow up to: rust-lang#111741 (comment) r? `@lcnr`
Use `ErrorGuaranteed` more in MIR type ops Delay bugs more eagerly and pass them through type op infra instead of delaying them at all the usage-sites. Follow up to: rust-lang/rust#111741 (comment) r? `@lcnr`
We already make one when evaluating the
CustomTypeOp
, so it's simpler to just pass it to the user. Removes a redundantObligationCtxt::new_in_snapshot
usage and simplifies some other code.This makes several refactorings related to opaque types in the new solver simpler, but those are not included in this PR.