(WIP) Trait specs#127
Conversation
21e3688 to
a7c4fb4
Compare
9afce0c to
c7e9910
Compare
|
There is one regression related to generics (not providing the correct substs, somewhere?). @JonasAlaif can you have a look at the overall structure in |
5c9a842 to
294721d
Compare
JonasAlaif
left a comment
There was a problem hiding this comment.
I didn't fully go through everything, but for now these changes should make things nicer and then I can go through the rest
| type TaskDescription<'tcx> = ( | ||
| DefId, // The function annotated with specs | ||
| bool, // If to encode as pure or not | ||
| DefId, // Context, i.e., where the specs are emitted |
There was a problem hiding this comment.
All uses pass in the same DefId for both? It also doesn't make sense to have a different spec from the point of view of different contexts? Let's remove the second DefId?
There was a problem hiding this comment.
This is needed to encode the behavioural subtyping checks: in the proof methods, we need to check the contract of the trait from the context of the implementor, i.e., we need to instantiate the contract with the correct TraitRef substitutions applied.
There was a problem hiding this comment.
Where is that? When I find all uses of MirSpecEnc it always takes the exact same two DefIds as input (i.e. (def_id, def_id, ...))
There was a problem hiding this comment.
There was a problem hiding this comment.
Ah ok, sorry yeah.
we need to instantiate the contract with the correct TraitRef substitutions applied
So you mean casting to concrete/generic correctly? It still seems like a lot of extra complication in MirSpecEnc for this one case - could we not add something like a MirSpecUseEnc wrapper that provides an api with the casts applied as necessary?
| for def_id in tcx.hir_crate_items(()).definitions() { | ||
| if let hir::def::DefKind::Impl { of_trait: true } = tcx.def_kind(def_id) { | ||
| TraitImplEnc::encode(def_id.to_def_id(), false).unwrap(); | ||
| match tcx.def_kind(def_id) { |
There was a problem hiding this comment.
We should think about if these are necessary (since we're also skipping all the traits defined outside this crate anyway). Or do we need this to check behavioural subtyping? In #137 the hir::def::DefKind::Impl case is removed, since the TraitEnc goes through all impls anyway.
There was a problem hiding this comment.
Yes, TraitImplEnc causes the behavioural subtyping checks to be emitted. Any crate can define:
- types
We don't have to force the encoding of a type if it appears in a crate but doesn't get used; though it would be surprising from a Prusti user perspective if a type that causes Prusti to error out only does this when verifying a downstream crate that uses that type. - traits
When there are method bodies, we already encode them (it is a body owner in the crate). For methods without a default implementation, we should make sure the specs are well-formed, which will happen in (WIP) Encode spec items as methods #138. Associated types etc probably don't matter much when the trait is not used. - trait
implblocks connecting (sets of) types to traits, where either the type or the trait may be coming from an upstream crate
Here we should emit the behavioural subtyping check, and it shouldn't be pushed to downstream crates, i.e., iterating through all knownimpls (as in Concrete trait impl checks #137) seems wrong to me here.
| substs, | ||
| // TODO: should this be `def_id` or `caller_def_id` | ||
| caller_def_id: Some(def_id), | ||
| caller_def_id: Some(context_def_id), |
There was a problem hiding this comment.
We should have a deep think about if this caller_def_id is necessary. Maybe there should be a MirPureUseEnc which does all the required casting of arguments to/from generic?
There was a problem hiding this comment.
Ok I guess this is ultimately a question of whether we want to trust that our encoding of generics will lead to the same results as rustc monomorphisation. A different caller_def_ids here will cause MirPureEnc to request a different body monomorphisation, which eventually affects the choice of the typing environment here:
(Which can then change whether we know a particular associated type resolves to a concrete type, for example.)
Either way I would prefer to postpone this to after this PR.
|
Ok so this causes a cycle: trait Bar {
type Target;
fn deref(x: Self::Target);
}And of course it would -- in trying to create the function identifier for So as you suggested at some point @JonasAlaif I think I need to change this to have a separate encoder for the trait itself (its |
|
Merged to unblock other PRs, in particular #137. |
For each trait:
For each trait impl:
If using generic encoding:
If monomorphising (which gets us better error tracking):