Move fulfillment into rustc_next_trait_solver - #160485
Conversation
rustbot
added
S-waiting-on-author
This comment has been minimized.
This comment has been minimized.
d614184 to
499da05
Compare
Signed-off-by: Amirhossein Akhlaghpour <m9.akhlaghpoor@gmail.com>
499da05 to
247c257
Compare
|
Some changes occurred to the core trait solver cc @rust-lang/initiative-trait-system-refactor |
rustbot
added
S-waiting-on-review
|
rustbot has assigned @ShoyuVanilla. Use Why was this reviewer chosen?The reviewer was selected based on:
|
|
r? lcnr |
| use super::delegate::SolverDelegate; | ||
| use crate::traits::{FulfillmentError, ScrubbedTraitError}; | ||
|
|
||
| #[path = "fulfill/derive_errors.rs"] |
There was a problem hiding this comment.
Why do we need this?
There was a problem hiding this comment.
I renamed the wrapper file. needed because the rustc-specific wrapper module was named rustc_fulfill while its file was still solve/fulfill.rs
| mod fulfill; | ||
| pub mod inspect; | ||
| mod normalize; | ||
| #[path = "solve/fulfill.rs"] |
There was a problem hiding this comment.
Not sure I quite understand this
There was a problem hiding this comment.
this was selecting file while exposing it locally as rustc_fulfill which wrapper renamed now
|
|
||
| /// An obligation that can be processed by the shared fulfillment engine. | ||
| /// | ||
| /// The concrete obligation and its diagnostic cause remain owned by the |
There was a problem hiding this comment.
This is merely curiosity, what would the alternative be to the diagnostic being owned by the frontend?
There was a problem hiding this comment.
would be for the shared fulfillment layer to own the complete obligation representation, including diagnostic metadata or even separate abstraction for constructing that metadata. the shared engine only accesses the parts needed for fulfillment through FulfillmentObligation
| } | ||
| } | ||
|
|
||
| fn into_rustc_solver_error<'tcx>( |
There was a problem hiding this comment.
Could this be a trait?
There was a problem hiding this comment.
that is cleaner , I did
| Overflow(O), | ||
| } | ||
|
|
||
| type PendingObligations<I, O> = ThinVec<(O, Option<GoalStalledOn<I>>)>; |
There was a problem hiding this comment.
Firstly I'm not overly familiar with this area of codebase, however, if I am not mistaken it looks like the type is PredicateObligation<'tcx>? Or an Obligation do we have something that O needs to implement or be constrained by? This applies to all instances of O, not exclusively this type definition.
There was a problem hiding this comment.
As shared engine only require the <I> while PredicateObligation<'tcx> not required.
I moved that bound onto FulfillmentCtxt<I, O> itself instead
There was a problem hiding this comment.
I think you cut yourself off mid-sentence?
There was a problem hiding this comment.
Sorry for the unclear wording :)
for rustc O is PredicateObligation<'tcx> but the shared engine only requires O: FulfillmentObligation<I> which moved that bound onto FulfillmentCtxt<I, O> itself now and it applies to every instance of the context.
Signed-off-by: Amirhossein Akhlaghpour <m9.akhlaghpoor@gmail.com>
6 participants
It moves the rustc fulfillment implementation into the shared crate
migrating rust-analyzer to the shared implementation will require the updated shared crates to be published and may need frontend specific adjustments
Part of #159654