Skip to content
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

Rollup of 6 pull requests #91458

Closed
wants to merge 21 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
21 commits
Select commit Hold shift + click to select a range
2c1cf5a
Document how recursion is handled for `ty::Ty`
camelid Nov 3, 2021
7907fa8
Clarify and tidy up explanation of E0038
graydon Nov 30, 2021
bb27b05
Separate `RemoveFalseEdges` from `SimplifyBranches`
ecstatic-morse Nov 30, 2021
d707707
Add "is" methods for projections to a given index
ecstatic-morse Nov 30, 2021
4f7605b
Add `RemoveUninitDrops` MIR pass
ecstatic-morse Nov 30, 2021
ce2959d
Add rationale for `RemoveUnneededDrops`
ecstatic-morse Nov 30, 2021
3e0e8be
Handle `DropAndReplace` in const-checking
ecstatic-morse Nov 30, 2021
58c996c
Move post-elaboration const-checking earlier in the pipeline
ecstatic-morse Dec 1, 2021
9aaca1d
Update MIR opt tests with new name
ecstatic-morse Dec 1, 2021
37fa925
Add regression test for #90770
ecstatic-morse Dec 1, 2021
b11d880
disable tests in Miri that take too long
RalfJung Dec 2, 2021
bd8d7e4
Transform const generics if the function uses rustc_legacy_const_gene…
GuillaumeGomez Oct 16, 2021
5c75a48
Add test for legacy-const-generic arguments
GuillaumeGomez Oct 16, 2021
51875e3
Add additional test from rust issue number 91068
steffahn Dec 2, 2021
b77ed83
Change to check-pass
jackh726 Dec 2, 2021
2ecc657
Rollup merge of #89954 - GuillaumeGomez:legacy-const-generic-doc, r=A…
matthiaskrgr Dec 2, 2021
426687b
Rollup merge of #90538 - camelid:doc-recur-ty, r=estebank
matthiaskrgr Dec 2, 2021
c8bee63
Rollup merge of #91387 - graydon:E0038-clarification, r=wesleywiser
matthiaskrgr Dec 2, 2021
816f2d6
Rollup merge of #91410 - ecstatic-morse:const-precise-live-drops-take…
matthiaskrgr Dec 2, 2021
6054fa8
Rollup merge of #91444 - RalfJung:miri-tests, r=dtolnay
matthiaskrgr Dec 2, 2021
0efed6c
Rollup merge of #91457 - steffahn:additional_test_from_91068, r=jackh726
matthiaskrgr Dec 2, 2021
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
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,8 @@ impl Visitor<'tcx> for CheckLiveDrops<'mir, 'tcx> {
trace!("visit_terminator: terminator={:?} location={:?}", terminator, location);

