Skip to content

Commit

Permalink
Auto merge of rust-lang#122493 - lukas-code:sized-constraint, r=<try>
Browse files Browse the repository at this point in the history
clean up `Sized` checking

This PR cleans up `sized_constraint` and related functions to make them simpler and faster. This should not make more or less code compile, but it can change error output in some rare cases.

## enums and unions are `Sized`, even if they are not WF

The previous code has some special handling for enums, which made them sized if and only if the last field of each variant is sized. For example given this definition (which is not WF)
```rust
enum E<T1: ?Sized, T2: ?Sized, U1: ?Sized, U2: ?Sized> {
    A(T1, T2),
    B(U1, U2),
}
```
the enum was sized if and only if `T2` and `U2` are sized, while `T1` and `T2` were ignored for `Sized` checking. After this PR this enum will always be sized.

Unsized enums are not a thing in Rust and removing this special case allows us to return an `Option<Ty>` from `sized_constraint`, rather than a `List<Ty>`.

Similarly, the old code made an union defined like this
```rust
union Union<T: ?Sized, U: ?Sized> {
    head: T,
    tail: U,
}
```
sized if and only if `U` is sized, completely ignoring `T`. This just makes no sense at all and now this union is always sized.

## apply the "perf hack" to all (non-error) types, instead of just type parameters

This "perf hack" skips evaluating `sized_constraint(adt): Sized` if `sized_constraint(adt): Sized` exactly matches a predicate defined on `adt`, for example:

```rust
// `Foo<T>: Sized` iff `T: Sized`, but we know `T: Sized` from a predicate of `Foo`
struct Foo<T /*: Sized */>(T);
```

Previously this was only applied to type parameters and now it is applied to every type. This means that for example this type is now always sized:

```rust
// Note that this definition is WF, but the type `S<T>` not WF in the global/empty ParamEnv
struct S<T>([T]) where [T]: Sized;
```

I don't anticipate this to affect compile time of any real-world program, but it makes the code a bit nicer and it also makes error messages a bit more consistent if someone does write such a cursed type.

## tuples are sized if the last type is sized

The old solver already has this behavior and this PR also implements it for the new solver and `is_trivially_sized`. This makes it so that tuples work more like a struct defined like this:

```rust
struct TupleN<T1, T2, /* ... */ Tn: ?Sized>(T1, T2, /* ... */ Tn);
```

This might improve the compile time of programs with large tuples a little, but is mostly also a consistency fix.

## `is_trivially_sized` for more types

This function is used post-typeck code (borrowck, const eval, codegen) to skip evaluating `T: Sized` in some cases. It will now return `true` in more cases, most notably `UnsafeCell<T>` and `ManuallyDrop<T>` where `T.is_trivially_sized`.

I'm anticipating that this change will improve compile time for some real world programs.
  • Loading branch information
bors committed Mar 14, 2024
2 parents e69f14b + b5ef7bd commit c9b796a
Show file tree
Hide file tree
Showing 14 changed files with 113 additions and 117 deletions.
10 changes: 5 additions & 5 deletions compiler/rustc_const_eval/src/interpret/eval_context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1008,8 +1008,7 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
) -> InterpResult<'tcx> {
trace!("{:?} is now live", local);

