Skip to content

Commit b128140

Browse files
rust-bors[bot]rust-bors[bot]
authored
Auto merge of #145807 - zachs18:only-consider-auto-traits-empty, r=<try>
Only consider auto traits empty for the purposes of omitting vptrs from subtrait vtables.
2 parents f6d2341 + bae9ed3 commit b128140

File tree

7 files changed

+194
-12
lines changed

7 files changed

+194
-12
lines changed

‎compiler/rustc_trait_selection/src/traits/vtable.rs

Lines changed: 14 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -153,16 +153,26 @@ fn prepare_vtable_segments_inner<'tcx, T>(
153153

154154
// emit innermost item, move to next sibling and stop there if possible, otherwise jump to outer level.
155155
while let Some((inner_most_trait_ref, emit_vptr, mut siblings)) = stack.pop() {
156-
let has_entries = has_own_existential_vtable_entries(tcx, inner_most_trait_ref.def_id);
157-
156+
let is_non_auto = !tcx.trait_is_auto(inner_most_trait_ref.def_id);
157+
158+
// We don't need to emit a vptr for "truly-empty" supertraits, but we *do* need to emit a
159+
// vptr for supertraits that have no methods, but that themselves have supertraits
160+
// with methods (so we can't just use `own_existential_vtable_entries`, as that
161+
// does not account for vtable entries coming from supertraits).
162+
// As an overapproximation, we treat all non-auto traits as though they are not empty.
163+
// FIXME: don't emit vptr for empty supertraits (whose vtables would only have DSA),
164+
// (even if they aren't auto traits), but in a way that doesn't
165+
// regress #131813 or #145752. Note that this would require considering their
166+
// supertraits, not just their own entries.
158167
segment_visitor(VtblSegment::TraitOwnEntries {
159168
trait_ref: inner_most_trait_ref,
160-
emit_vptr: emit_vptr && has_entries && !tcx.sess.opts.unstable_opts.no_trait_vptr,
169+
emit_vptr: emit_vptr && is_non_auto && !tcx.sess.opts.unstable_opts.no_trait_vptr,
161170
})?;
162171

163172
// If we've emitted (fed to `segment_visitor`) a trait that has methods present in the vtable,
164173
// we'll need to emit vptrs from now on.
165-
emit_vptr_on_new_entry |= has_entries;
174+
// FIXME: as above, we overapproximate and assume any non-auto trait could have methods.
175+
emit_vptr_on_new_entry |= is_non_auto;
166176

167177
if let Some(next_inner_most_trait_ref) =
168178
siblings.find(|&sibling| visited.insert(sibling.upcast(tcx)))
@@ -185,10 +195,6 @@ fn maybe_iter<I: Iterator>(i: Option<I>) -> impl Iterator<Item = I::Item> {
185195
i.into_iter().flatten()
186196
}
187197

188-
fn has_own_existential_vtable_entries(tcx: TyCtxt<'_>, trait_def_id: DefId) -> bool {
189-
own_existential_vtable_entries_iter(tcx, trait_def_id).next().is_some()
190-
}
191-
192198
fn own_existential_vtable_entries(tcx: TyCtxt<'_>, trait_def_id: DefId) -> &[DefId] {
193199
tcx.arena.alloc_from_iter(own_existential_vtable_entries_iter(tcx, trait_def_id))
194200
}

‎tests/ui/traits/vtable/empty-supertrait-with-nonempty-supersupertrait.dump.stderr

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
error: vtable entries: [
2+
MetadataDropInPlace,
3+
MetadataSize,
4+
MetadataAlign,
5+
Method(<dyn OneTwo as One>::one - shim(reify)),
6+
Method(<dyn OneTwo as Two>::two - shim(reify)),
7+
TraitVPtr(<dyn OneTwo as Two>),
8+
TraitVPtr(<dyn OneTwo as TwoAgain>),
9+
]
10+
--> $DIR/empty-supertrait-with-nonempty-supersupertrait.rs:40:1
11+
|
12+
LL | type T = dyn OneTwo;
13+
| ^^^^^^
14+
15+
error: aborting due to 1 previous error
16+

‎tests/ui/traits/vtable/empty-supertrait-with-nonempty-supersupertrait.rs

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,49 @@
1+
//@ revisions: run dump
2+
//@[run] run-pass
3+
//@[dump] check-fail
4+
//! Regression test for #145752
5+
//! Ensure that `OneTwo` contains a vptr for `TwoAgain`
6+
#![allow(unused)]
7+
#![cfg_attr(dump, feature(rustc_attrs))]
8+
9+
trait One {
10+
fn one(&self) {
11+
panic!("don't call this");
12+
}
13+
}
14+
impl One for () {}
15+
16+
trait Two {
17+
fn two(&self) {
18+
println!("good");
19+
}
20+
}
21+
impl Two for () {}
22+
23+
trait TwoAgain: Two {}
24+
impl<T: Two> TwoAgain for T {}
25+
26+
trait OneTwo: One + TwoAgain {}
27+
impl<T: One + Two> OneTwo for T {}
28+
29+
fn main() {
30+
(&()).two();
31+
(&() as &dyn OneTwo).two();
32+
(&() as &dyn OneTwo as &dyn Two).two();
33+
34+
// these two used to panic because they called `one` due to #145752
35+
(&() as &dyn OneTwo as &dyn TwoAgain).two();
36+
(&() as &dyn OneTwo as &dyn TwoAgain as &dyn Two).two();
37+
}
38+
39+
#[cfg_attr(dump, rustc_dump_vtable)]
40+
type T = dyn OneTwo;
41+
//[dump]~^ ERROR vtable entries: [
42+
//[dump]~| ERROR MetadataDropInPlace,
43+
//[dump]~| ERROR MetadataSize,
44+
//[dump]~| ERROR MetadataAlign,
45+
//[dump]~| ERROR Method(<dyn OneTwo as One>::one - shim(reify)),
46+
//[dump]~| ERROR Method(<dyn OneTwo as Two>::two - shim(reify)),
47+
//[dump]~| ERROR TraitVPtr(<dyn OneTwo as Two>),
48+
//[dump]~| ERROR TraitVPtr(<dyn OneTwo as TwoAgain>),
49+
//[dump]~| ERROR ]

‎tests/ui/traits/vtable/multiple-auto.rs

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,50 @@
1+
// Related to <https://github.com/rust-lang/rust/issues/113840>
2+
//
3+
// This test makes sure that multiple auto traits can reuse the
4+
// same pointer for upcasting (e.g. `Send`/`Sync`)
5+
6+
#![crate_type = "lib"]
7+
#![feature(rustc_attrs, auto_traits)]
8+
9+
// Markers
10+
auto trait M0 {}
11+
auto trait M1 {}
12+
auto trait M2 {}
13+
14+
// Just a trait with a method
15+
trait T {
16+
fn method(&self) {}
17+
}
18+
19+
trait A: M0 + M1 + M2 + T {}
20+
21+
trait B: M0 + M1 + T + M2 {}
22+
23+
trait C: M0 + T + M1 + M2 {}
24+
25+
trait D: T + M0 + M1 + M2 {}
26+
27+
struct S;
28+
29+
impl M0 for S {}
30+
impl M1 for S {}
31+
impl M2 for S {}
32+
impl T for S {}
33+
34+
#[rustc_dump_vtable]
35+
impl A for S {}
36+
//~^ ERROR vtable entries
37+
38+
#[rustc_dump_vtable]
39+
impl B for S {}
40+
//~^ ERROR vtable entries
41+
42+
#[rustc_dump_vtable]
43+
impl C for S {}
44+
//~^ ERROR vtable entries
45+
46+
#[rustc_dump_vtable]
47+
impl D for S {}
48+
//~^ ERROR vtable entries
49+
50+
fn main() {}

‎tests/ui/traits/vtable/multiple-auto.stderr

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,46 @@
1+
error: vtable entries: [
2+
MetadataDropInPlace,
3+
MetadataSize,
4+
MetadataAlign,
5+
Method(<S as T>::method),
6+
]
7+
--> $DIR/multiple-auto.rs:35:1
8+
|
9+
LL | impl A for S {}
10+
| ^^^^^^^^^^^^
11+
12+
error: vtable entries: [
13+
MetadataDropInPlace,
14+
MetadataSize,
15+
MetadataAlign,
16+
Method(<S as T>::method),
17+
]
18+
--> $DIR/multiple-auto.rs:39:1
19+
|
20+
LL | impl B for S {}
21+
| ^^^^^^^^^^^^
22+
23+
error: vtable entries: [
24+
MetadataDropInPlace,
25+
MetadataSize,
26+
MetadataAlign,
27+
Method(<S as T>::method),
28+
]
29+
--> $DIR/multiple-auto.rs:43:1
30+
|
31+
LL | impl C for S {}
32+
| ^^^^^^^^^^^^
33+
34+
error: vtable entries: [
35+
MetadataDropInPlace,
36+
MetadataSize,
37+
MetadataAlign,
38+
Method(<S as T>::method),
39+
]
40+
--> $DIR/multiple-auto.rs:47:1
41+
|
42+
LL | impl D for S {}
43+
| ^^^^^^^^^^^^
44+
45+
error: aborting due to 4 previous errors
46+

‎tests/ui/traits/vtable/multiple-markers.rs

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,9 @@
22
//
33
// This test makes sure that multiple marker (method-less) traits can reuse the
44
// same pointer for upcasting.
5+
//
6+
// FIXME: This optimization is currently not performed due to unsoundness in the implementation
7+
// (it considered empty traits with non-empty supertraits as marker traits, see #145752).
58

69
#![crate_type = "lib"]
710
#![feature(rustc_attrs)]

‎tests/ui/traits/vtable/multiple-markers.stderr

Lines changed: 16 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2,9 +2,12 @@ error: vtable entries: [
22
MetadataDropInPlace,
33
MetadataSize,
44
MetadataAlign,
5+
TraitVPtr(<S as M1>),
6+
TraitVPtr(<S as M2>),
57
Method(<S as T>::method),
8+
TraitVPtr(<S as T>),
69
]
7-
--> $DIR/multiple-markers.rs:35:1
10+
--> $DIR/multiple-markers.rs:38:1
811
|
912
LL | impl A for S {}
1013
| ^^^^^^^^^^^^
@@ -13,9 +16,12 @@ error: vtable entries: [
1316
MetadataDropInPlace,
1417
MetadataSize,
1518
MetadataAlign,
19+
TraitVPtr(<S as M1>),
1620
Method(<S as T>::method),
21+
TraitVPtr(<S as T>),
22+
TraitVPtr(<S as M2>),
1723
]
18-
--> $DIR/multiple-markers.rs:39:1
24+
--> $DIR/multiple-markers.rs:42:1
1925
|
2026
LL | impl B for S {}
2127
| ^^^^^^^^^^^^
@@ -25,8 +31,11 @@ error: vtable entries: [
2531
MetadataSize,
2632
MetadataAlign,
2733
Method(<S as T>::method),
34+
TraitVPtr(<S as T>),
35+
TraitVPtr(<S as M1>),
36+
TraitVPtr(<S as M2>),
2837
]
29-
--> $DIR/multiple-markers.rs:43:1
38+
--> $DIR/multiple-markers.rs:46:1
3039
|
3140
LL | impl C for S {}
3241
| ^^^^^^^^^^^^
@@ -36,8 +45,11 @@ error: vtable entries: [
3645
MetadataSize,
3746
MetadataAlign,
3847
Method(<S as T>::method),
48+
TraitVPtr(<S as M0>),
49+
TraitVPtr(<S as M1>),
50+
TraitVPtr(<S as M2>),
3951
]
40-
--> $DIR/multiple-markers.rs:47:1
52+
--> $DIR/multiple-markers.rs:50:1
4153
|
4254
LL | impl D for S {}
4355
| ^^^^^^^^^^^^

0 commit comments

Comments
 (0)