Skip to content

opaque types: consider yeeting structural lookup #271

Description

@lcnr

We currently lookup opaque types in the opaque type storage by storing the key in a hashmap
https://github.com/rust-lang/rust/blob/b2fabe39bde5174e8d728bb85f2b8d0572c35b74/compiler/rustc_infer/src/infer/opaque_types/table.rs#L13-L14

This is annoying and brittle if the key contains inference variables or regions. If we make inference progress keys end up overlapping without us noticing. This is especially annoying for the bivariant lifetime parameters of the opaque, though that could be solved by replacing them with 'erased before putting them in storage.

One of the cases where this comes up is

fn foo() -> impl Sized {
    if false {
        let _: (u32, _)  = foo();
    }

    (Default::default(), 1i32)
}

fn bar<'a>() -> impl Sized {
    if false {
        let _: (u32, _)  = bar();
    }

    (Default::default(), 1i32) // error: can't use inference constraints from `bar()` as the regions differ
}

We also need to have the list of duplicate_entries anyways, because we do resolve infer vars in keys when returning them from canonical queries (or when canonicalizing the query input)

This has a lot of interesting side-effects and additional considerations. The opaque type storage will be more different between typing modes.

In HIR typeck

  • don't need to store the currently getting inferred opaque types in the storage in general, we just keep these goals as ambiguous until we know the "definition site type" for the opaque, and then it's just eq
  • do need to store a map from infer var to the opaque type keys (other way around than what we currently do), to enable <impl Iterator<Item = u32> as Iterator>::Item to normalize to u32 even while the type is still unknown
  • need to store the "defining site type" once it's inferred
  • when normalizing an opaque, try to eagerly compute the "definition site type" if the key is fully known.

In MIR borrowck

  • the goal normalizing opaque types need to succeed right away, MIR borrowck can't delay them until we've inferred the definition site type considering regions
  • this means we should just return every defining use in a list, without even trying to unify them

This should mostly work because outside of recursive calls, we should pretty much always eagerly normalize the opaque to its underlying type and only ever use that one. The opaque storage has only one entry in nearly all cases

I don't know how this will work at the edges and while it seems simpler, it also has some odd behavior.

Have been thinking about this as e.g. agda does not try to unify its constraints for higher kinded infer vars this way, and doing so is quite jank: It means adding a new lifetime to your function can potentially result in really odd errors (it still does as it may change it from defining to non-defining, but that's less often a problem, as returning from the function is pretty much always the defining use you want)

I would love to experiment with this and worry that by waiting until after we stabilized the new solver, doing so is significantly harder.

Pinned by lcnr

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

No labels
No labels

Type

No type

Projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions