Skip to content

Implement selection for Unsize for better coercion behavior #113353

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Jul 13, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion compiler/rustc_hir_typeck/src/coercion.rs
Original file line number Diff line number Diff line change
Expand Up @@ -636,6 +636,7 @@ impl<'f, 'tcx> Coerce<'f, 'tcx> {
Some(ty::PredicateKind::Clause(ty::ClauseKind::Trait(trait_pred)))
if traits.contains(&trait_pred.def_id()) =>
{
let trait_pred = self.resolve_vars_if_possible(trait_pred);
if unsize_did == trait_pred.def_id() {
let self_ty = trait_pred.self_ty();
let unsize_ty = trait_pred.trait_ref.substs[1].expect_ty();
Expand All @@ -662,7 +663,6 @@ impl<'f, 'tcx> Coerce<'f, 'tcx> {
// Uncertain or unimplemented.
Ok(None) => {
if trait_pred.def_id() == unsize_did {
let trait_pred = self.resolve_vars_if_possible(trait_pred);
let self_ty = trait_pred.self_ty();
let unsize_ty = trait_pred.trait_ref.substs[1].expect_ty();
debug!("coerce_unsized: ambiguous unsize case for {:?}", trait_pred);
Expand Down
145 changes: 144 additions & 1 deletion compiler/rustc_trait_selection/src/solve/eval_ctxt/select.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
use std::ops::ControlFlow;

use rustc_hir as hir;
use rustc_hir::def_id::DefId;
use rustc_infer::infer::{DefineOpaqueTypes, InferCtxt, InferOk};
use rustc_infer::traits::util::supertraits;
Expand All @@ -11,7 +12,7 @@ use rustc_middle::traits::{
ImplSource, ImplSourceObjectData, ImplSourceTraitUpcastingData, ImplSourceUserDefinedData,
ObligationCause, SelectionError,
};
use rustc_middle::ty::{self, TyCtxt};
use rustc_middle::ty::{self, Ty, TyCtxt};
use rustc_span::DUMMY_SP;

use crate::solve::assembly::{BuiltinImplSource, Candidate, CandidateSource};
Expand Down Expand Up @@ -113,6 +114,12 @@ impl<'tcx> InferCtxtSelectExt<'tcx> for InferCtxt<'tcx> {
),
) => rematch_object(self, goal, nested_obligations),

(Certainty::Maybe(_), CandidateSource::BuiltinImpl(BuiltinImplSource::Misc))
if self.tcx.lang_items().unsize_trait() == Some(goal.predicate.def_id()) =>
{
rematch_unsize(self, goal, nested_obligations)
}

// Technically some builtin impls have nested obligations, but if
// `Certainty::Yes`, then they should've all been verified and don't
// need re-checking.
Expand Down Expand Up @@ -232,6 +239,9 @@ fn rematch_object<'tcx>(
{
assert_eq!(source_kind, ty::Dyn, "cannot upcast dyn*");
if let ty::Dynamic(data, _, ty::Dyn) = goal.predicate.trait_ref.substs.type_at(1).kind() {
// FIXME: We also need to ensure that the source lifetime outlives the
// target lifetime. This doesn't matter for codegen, though, and only
// *really* matters if the goal's certainty is ambiguous.
(true, data.principal().unwrap().with_self_ty(infcx.tcx, self_ty))
} else {
bug!()
Expand Down Expand Up @@ -305,3 +315,136 @@ fn rematch_object<'tcx>(
ImplSource::Object(ImplSourceObjectData { vtable_base, nested })
}))
}

/// The `Unsize` trait is particularly important to coercion, so we try rematch it.
/// NOTE: This must stay in sync with `consider_builtin_unsize_candidate` in trait
/// goal assembly in the solver, both for soundness and in order to avoid ICEs.
fn rematch_unsize<'tcx>(
infcx: &InferCtxt<'tcx>,
goal: Goal<'tcx, ty::TraitPredicate<'tcx>>,
mut nested: Vec<PredicateObligation<'tcx>>,
) -> SelectionResult<'tcx, Selection<'tcx>> {
let tcx = infcx.tcx;
let a_ty = goal.predicate.self_ty();
let b_ty = goal.predicate.trait_ref.substs.type_at(1);

match (a_ty.kind(), b_ty.kind()) {
(_, &ty::Dynamic(data, region, ty::Dyn)) => {
// Check that the type implements all of the predicates of the def-id.
// (i.e. the principal, all of the associated types match, and any auto traits)
nested.extend(data.iter().map(|pred| {
Obligation::new(
infcx.tcx,
ObligationCause::dummy(),
goal.param_env,
pred.with_self_ty(tcx, a_ty),
)
}));
// The type must be Sized to be unsized.
let sized_def_id = tcx.require_lang_item(hir::LangItem::Sized, None);
nested.push(Obligation::new(
infcx.tcx,
ObligationCause::dummy(),
goal.param_env,
ty::TraitRef::new(tcx, sized_def_id, [a_ty]),
));
// The type must outlive the lifetime of the `dyn` we're unsizing into.
nested.push(Obligation::new(
infcx.tcx,
ObligationCause::dummy(),
goal.param_env,
ty::Binder::dummy(ty::OutlivesPredicate(a_ty, region)),
));
}
// `[T; n]` -> `[T]` unsizing
(&ty::Array(a_elem_ty, ..), &ty::Slice(b_elem_ty)) => {
nested.extend(
infcx
.at(&ObligationCause::dummy(), goal.param_env)
.eq(DefineOpaqueTypes::No, a_elem_ty, b_elem_ty)
.expect("expected rematch to succeed")
.into_obligations(),
);
}
// Struct unsizing `Struct<T>` -> `Struct<U>` where `T: Unsize<U>`
(&ty::Adt(a_def, a_substs), &ty::Adt(b_def, b_substs))
if a_def.is_struct() && a_def.did() == b_def.did() =>
{
let unsizing_params = tcx.unsizing_params_for_adt(a_def.did());
// We must be unsizing some type parameters. This also implies
// that the struct has a tail field.
if unsizing_params.is_empty() {
bug!("expected rematch to succeed")
}

let tail_field = a_def
.non_enum_variant()
.fields
.raw
.last()
.expect("expected unsized ADT to have a tail field");
let tail_field_ty = tcx.type_of(tail_field.did);

let a_tail_ty = tail_field_ty.subst(tcx, a_substs);
let b_tail_ty = tail_field_ty.subst(tcx, b_substs);

// Substitute just the unsizing params from B into A. The type after
// this substitution must be equal to B. This is so we don't unsize
// unrelated type parameters.
let new_a_substs =
tcx.mk_substs_from_iter(a_substs.iter().enumerate().map(|(i, a)| {
if unsizing_params.contains(i as u32) { b_substs[i] } else { a }
}));
let unsized_a_ty = Ty::new_adt(tcx, a_def, new_a_substs);

nested.extend(
infcx
.at(&ObligationCause::dummy(), goal.param_env)
.eq(DefineOpaqueTypes::No, unsized_a_ty, b_ty)
.expect("expected rematch to succeed")
.into_obligations(),
);

// Finally, we require that `TailA: Unsize<TailB>` for the tail field
// types.
nested.push(Obligation::new(
tcx,
ObligationCause::dummy(),
goal.param_env,
ty::TraitRef::new(tcx, goal.predicate.def_id(), [a_tail_ty, b_tail_ty]),
));
}
// Tuple unsizing `(.., T)` -> `(.., U)` where `T: Unsize<U>`
(&ty::Tuple(a_tys), &ty::Tuple(b_tys))
if a_tys.len() == b_tys.len() && !a_tys.is_empty() =>
{
let (a_last_ty, a_rest_tys) = a_tys.split_last().unwrap();
let b_last_ty = b_tys.last().unwrap();

// Substitute just the tail field of B., and require that they're equal.
let unsized_a_ty =
Ty::new_tup_from_iter(tcx, a_rest_tys.iter().chain([b_last_ty]).copied());
nested.extend(
infcx
.at(&ObligationCause::dummy(), goal.param_env)
.eq(DefineOpaqueTypes::No, unsized_a_ty, b_ty)
.expect("expected rematch to succeed")
.into_obligations(),
);

// Similar to ADTs, require that the rest of the fields are equal.
nested.push(Obligation::new(
tcx,
ObligationCause::dummy(),
goal.param_env,
ty::TraitRef::new(tcx, goal.predicate.def_id(), [*a_last_ty, *b_last_ty]),
));
}
// FIXME: We *could* ICE here if either:
// 1. the certainty is `Certainty::Yes`,
// 2. we're in codegen (which should mean `Certainty::Yes`).
_ => return Ok(None),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please ICE here to detect mismatches

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We cannot ICE here because this goal is selected during coercion, so we legitimately may see, e.g. [T; N]: Unsize<_> during selection.

One more point towards having a typeck and codegen mode for selection...

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Left a fixme anyways.

}

Ok(Some(ImplSource::Builtin(nested)))
}
2 changes: 1 addition & 1 deletion src/tools/tidy/src/ui_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ use std::path::{Path, PathBuf};

const ENTRY_LIMIT: usize = 900;
// FIXME: The following limits should be reduced eventually.
const ISSUES_ENTRY_LIMIT: usize = 1896;
const ISSUES_ENTRY_LIMIT: usize = 1894;
const ROOT_ENTRY_LIMIT: usize = 870;

const EXPECTED_TEST_FILE_EXTENSIONS: &[&str] = &[
Expand Down
2 changes: 2 additions & 0 deletions tests/ui/traits/issue-24010.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
// run-pass
// revisions: classic next
//[next] compile-flags: -Ztrait-solver=next

trait Foo: Fn(i32) -> i32 + Send {}

Expand Down
13 changes: 13 additions & 0 deletions tests/ui/traits/new-solver/unsize-although-ambiguous.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
// check-pass
// compile-flags: -Ztrait-solver=next

use std::fmt::Display;

fn box_dyn_display(_: Box<dyn Display>) {}

fn main() {
// During coercion, we don't necessarily know whether `{integer}` implements
// `Display`. Before, that would cause us to bail out in the coercion loop when
// checking `{integer}: Unsize<dyn Display>`.
box_dyn_display(Box::new(1));
}
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
error[E0658]: cannot cast `dyn Fn()` to `dyn FnMut()`, trait upcasting coercion is experimental
--> $DIR/issue-11515.rs:9:38
--> $DIR/issue-11515.rs:10:38
|
LL | let test = Box::new(Test { func: closure });
| ^^^^^^^
Expand Down
13 changes: 13 additions & 0 deletions tests/ui/traits/trait-upcasting/issue-11515.next.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
error[E0658]: cannot cast `dyn Fn()` to `dyn FnMut()`, trait upcasting coercion is experimental
--> $DIR/issue-11515.rs:10:38
|
LL | let test = Box::new(Test { func: closure });
| ^^^^^^^
|
= note: see issue #65991 <https://github.com/rust-lang/rust/issues/65991> for more information
= help: add `#![feature(trait_upcasting)]` to the crate attributes to enable
= note: required when coercing `Box<(dyn Fn() + 'static)>` into `Box<(dyn FnMut() + 'static)>`

error: aborting due to previous error

For more information about this error, try `rustc --explain E0658`.
Original file line number Diff line number Diff line change
@@ -1,9 +1,10 @@
// revisions: current next
//[next] compile-flags: -Ztrait-solver=next

struct Test {
func: Box<dyn FnMut() + 'static>,
}



fn main() {
let closure: Box<dyn Fn() + 'static> = Box::new(|| ());
let test = Box::new(Test { func: closure }); //~ ERROR trait upcasting coercion is experimental [E0658]
Expand Down