match &terminator.kind {
mir::TerminatorKind::Drop { place: dropped_place, .. } => {
mir::TerminatorKind::Drop { place: dropped_place, .. }
| mir::TerminatorKind::DropAndReplace { place: dropped_place, .. } => {
let dropped_ty = dropped_place.ty(self.body, self.tcx).ty;
if !NeedsNonConstDrop::in_any_value_of_ty(self.ccx, dropped_ty) {
// Instead of throwing a bug, we just return here. This is because we have to
Expand All @@ -104,11 +105,6 @@ impl Visitor<'tcx> for CheckLiveDrops<'mir, 'tcx> {
}
}

mir::TerminatorKind::DropAndReplace { .. } => span_bug!(
terminator.source_info.span,
"`DropAndReplace` should be removed by drop elaboration",
),

mir::TerminatorKind::Abort
| mir::TerminatorKind::Call { .. }
| mir::TerminatorKind::Assert { .. }
Expand Down
97 changes: 65 additions & 32 deletions compiler/rustc_error_codes/src/error_codes/E0038.md
Original file line number Diff line number Diff line change
@@ -1,34 +1,64 @@
Trait objects like `Box<Trait>` can only be constructed when certain
requirements are satisfied by the trait in question.

Trait objects are a form of dynamic dispatch and use a dynamically sized type
for the inner type. So, for a given trait `Trait`, when `Trait` is treated as a
type, as in `Box<Trait>`, the inner type is 'unsized'. In such cases the boxed
pointer is a 'fat pointer' that contains an extra pointer to a table of methods
(among other things) for dynamic dispatch. This design mandates some
restrictions on the types of traits that are allowed to be used in trait
objects, which are collectively termed as 'object safety' rules.

Attempting to create a trait object for a non object-safe trait will trigger
this error.

There are various rules:

### The trait cannot require `Self: Sized`

When `Trait` is treated as a type, the type does not implement the special
`Sized` trait, because the type does not have a known size at compile time and
can only be accessed behind a pointer. Thus, if we have a trait like the
following:
For any given trait `Trait` there may be a related _type_ called the _trait
object type_ which is typically written as `dyn Trait`. In earlier editions of
Rust, trait object types were written as plain `Trait` (just the name of the
trait, written in type positions) but this was a bit too confusing, so we now
write `dyn Trait`.

Some traits are not allowed to be used as trait object types. The traits that
are allowed to be used as trait object types are called "object-safe" traits.
Attempting to use a trait object type for a trait that is not object-safe will
trigger error E0038.

Two general aspects of trait object types give rise to the restrictions:

1. Trait object types are dynamically sized types (DSTs), and trait objects of
these types can only be accessed through pointers, such as `&dyn Trait` or
`Box<dyn Trait>`. The size of such a pointer is known, but the size of the
`dyn Trait` object pointed-to by the pointer is _opaque_ to code working
with it, and different tait objects with the same trait object type may
have different sizes.

2. The pointer used to access a trait object is paired with an extra pointer
to a "virtual method table" or "vtable", which is used to implement dynamic
dispatch to the object's implementations of the trait's methods. There is a
single such vtable for each trait implementation, but different trait
objects with the same trait object type may point to vtables from different
implementations.

The specific conditions that violate object-safety follow, most of which relate
to missing size information and vtable polymorphism arising from these aspects.

### The trait requires `Self: Sized`

Traits that are declared as `Trait: Sized` or which otherwise inherit a
constraint of `Self:Sized` are not object-safe.

The reasoning behind this is somewhat subtle. It derives from the fact that Rust
requires (and defines) that every trait object type `dyn Trait` automatically
implements `Trait`. Rust does this to simplify error reporting and ease
interoperation between static and dynamic polymorphism. For example, this code
works:

```
trait Foo where Self: Sized {
trait Trait {
}

fn static_foo<T:Trait + ?Sized>(b: &T) {
}

fn dynamic_bar(a: &dyn Trait) {
static_foo(a)
}
```

We cannot create an object of type `Box<Foo>` or `&Foo` since in this case
`Self` would not be `Sized`.
This code works because `dyn Trait`, if it exists, always implements `Trait`.

However as we know, any `dyn Trait` is also unsized, and so it can never
implement a sized trait like `Trait:Sized`. So, rather than allow an exception
to the rule that `dyn Trait` always implements `Trait`, Rust chooses to prohibit
such a `dyn Trait` from existing at all.

Only unsized traits are considered object-safe.

Generally, `Self: Sized` is used to indicate that the trait should not be used
as a trait object. If the trait comes from your own crate, consider removing
Expand Down Expand Up @@ -67,7 +97,7 @@ trait Trait {
fn foo(&self) -> Self;
}

fn call_foo(x: Box<Trait>) {
fn call_foo(x: Box<dyn Trait>) {
let y = x.foo(); // What type is y?
// ...
}
Expand All @@ -76,7 +106,8 @@ fn call_foo(x: Box<Trait>) {
If only some methods aren't object-safe, you can add a `where Self: Sized` bound
on them to mark them as explicitly unavailable to trait objects. The
functionality will still be available to all other implementers, including
`Box<Trait>` which is itself sized (assuming you `impl Trait for Box<Trait>`).
`Box<dyn Trait>` which is itself sized (assuming you `impl Trait for Box<dyn
Trait>`).

```
trait Trait {
Expand Down Expand Up @@ -115,7 +146,9 @@ impl Trait for u8 {
```

At compile time each implementation of `Trait` will produce a table containing
the various methods (and other items) related to the implementation.
the various methods (and other items) related to the implementation, which will
be used as the virtual method table for a `dyn Trait` object derived from that
implementation.

This works fine, but when the method gains generic parameters, we can have a
problem.
Expand Down Expand Up @@ -174,7 +207,7 @@ Now, if we have the following code:
# impl Trait for u8 { fn foo<T>(&self, on: T) {} }
# impl Trait for bool { fn foo<T>(&self, on: T) {} }
# // etc.
fn call_foo(thing: Box<Trait>) {
fn call_foo(thing: Box<dyn Trait>) {
thing.foo(true); // this could be any one of the 8 types above
thing.foo(1);
thing.foo("hello");
Expand All @@ -200,7 +233,7 @@ trait Trait {
```

If this is not an option, consider replacing the type parameter with another
trait object (e.g., if `T: OtherTrait`, use `on: Box<OtherTrait>`). If the
trait object (e.g., if `T: OtherTrait`, use `on: Box<dyn OtherTrait>`). If the
number of types you intend to feed to this method is limited, consider manually
listing out the methods of different types.

Expand All @@ -226,7 +259,7 @@ trait Foo {
}
```

### The trait cannot contain associated constants
### Trait contains associated constants

Just like static functions, associated constants aren't stored on the method
table. If the trait or any subtrait contain an associated constant, they cannot
Expand All @@ -248,7 +281,7 @@ trait Foo {
}
```

### The trait cannot use `Self` as a type parameter in the supertrait listing
### Trait uses `Self` as a type parameter in the supertrait listing

This is similar to the second sub-error, but subtler. It happens in situations
like the following:
Expand Down
10 changes: 10 additions & 0 deletions compiler/rustc_middle/src/mir/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1803,6 +1803,16 @@ impl<V, T> ProjectionElem<V, T> {
| Self::Downcast(_, _) => false,
}
}

/// Returns `true` if this is a `Downcast` projection with the given `VariantIdx`.
pub fn is_downcast_to(&self, v: VariantIdx) -> bool {
matches!(*self, Self::Downcast(_, x) if x == v)
}

/// Returns `true` if this is a `Field` projection with the given index.
pub fn is_field_to(&self, f: Field) -> bool {
matches!(*self, Self::Field(x, _) if x == f)
}
}

/// Alias for projections as they appear in places, where the base is a place
Expand Down
22 changes: 22 additions & 0 deletions compiler/rustc_middle/src/ty/adt.rs
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,28 @@ bitflags! {
/// Moreover, Rust only allows recursive data types through indirection.
///
/// [adt]: https://en.wikipedia.org/wiki/Algebraic_data_type
///
/// # Recursive types
///
/// It may seem impossible to represent recursive types using [`Ty`],
/// since [`TyKind::Adt`] includes [`AdtDef`], which includes its fields,
/// creating a cycle. However, `AdtDef` does not actually include the *types*
/// of its fields; it includes just their [`DefId`]s.
///
/// For example, the following type:
///
/// ```
/// struct S { x: Box<S> }
/// ```
///
/// is essentially represented with [`Ty`] as the following pseudocode:
///
/// ```
/// struct S { x }
/// ```
///
/// where `x` here represents the `DefId` of `S.x`. Then, the `DefId`
/// can be used with [`TyCtxt::type_of()`] to get the type of the field.
pub struct AdtDef {
/// The `DefId` of the struct, enum or union item.
pub did: DefId,
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_middle/src/ty/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1735,7 +1735,7 @@ impl ReprOptions {

impl<'tcx> FieldDef {
/// Returns the type of this field. The resulting type is not normalized. The `subst` is
/// typically obtained via the second field of `TyKind::AdtDef`.
/// typically obtained via the second field of [`TyKind::Adt`].
pub fn ty(&self, tcx: TyCtxt<'tcx>, subst: SubstsRef<'tcx>) -> Ty<'tcx> {
tcx.type_of(self.did).subst(tcx, subst)
}
Expand Down
24 changes: 19 additions & 5 deletions compiler/rustc_mir_transform/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -60,8 +60,10 @@ mod match_branches;
mod multiple_return_terminators;
mod normalize_array_len;
mod nrvo;
mod remove_false_edges;
mod remove_noop_landing_pads;
mod remove_storage_markers;
mod remove_uninit_drops;
mod remove_unneeded_drops;
mod remove_zsts;
mod required_consts;
Expand All @@ -75,7 +77,7 @@ mod simplify_try;
mod uninhabited_enum_branching;
mod unreachable_prop;

use rustc_const_eval::transform::check_consts;
use rustc_const_eval::transform::check_consts::{self, ConstCx};
use rustc_const_eval::transform::promote_consts;
use rustc_const_eval::transform::validate;
use rustc_mir_dataflow::rustc_peek;
Expand Down Expand Up @@ -444,8 +446,20 @@ fn mir_drops_elaborated_and_const_checked<'tcx>(
let (body, _) = tcx.mir_promoted(def);
let mut body = body.steal();

// IMPORTANT
remove_false_edges::RemoveFalseEdges.run_pass(tcx, &mut body);

// Do a little drop elaboration before const-checking if `const_precise_live_drops` is enabled.
//
// FIXME: Can't use `run_passes` for these, since `run_passes` SILENTLY DOES NOTHING IF THE MIR
// PHASE DOESN'T CHANGE.
if check_consts::post_drop_elaboration::checking_enabled(&ConstCx::new(tcx, &body)) {
simplify::SimplifyCfg::new("remove-false-edges").run_pass(tcx, &mut body);
remove_uninit_drops::RemoveUninitDrops.run_pass(tcx, &mut body);
check_consts::post_drop_elaboration::check_live_drops(tcx, &body);
}

run_post_borrowck_cleanup_passes(tcx, &mut body);
check_consts::post_drop_elaboration::check_live_drops(tcx, &body);
tcx.alloc_steal_mir(body)
}

Expand All @@ -455,7 +469,7 @@ fn run_post_borrowck_cleanup_passes<'tcx>(tcx: TyCtxt<'tcx>, body: &mut Body<'tc

let post_borrowck_cleanup: &[&dyn MirPass<'tcx>] = &[
// Remove all things only needed by analysis
&simplify_branches::SimplifyBranches::new("initial"),
&simplify_branches::SimplifyConstCondition::new("initial"),
&remove_noop_landing_pads::RemoveNoopLandingPads,
&cleanup_post_borrowck::CleanupNonCodegenStatements,
&simplify::SimplifyCfg::new("early-opt"),
Expand Down Expand Up @@ -514,13 +528,13 @@ fn run_optimization_passes<'tcx>(tcx: TyCtxt<'tcx>, body: &mut Body<'tcx>) {
&instcombine::InstCombine,
&separate_const_switch::SeparateConstSwitch,
&const_prop::ConstProp,
&simplify_branches::SimplifyBranches::new("after-const-prop"),
&simplify_branches::SimplifyConstCondition::new("after-const-prop"),
&early_otherwise_branch::EarlyOtherwiseBranch,
&simplify_comparison_integral::SimplifyComparisonIntegral,
&simplify_try::SimplifyArmIdentity,
&simplify_try::SimplifyBranchSame,
&dest_prop::DestinationPropagation,
&simplify_branches::SimplifyBranches::new("final"),
&simplify_branches::SimplifyConstCondition::new("final"),
&remove_noop_landing_pads::RemoveNoopLandingPads,
&simplify::SimplifyCfg::new("final"),
&nrvo::RenameReturnPlace,
Expand Down
29 changes: 29 additions & 0 deletions compiler/rustc_mir_transform/src/remove_false_edges.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
use rustc_middle::mir::{Body, TerminatorKind};
use rustc_middle::ty::TyCtxt;

use crate::MirPass;

/// Removes `FalseEdge` and `FalseUnwind` terminators from the MIR.
///
/// These are only needed for borrow checking, and can be removed afterwards.
///
/// FIXME: This should probably have its own MIR phase.
pub struct RemoveFalseEdges;

impl<'tcx> MirPass<'tcx> for RemoveFalseEdges {
fn run_pass(&self, _: TyCtxt<'tcx>, body: &mut Body<'tcx>) {
for block in body.basic_blocks_mut() {
let terminator = block.terminator_mut();
terminator.kind = match terminator.kind {
TerminatorKind::FalseEdge { real_target, .. } => {
TerminatorKind::Goto { target: real_target }
}
TerminatorKind::FalseUnwind { real_target, .. } => {
TerminatorKind::Goto { target: real_target }
}

_ => continue,
}
}
}
}
Loading