// We avoid `ty.is_trivially_sized` since that (a) cannot assume WF, so it recurses through
// all fields of a tuple, and (b) does something expensive for ADTs.
// We avoid `ty.is_trivially_sized` since that does something expensive for ADTs.
fn is_very_trivially_sized(ty: Ty<'_>) -> bool {
match ty.kind() {
ty::Infer(ty::IntVar(_) | ty::FloatVar(_))
Expand All @@ -1028,11 +1027,12 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
| ty::Closure(..)
| ty::CoroutineClosure(..)
| ty::Never
| ty::Error(_) => true,
| ty::Error(_)
| ty::Dynamic(_, _, ty::DynStar) => true,

ty::Str | ty::Slice(_) | ty::Dynamic(..) | ty::Foreign(..) => false,
ty::Str | ty::Slice(_) | ty::Dynamic(_, _, ty::Dyn) | ty::Foreign(..) => false,

ty::Tuple(tys) => tys.last().iter().all(|ty| is_very_trivially_sized(**ty)),
ty::Tuple(tys) => tys.last().map_or(true, |&ty| is_very_trivially_sized(ty)),

// We don't want to do any queries, so there is not much we can do with ADTs.
ty::Adt(..) => false,
Expand Down
4 changes: 2 additions & 2 deletions compiler/rustc_middle/src/query/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -703,8 +703,8 @@ rustc_queries! {
separate_provide_extern
}

query adt_sized_constraint(key: DefId) -> ty::EarlyBinder<&'tcx ty::List<Ty<'tcx>>> {
desc { |tcx| "computing `Sized` constraints for `{}`", tcx.def_path_str(key) }
query adt_sized_constraint(key: DefId) -> Option<ty::EarlyBinder<Ty<'tcx>>> {
desc { |tcx| "computing `Sized` constraint for `{}`", tcx.def_path_str(key) }
}

query adt_dtorck_constraint(
Expand Down
10 changes: 5 additions & 5 deletions compiler/rustc_middle/src/ty/adt.rs
Original file line number Diff line number Diff line change
Expand Up @@ -590,16 +590,16 @@ impl<'tcx> AdtDef<'tcx> {
tcx.adt_destructor(self.did())
}

/// Returns a list of types such that `Self: Sized` if and only if that
/// type is `Sized`, or `ty::Error` if this type has a recursive layout.
pub fn sized_constraint(self, tcx: TyCtxt<'tcx>) -> ty::EarlyBinder<&'tcx ty::List<Ty<'tcx>>> {
tcx.adt_sized_constraint(self.did())
/// Returns a type such that `Self: Sized` if and only if that type is `Sized`, or `None`
/// if the type is always sized, or `ty::Error` if this type has a recursive layout.
pub fn sized_constraint(self, tcx: TyCtxt<'tcx>) -> Option<ty::EarlyBinder<Ty<'tcx>>> {
if self.is_struct() { tcx.adt_sized_constraint(self.did()) } else { None }
}
}

#[derive(Clone, Copy, Debug)]
#[derive(HashStable)]
pub enum Representability {
Representable,
Infinite,
Infinite(ErrorGuaranteed),
}
2 changes: 1 addition & 1 deletion compiler/rustc_middle/src/ty/inhabitedness/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ pub(crate) fn provide(providers: &mut Providers) {
/// requires calling [`InhabitedPredicate::instantiate`]
fn inhabited_predicate_adt(tcx: TyCtxt<'_>, def_id: DefId) -> InhabitedPredicate<'_> {
if let Some(def_id) = def_id.as_local() {
if matches!(tcx.representability(def_id), ty::Representability::Infinite) {
if matches!(tcx.representability(def_id), ty::Representability::Infinite(_)) {
return InhabitedPredicate::True;
}
}
Expand Down
11 changes: 7 additions & 4 deletions compiler/rustc_middle/src/ty/sty.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2483,13 +2483,16 @@ impl<'tcx> Ty<'tcx> {
| ty::Closure(..)
| ty::CoroutineClosure(..)
| ty::Never
| ty::Error(_) => true,
| ty::Error(_)
| ty::Dynamic(_, _, ty::DynStar) => true,

ty::Str | ty::Slice(_) | ty::Dynamic(..) | ty::Foreign(..) => false,
ty::Str | ty::Slice(_) | ty::Dynamic(_, _, ty::Dyn) | ty::Foreign(..) => false,

ty::Tuple(tys) => tys.iter().all(|ty| ty.is_trivially_sized(tcx)),
ty::Tuple(tys) => tys.last().map_or(true, |ty| ty.is_trivially_sized(tcx)),

ty::Adt(def, _args) => def.sized_constraint(tcx).skip_binder().is_empty(),
ty::Adt(def, args) => def
.sized_constraint(tcx)
.map_or(true, |ty| ty.instantiate(tcx, args).is_trivially_sized(tcx)),

ty::Alias(..) | ty::Param(_) | ty::Placeholder(..) | ty::Bound(..) => false,

Expand Down
8 changes: 4 additions & 4 deletions compiler/rustc_middle/src/values.rs
Original file line number Diff line number Diff line change
Expand Up @@ -106,8 +106,8 @@ impl<'tcx> Value<TyCtxt<'tcx>> for Representability {
representable_ids.insert(def_id);
}
}
recursive_type_error(tcx, item_and_field_ids, &representable_ids);
Representability::Infinite
let guar = recursive_type_error(tcx, item_and_field_ids, &representable_ids);
Representability::Infinite(guar)
}
}

Expand Down Expand Up @@ -268,7 +268,7 @@ pub fn recursive_type_error(
tcx: TyCtxt<'_>,
mut item_and_field_ids: Vec<(LocalDefId, LocalDefId)>,
representable_ids: &FxHashSet<LocalDefId>,
) {
) -> ErrorGuaranteed {
const ITEM_LIMIT: usize = 5;

// Rotate the cycle so that the item with the lowest span is first
Expand Down Expand Up @@ -344,7 +344,7 @@ pub fn recursive_type_error(
suggestion,
Applicability::HasPlaceholders,
)
.emit();
.emit()
}

fn find_item_ty_spans(
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_trait_selection/src/solve/assembly/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,7 @@ pub(super) trait GoalKind<'tcx>:
goal: Goal<'tcx, Self>,
) -> QueryResult<'tcx>;

/// A type is `Copy` or `Clone` if its components are `Sized`.
/// A type is `Sized` if its tail component is `Sized`.
///
/// These components are given by built-in rules from
/// [`structural_traits::instantiate_constituent_tys_for_sized_trait`].
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -152,11 +152,13 @@ pub(in crate::solve) fn instantiate_constituent_tys_for_sized_trait<'tcx>(
bug!("unexpected type `{ty}`")
}

ty::Tuple(tys) => Ok(tys.iter().map(ty::Binder::dummy).collect()),
ty::Tuple(tys) => Ok(tys.last().map_or_else(Vec::new, |&ty| vec![ty::Binder::dummy(ty)])),

ty::Adt(def, args) => {
let sized_crit = def.sized_constraint(ecx.tcx());
Ok(sized_crit.iter_instantiated(ecx.tcx(), args).map(ty::Binder::dummy).collect())
Ok(sized_crit.map_or_else(Vec::new, |ty| {
vec![ty::Binder::dummy(ty.instantiate(ecx.tcx(), args))]
}))
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,14 +20,11 @@ impl<'tcx> super::QueryTypeOp<'tcx> for ProvePredicate<'tcx> {
// such cases.
if let ty::PredicateKind::Clause(ty::ClauseKind::Trait(trait_ref)) =
key.value.predicate.kind().skip_binder()
&& let Some(sized_def_id) = tcx.lang_items().sized_trait()
&& trait_ref.def_id() == sized_def_id
&& trait_ref.self_ty().is_trivially_sized(tcx)
{
if let Some(sized_def_id) = tcx.lang_items().sized_trait() {
if trait_ref.def_id() == sized_def_id {
if trait_ref.self_ty().is_trivially_sized(tcx) {
return Some(());
}
}
}
return Some(());
}

if let ty::PredicateKind::Clause(ty::ClauseKind::WellFormed(arg)) =
Expand Down
8 changes: 3 additions & 5 deletions compiler/rustc_trait_selection/src/traits/select/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2120,11 +2120,9 @@ impl<'tcx> SelectionContext<'_, 'tcx> {
ty::Adt(def, args) => {
let sized_crit = def.sized_constraint(self.tcx());
// (*) binder moved here
Where(
obligation
.predicate
.rebind(sized_crit.iter_instantiated(self.tcx(), args).collect()),
)
Where(obligation.predicate.rebind(
sized_crit.map_or_else(Vec::new, |ty| vec![ty.instantiate(self.tcx(), args)]),
))
}

ty::Alias(..) | ty::Param(_) | ty::Placeholder(..) => None,
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_ty_utils/src/representability.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ pub(crate) fn provide(providers: &mut Providers) {
macro_rules! rtry {
($e:expr) => {
match $e {
e @ Representability::Infinite => return e,
e @ Representability::Infinite(_) => return e,
Representability::Representable => {}
}
};
Expand Down
141 changes: 72 additions & 69 deletions compiler/rustc_ty_utils/src/ty.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,76 +3,56 @@ use rustc_hir as hir;
use rustc_hir::def::DefKind;
use rustc_index::bit_set::BitSet;
use rustc_middle::query::Providers;
use rustc_middle::ty::{self, EarlyBinder, Ty, TyCtxt, TypeVisitor};
use rustc_middle::ty::{self, EarlyBinder, Ty, TyCtxt, TypeVisitableExt, TypeVisitor};
use rustc_middle::ty::{ToPredicate, TypeSuperVisitable, TypeVisitable};
use rustc_span::def_id::{DefId, LocalDefId, CRATE_DEF_ID};
use rustc_span::DUMMY_SP;
use rustc_trait_selection::traits;

fn sized_constraint_for_ty<'tcx>(
tcx: TyCtxt<'tcx>,
adtdef: ty::AdtDef<'tcx>,
ty: Ty<'tcx>,
) -> Vec<Ty<'tcx>> {
#[instrument(level = "debug", skip(tcx), ret)]
fn sized_constraint_for_ty<'tcx>(tcx: TyCtxt<'tcx>, ty: Ty<'tcx>) -> Option<Ty<'tcx>> {
use rustc_type_ir::TyKind::*;

let result = match ty.kind() {
Bool | Char | Int(..) | Uint(..) | Float(..) | RawPtr(..) | Ref(..) | FnDef(..)
| FnPtr(_) | Array(..) | Closure(..) | CoroutineClosure(..) | Coroutine(..) | Never => {
vec![]
}

Str | Dynamic(..) | Slice(_) | Foreign(..) | Error(_) | CoroutineWitness(..) => {
// these are never sized - return the target type
vec![ty]
}

Tuple(tys) => match tys.last() {
None => vec![],
Some(&ty) => sized_constraint_for_ty(tcx, adtdef, ty),
},

match ty.kind() {
// these are always sized
Bool
| Char
| Int(..)
| Uint(..)
| Float(..)
| RawPtr(..)
| Ref(..)
| FnDef(..)
| FnPtr(..)
| Array(..)
| Closure(..)
| CoroutineClosure(..)
| Coroutine(..)
| CoroutineWitness(..)
| Never
| Dynamic(_, _, ty::DynStar) => None,

// these are never sized
Str | Slice(..) | Dynamic(_, _, ty::Dyn) | Foreign(..) => Some(ty),

Tuple(tys) => tys.last().and_then(|&ty| sized_constraint_for_ty(tcx, ty)),

// recursive case
Adt(adt, args) => {
// recursive case
let adt_tys = adt.sized_constraint(tcx);
debug!("sized_constraint_for_ty({:?}) intermediate = {:?}", ty, adt_tys);
adt_tys
.iter_instantiated(tcx, args)
.flat_map(|ty| sized_constraint_for_ty(tcx, adtdef, ty))
.collect()
}

Alias(..) => {
// must calculate explicitly.
// FIXME: consider special-casing always-Sized projections
vec![ty]
let intermediate = adt.sized_constraint(tcx);
intermediate.and_then(|intermediate| {
let ty = intermediate.instantiate(tcx, args);
sized_constraint_for_ty(tcx, ty)
})
}

Param(..) => {
// perf hack: if there is a `T: Sized` bound, then
// we know that `T` is Sized and do not need to check
// it on the impl.

let Some(sized_trait_def_id) = tcx.lang_items().sized_trait() else { return vec![ty] };
let predicates = tcx.predicates_of(adtdef.did()).predicates;
if predicates.iter().any(|(p, _)| {
p.as_trait_clause().is_some_and(|trait_pred| {
trait_pred.def_id() == sized_trait_def_id
&& trait_pred.self_ty().skip_binder() == ty
})
}) {
vec![]
} else {
vec![ty]
}
}
// these can be sized or unsized
Param(..) | Alias(..) | Error(_) => Some(ty),

Placeholder(..) | Bound(..) | Infer(..) => {
bug!("unexpected type `{:?}` in sized_constraint_for_ty", ty)
bug!("unexpected type `{ty:?}` in sized_constraint_for_ty")
}
};
debug!("sized_constraint_for_ty({:?}) = {:?}", ty, result);
result
}
}

fn defaultness(tcx: TyCtxt<'_>, def_id: LocalDefId) -> hir::Defaultness {
Expand All @@ -90,29 +70,52 @@ fn defaultness(tcx: TyCtxt<'_>, def_id: LocalDefId) -> hir::Defaultness {
///
/// In fact, there are only a few options for the types in the constraint:
/// - an obviously-unsized type
/// - a type parameter or projection whose Sizedness can't be known
/// - a tuple of type parameters or projections, if there are multiple
/// such.
/// - a type parameter or projection whose sizedness can't be known
/// - an Error, if a type is infinitely sized
#[instrument(level = "debug", skip(tcx), ret)]
fn adt_sized_constraint<'tcx>(
tcx: TyCtxt<'tcx>,
def_id: DefId,
) -> ty::EarlyBinder<&'tcx ty::List<Ty<'tcx>>> {
) -> Option<ty::EarlyBinder<Ty<'tcx>>> {
if let Some(def_id) = def_id.as_local() {
if matches!(tcx.representability(def_id), ty::Representability::Infinite) {
return ty::EarlyBinder::bind(tcx.mk_type_list(&[Ty::new_misc_error(tcx)]));
if let ty::Representability::Infinite(guar) = tcx.representability(def_id) {
return Some(ty::EarlyBinder::bind(Ty::new_error(tcx, guar)));
}
}
let def = tcx.adt_def(def_id);

let result =
tcx.mk_type_list_from_iter(def.variants().iter().filter_map(|v| v.tail_opt()).flat_map(
|f| sized_constraint_for_ty(tcx, def, tcx.type_of(f.did).instantiate_identity()),
));
if !def.is_struct() {
bug!("`adt_sized_constraint` called on non-struct type: {def:?}");
}

let tail_def = def.non_enum_variant().tail_opt()?;
let tail_ty = tcx.type_of(tail_def.did).instantiate_identity();

let result = sized_constraint_for_ty(tcx, tail_ty);

let result = result.filter(|&ty| {
// perf hack: if there is a `ty: Sized` bound, then we know that
// the type is sized and do not need to check it on the impl.

if ty.references_error() {
return true;
}

let Some(sized_trait_def_id) = tcx.lang_items().sized_trait() else {
return true;
};

let predicates = tcx.predicates_of(def.did()).predicates;

debug!("adt_sized_constraint: {:?} => {:?}", def, result);
!predicates.iter().any(|(p, _)| {
p.as_trait_clause().is_some_and(|trait_pred| {
trait_pred.def_id() == sized_trait_def_id
&& trait_pred.self_ty().skip_binder() == ty
})
})
});

ty::EarlyBinder::bind(result)
result.map(ty::EarlyBinder::bind)
}

/// See `ParamEnv` struct definition for details.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,5 +29,5 @@ where

fn main() {
let mut list = RcNode::<i32>::new();
//~^ ERROR the size for values of type `Node<i32, RcFamily>` cannot be known at compilation time
//~^ ERROR trait bounds were not satisfied
}
Loading

0 comments on commit c9b796a

Please sign in to comment.