MIR dead code analysis affects TAIT inference

[compiler-errors]

Given the code:

#![feature(type_alias_impl_trait)]

type Tait = impl Sized;

struct One;
fn one() -> Tait { One }

struct Two<T>(T);
fn two() -> Tait { Two::<()>(todo!()) }

If you run the code, you can see that the compiler ends up "successfully" inferencing Tait = One and succeeding compilation, even though upon visual inspection, the compiler should be raising an error since fn two should cause the conflicting inference Tait = Two<()>.

The key here is the todo!() macro, which causes the SimplifyCfg MIR pass to remove MIR that actually causes us to make this second inference.

Is this expected? How would we even change this behavior, since it would require us to disentangle TAIT inference from borrowck..?

[compiler-errors]

I want to fix this but have no idea how I would begin, lol.

[oli-obk]

I'm fairly certain we don't run simplify cfg on MIR before doing borrowck.

Borrowck still does enough dataflow analysis to realize that everything after the panic is dead. I guess typeck will to it correctly... so maybe we have to return all hidden types from typeck and seed the borrowck hidden types with it (so that borrowck only fills in the lifetimes instead of also figuring out the types)

[compiler-errors]

doesn't this run before borrowck: https://doc.rust-lang.org/nightly/nightly-rustc/src/rustc_mir_transform/lib.rs.html#244

[oli-obk]

Oh... huh... i was sure I've seems dead code in borrowck before, guess not. Either way, the "grab things from typeck" plan would work here, too

[compiler-errors]

Either way, the "grab things from typeck" plan would work here, too

I'm finding a few issues with this strategy. Firstly, since the typeck results have their non-late regions erased, I'm not exactly sure how we're supposed to use the erased hidden type to seed the borrowck hidden types.

One thought I had was that we could capture the opaque type key and hidden type's lifetimes in a binder, like generator interior does to avoid being erased in the typeck resuls.
So if we had inferred Tait<'a> = &'a () during typeck, the typeck results would then store Binder(OpaqueTypeKey { def_id: Tait, substs: [ReLateBound#1] }, HiddenTy { ty: &ReLateBound#1 () }, [BrAnon(1)]) or something, if that makes sense.

But I found a problem that it's not even guaranteed there's a useful relationship between the regions present in the hidden type and those present in the substs of the opaque key...
That is, I'm finding that all we have in opaque type storage during writeback is something more like Tait<'a> = &'_#1r () -- maybe there's a relationship between that '_#1r, but it seems impossible to extract that relationship during writeback... we don't even do lexical region resolution anymore it seems, so fully_resolve on that region just gives us ReErased 😓.

[oli-obk]

Yes, that's why we actually need to do opaque type resolution in borrowck. The typeck hidden type would only be used as the initial value for the opaque types constrained by this item. All erased regions can be replaced by inference vars (which is effectively what borrowck does for all regions in the MIR). Then, the each time the hidden type is constrained again, the inference lifetimes from the seed hidden type are compared against the lifetimes of the new hidden type. If the only constraining happens in dead code, you'll end up with a lifetime error (unless the type has no lifetimes)

[compiler-errors]

I'm mostly writing this out for my understanding, and to express some continuing confusion.

So I've gathered that we handle opaque types in two stages during borrowck -- first during type_check, where we collect a list of opaques opaque_type_values: VecMap<OpaqueTypeKey, OpaqueTypeDecl> , and then later on in RegionCtxt::infer_opaque_types, where squash all of the OpaqueTypeKeys with different substs into a VecMap<DefId, OpaqueHiddenTy> by mapping each key's substs back to the opaque's substs...

Between those two places, I have no idea where we'd put this new logic. I don't think we can just stick typeck's erased copy of these hidden types into the opaque_type_values mapping collected during mir type check, since we don't have any substs that go with this opaque type to make an OpaqueTypeKey out of.

On the other hand, I don't know where in infer_opaque_types I would put this logic either. Seems like it would have to live after all of the infer_opaque_definition_from_instantiation calls, since only then do we have a VecMap<LocalDefId, OpaqueHiddenType<'tcx>> to actually compare against the VecMap<LocalDefId, Ty<'tcx>> we get from typeck, to perhaps fill in any missing types that didn't get constrained during borrowck.

[oli-obk]

since we don't have any substs that go with this opaque type to make an OpaqueTypeKey out of.

You have identity substs, so yea, we could do it at the end where we convert everything to identity substs anyway

[compiler-errors]

we could do it at the end where we convert everything to identity substs anyway

yup i can see this being the right way to do it. working on it now.

[oli-obk]

we could do it at the end where we convert everything to identity substs anyway

This sounds straight forward, and the implementation is, but now libcore doesn't compile anymore, because lifetime resolution is already done and thus the lifetimes of the identity-substituted type can't be equated to the other types that were already found.

Maybe we should only assert at the end that the hidden type found by typeck is the same as the one found by borrowck (modulo lifetimes). If borrowck found no hidden type, but typeck did, use the typeck type (with erased regions) to check that all other hidden types that were found by other functions at least match (again, modulo regions)

[compiler-errors]

Yeah, I was curious if we could move all of this logic into find_opaque_ty_constraints_for_tait actually. We could make a fresh infcx there and equate all the different hidden types we find both during typeck (with erased regions -> fresh regions) and the ones we find during borrowck?

[oli-obk]

Yea, that seems more reasonable. Started doing that an hour ago and ironically I immediately found dead code in the opaque type processing logic 😆