Skip to content

Commit

Permalink
Auto merge of #123864 - oli-obk:define_opaque_types3, r=<try>
Browse files Browse the repository at this point in the history
Remove a HACK by instead inferring opaque types during expected/formal type checking

I was wondering why I couldn't come up with a test that hits the code path of the argument check checking the types we inferred from the return type... Turns out we reject those attempts early during fudging.

I have absolutely no information for you as to what kind of type inference changes this may incur, but I think we should just land this out of two reasons:

* had I found the other place to use opaque type inference on before I added the hack, we'd be using that today and this PR would never have happened
* if it is possible to hit this path, it requires some god awful recursive RPIT logic that I doubt anyone would have written without actively trying to write obscure code

r? `@ghost`
  • Loading branch information
bors committed Apr 12, 2024
2 parents bd71213 + 24653a5 commit c0f799a
Show file tree
Hide file tree
Showing 2 changed files with 6 additions and 36 deletions.
26 changes: 0 additions & 26 deletions compiler/rustc_hir_typeck/src/fn_ctxt/_impl.rs
Original file line number Diff line number Diff line change
Expand Up @@ -711,32 +711,6 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
let formal_ret = self.resolve_vars_with_obligations(formal_ret);
let ret_ty = expected_ret.only_has_type(self)?;

// HACK(oli-obk): This is a hack to keep RPIT and TAIT in sync wrt their behaviour.
// Without it, the inference
// variable will get instantiated with the opaque type. The inference variable often
// has various helpful obligations registered for it that help closures figure out their
// signature. If we infer the inference var to the opaque type, the closure won't be able
// to find those obligations anymore, and it can't necessarily find them from the opaque
// type itself. We could be more powerful with inference if we *combined* the obligations
// so that we got both the obligations from the opaque type and the ones from the inference
// variable. That will accept more code than we do right now, so we need to carefully consider
// the implications.
// Note: this check is pessimistic, as the inference type could be matched with something other
// than the opaque type, but then we need a new `TypeRelation` just for this specific case and
// can't re-use `sup` below.
// See tests/ui/impl-trait/hidden-type-is-opaque.rs and
// tests/ui/impl-trait/hidden-type-is-opaque-2.rs for examples that hit this path.
if formal_ret.has_infer_types() {
for ty in ret_ty.walk() {
if let ty::GenericArgKind::Type(ty) = ty.unpack()
&& let ty::Alias(ty::Opaque, ty::AliasTy { def_id, .. }) = *ty.kind()
&& self.can_define_opaque_ty(def_id)
{
return None;
}
}
}

let expect_args = self
.fudge_inference_if_ok(|| {
let ocx = ObligationCtxt::new(self);
Expand Down
16 changes: 6 additions & 10 deletions compiler/rustc_hir_typeck/src/fn_ctxt/checks.rs
Original file line number Diff line number Diff line change
Expand Up @@ -297,22 +297,18 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
// 3. Check if the formal type is a supertype of the checked one
// and register any such obligations for future type checks
let supertype_error = self.at(&self.misc(provided_arg.span), self.param_env).sup(
DefineOpaqueTypes::No,
DefineOpaqueTypes::Yes,
formal_input_ty,
coerced_ty,
);
let subtyping_error = match supertype_error {

// If neither check failed, the types are compatible
match supertype_error {
Ok(InferOk { obligations, value: () }) => {
self.register_predicates(obligations);
None
Compatibility::Compatible
}
Err(err) => Some(err),
};

// If neither check failed, the types are compatible
match subtyping_error {
None => Compatibility::Compatible,
Some(_) => Compatibility::Incompatible(subtyping_error),
Err(err) => Compatibility::Incompatible(Some(err)),
}
};

Expand Down

0 comments on commit c0f799a

Please sign in to comment.