Skip to content

Commit 7221383

Browse files
Dylan-DPCDylan-DPC
authored
Rollup merge of #107082 - dtolnay:autotraits, r=lcnr
Autotrait bounds on dyn-safe trait methods This PR is a successor to #106604 implementing the approach encouraged by #106604 (comment). **I propose making it legal to use autotraits as trait bounds on the `Self` type of trait methods in a trait object.** #51443 (comment) justifies why this use case is particularly important in the context of the async-trait crate. ```rust #![feature(auto_traits)] #![deny(where_clauses_object_safety)] auto trait AutoTrait {} trait MyTrait { fn f(&self) where Self: AutoTrait; } fn main() { let _: &dyn MyTrait; } ``` Previously this would fail with: ```console error: the trait `MyTrait` cannot be made into an object --> src/main.rs:7:8 | 7 | fn f(&self) where Self: AutoTrait; | ^ | = warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release! = note: for more information, see issue #51443 <#51443> note: for a trait to be "object safe" it needs to allow building a vtable to allow the call to be resolvable dynamically; for more information visit <https://doc.rust-lang.org/reference/items/traits.html#object-safety> --> src/main.rs:7:8 | 6 | trait MyTrait { | ------- this trait cannot be made into an object... 7 | fn f(&self) where Self: AutoTrait; | ^ ...because method `f` references the `Self` type in its `where` clause = help: consider moving `f` to another trait ``` In order for this to be sound without hitting #50781, **I further propose that we disallow handwritten autotrait impls that apply to trait objects.** Both of the following were previously allowed (_on nightly_) and no longer allowed in my proposal: ```rust auto trait AutoTrait {} trait MyTrait {} impl AutoTrait for dyn MyTrait {} // NOT ALLOWED impl<T: ?Sized> AutoTrait for T {} // NOT ALLOWED ``` (`impl<T> AutoTrait for T {}` remains allowed.) After this change, traits with a default impl are implemented for a trait object **if and only if** the autotrait is one of the trait object's trait bounds (or a supertrait of a bound). In other words `dyn Trait + AutoTrait` always implements AutoTrait while `dyn Trait` never implements AutoTrait. Fixes dtolnay/async-trait#228. r? `@lcnr`
2 parents a94b9fd + 4501d3a commit 7221383

9 files changed

+377
-78
lines changed

‎compiler/rustc_data_structures/src/sync.rs

+2-2
Original file line numberDiff line numberDiff line change
@@ -31,8 +31,8 @@ cfg_if! {
3131
pub auto trait Send {}
3232
pub auto trait Sync {}
3333

34-
impl<T: ?Sized> Send for T {}
35-
impl<T: ?Sized> Sync for T {}
34+
impl<T> Send for T {}
35+
impl<T> Sync for T {}
3636

3737
#[macro_export]
3838
macro_rules! rustc_erase_owner {

‎compiler/rustc_hir_analysis/src/coherence/orphan.rs

+183-40
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ use rustc_hir as hir;
88
use rustc_middle::ty::subst::InternalSubsts;
99
use rustc_middle::ty::util::IgnoreRegions;
1010
use rustc_middle::ty::{
11-
self, ImplPolarity, Ty, TyCtxt, TypeSuperVisitable, TypeVisitable, TypeVisitor,
11+
self, AliasKind, ImplPolarity, Ty, TyCtxt, TypeSuperVisitable, TypeVisitable, TypeVisitor,
1212
};
1313
use rustc_session::lint;
1414
use rustc_span::def_id::{DefId, LocalDefId};
@@ -86,7 +86,7 @@ fn do_orphan_check_impl<'tcx>(
8686
// struct B { }
8787
// impl Foo for A { }
8888
// impl Foo for B { }
89-
// impl !Send for (A, B) { }
89+
// impl !Foo for (A, B) { }
9090
// ```
9191
//
9292
// This final impl is legal according to the orphan
@@ -99,50 +99,193 @@ fn do_orphan_check_impl<'tcx>(
9999
tcx.trait_is_auto(trait_def_id)
100100
);
101101

102-
if tcx.trait_is_auto(trait_def_id) && !trait_def_id.is_local() {
102+
if tcx.trait_is_auto(trait_def_id) {
103103
let self_ty = trait_ref.self_ty();
104-
let opt_self_def_id = match *self_ty.kind() {
105-
ty::Adt(self_def, _) => Some(self_def.did()),
106-
ty::Foreign(did) => Some(did),
107-
_ => None,
108-
};
109104

110-
let msg = match opt_self_def_id {
111-
// We only want to permit nominal types, but not *all* nominal types.
112-
// They must be local to the current crate, so that people
113-
// can't do `unsafe impl Send for Rc<SomethingLocal>` or
114-
// `impl !Send for Box<SomethingLocalAndSend>`.
115-
Some(self_def_id) => {
116-
if self_def_id.is_local() {
117-
None
105+
// If the impl is in the same crate as the auto-trait, almost anything
106+
// goes.
107+
//
108+
// impl MyAuto for Rc<Something> {} // okay
109+
// impl<T> !MyAuto for *const T {} // okay
110+
// impl<T> MyAuto for T {} // okay
111+
//
112+
// But there is one important exception: implementing for a trait object
113+
// is not allowed.
114+
//
115+
// impl MyAuto for dyn Trait {} // NOT OKAY
116+
// impl<T: ?Sized> MyAuto for T {} // NOT OKAY
117+
//
118+
// With this restriction, it's guaranteed that an auto-trait is
119+
// implemented for a trait object if and only if the auto-trait is one
120+
// of the trait object's trait bounds (or a supertrait of a bound). In
121+
// other words `dyn Trait + AutoTrait` always implements AutoTrait,
122+
// while `dyn Trait` never implements AutoTrait.
123+
//
124+
// This is necessary in order for autotrait bounds on methods of trait
125+
// objects to be sound.
126+
//
127+
// auto trait AutoTrait {}
128+
//
129+
// trait ObjectSafeTrait {
130+
// fn f(&self) where Self: AutoTrait;
131+
// }
132+
//
133+
// We can allow f to be called on `dyn ObjectSafeTrait + AutoTrait`.
134+
//
135+
// If we didn't deny `impl AutoTrait for dyn Trait`, it would be unsound
136+
// for the ObjectSafeTrait shown above to be object safe because someone
137+
// could take some type implementing ObjectSafeTrait but not AutoTrait,
138+
// unsize it to `dyn ObjectSafeTrait`, and call .f() which has no
139+
// concrete implementation (issue #50781).
140+
enum LocalImpl {
141+
Allow,
142+
Disallow { problematic_kind: &'static str },
143+
}
144+
145+
// If the auto-trait is from a dependency, it must only be getting
146+
// implemented for a nominal type, and specifically one local to the
147+
// current crate.
148+
//
149+
// impl<T> Sync for MyStruct<T> {} // okay
150+
//
151+
// impl Sync for Rc<MyStruct> {} // NOT OKAY
152+
enum NonlocalImpl {
153+
Allow,
154+
DisallowBecauseNonlocal,
155+
DisallowOther,
156+
}
157+
158+
// Exhaustive match considering that this logic is essential for
159+
// soundness.
160+
let (local_impl, nonlocal_impl) = match self_ty.kind() {
161+
// struct Struct<T>;
162+
// impl AutoTrait for Struct<Foo> {}
163+
ty::Adt(self_def, _) => (
164+
LocalImpl::Allow,
165+
if self_def.did().is_local() {
166+
NonlocalImpl::Allow
167+
} else {
168+
NonlocalImpl::DisallowBecauseNonlocal
169+
},
170+
),
171+
172+
// extern { type OpaqueType; }
173+
// impl AutoTrait for OpaqueType {}
174+
ty::Foreign(did) => (
175+
LocalImpl::Allow,
176+
if did.is_local() {
177+
NonlocalImpl::Allow
118178
} else {
119-
Some((
120-
format!(
121-
"cross-crate traits with a default impl, like `{}`, \
122-
can only be implemented for a struct/enum type \
123-
defined in the current crate",
124-
tcx.def_path_str(trait_def_id)
125-
),
126-
"can't implement cross-crate trait for type in another crate",
127-
))
179+
NonlocalImpl::DisallowBecauseNonlocal
180+
},
181+
),
182+
183+
// impl AutoTrait for dyn Trait {}
184+
ty::Dynamic(..) => (
185+
LocalImpl::Disallow { problematic_kind: "trait object" },
186+
NonlocalImpl::DisallowOther,
187+
),
188+
189+
// impl<T> AutoTrait for T {}
190+
// impl<T: ?Sized> AutoTrait for T {}
191+
ty::Param(..) => (
192+
if self_ty.is_sized(tcx, tcx.param_env(def_id)) {
193+
LocalImpl::Allow
194+
} else {
195+
LocalImpl::Disallow { problematic_kind: "generic type" }
196+
},
197+
NonlocalImpl::DisallowOther,
198+
),
199+
200+
// trait Id { type This: ?Sized; }
201+
// impl<T: ?Sized> Id for T {
202+
// type This = T;
203+
// }
204+
// impl<T: ?Sized> AutoTrait for <T as Id>::This {}
205+
ty::Alias(AliasKind::Projection, _) => (
206+
LocalImpl::Disallow { problematic_kind: "associated type" },
207+
NonlocalImpl::DisallowOther,
208+
),
209+
210+
// type Opaque = impl Trait;
211+
// impl AutoTrait for Opaque {}
212+
ty::Alias(AliasKind::Opaque, _) => (
213+
LocalImpl::Disallow { problematic_kind: "opaque type" },
214+
NonlocalImpl::DisallowOther,
215+
),
216+
217+
ty::Bool
218+
| ty::Char
219+
| ty::Int(..)
220+
| ty::Uint(..)
221+
| ty::Float(..)
222+
| ty::Str
223+
| ty::Array(..)
224+
| ty::Slice(..)
225+
| ty::RawPtr(..)
226+
| ty::Ref(..)
227+
| ty::FnDef(..)
228+
| ty::FnPtr(..)
229+
| ty::Never
230+
| ty::Tuple(..) => (LocalImpl::Allow, NonlocalImpl::DisallowOther),
231+
232+
ty::Closure(..)
233+
| ty::Generator(..)
234+
| ty::GeneratorWitness(..)
235+
| ty::GeneratorWitnessMIR(..)
236+
| ty::Bound(..)
237+
| ty::Placeholder(..)
238+
| ty::Infer(..) => span_bug!(sp, "weird self type for autotrait impl"),
239+
240+
ty::Error(..) => (LocalImpl::Allow, NonlocalImpl::Allow),
241+
};
242+
243+
if trait_def_id.is_local() {
244+
match local_impl {
245+
LocalImpl::Allow => {}
246+
LocalImpl::Disallow { problematic_kind } => {
247+
let msg = format!(
248+
"traits with a default impl, like `{trait}`, \
249+
cannot be implemented for {problematic_kind} `{self_ty}`",
250+
trait = tcx.def_path_str(trait_def_id),
251+
);
252+
let label = format!(
253+
"a trait object implements `{trait}` if and only if `{trait}` \
254+
is one of the trait object's trait bounds",
255+
trait = tcx.def_path_str(trait_def_id),
256+
);
257+
let reported =
258+
struct_span_err!(tcx.sess, sp, E0321, "{}", msg).note(label).emit();
259+
return Err(reported);
128260
}
129261
}
130-
_ => Some((
131-
format!(
132-
"cross-crate traits with a default impl, like `{}`, can \
262+
} else {
263+
if let Some((msg, label)) = match nonlocal_impl {
264+
NonlocalImpl::Allow => None,
265+
NonlocalImpl::DisallowBecauseNonlocal => Some((
266+
format!(
267+
"cross-crate traits with a default impl, like `{}`, \
268+
can only be implemented for a struct/enum type \
269+
defined in the current crate",
270+
tcx.def_path_str(trait_def_id)
271+
),
272+
"can't implement cross-crate trait for type in another crate",
273+
)),
274+
NonlocalImpl::DisallowOther => Some((
275+
format!(
276+
"cross-crate traits with a default impl, like `{}`, can \
133277
only be implemented for a struct/enum type, not `{}`",
134-
tcx.def_path_str(trait_def_id),
135-
self_ty
136-
),
137-
"can't implement cross-crate trait with a default impl for \
138-
non-struct/enum type",
139-
)),
140-
};
141-
142-
if let Some((msg, label)) = msg {
143-
let reported =
144-
struct_span_err!(tcx.sess, sp, E0321, "{}", msg).span_label(sp, label).emit();
145-
return Err(reported);
278+
tcx.def_path_str(trait_def_id),
279+
self_ty
280+
),
281+
"can't implement cross-crate trait with a default impl for \
282+
non-struct/enum type",
283+
)),
284+
} {
285+
let reported =
286+
struct_span_err!(tcx.sess, sp, E0321, "{}", msg).span_label(sp, label).emit();
287+
return Err(reported);
288+
}
146289
}
147290
}
148291

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

+50-10
Original file line numberDiff line numberDiff line change
@@ -547,16 +547,56 @@ fn virtual_call_violation_for_method<'tcx>(
547547

548548
// NOTE: This check happens last, because it results in a lint, and not a
549549
// hard error.
550-
if tcx
551-
.predicates_of(method.def_id)
552-
.predicates
553-
.iter()
554-
// A trait object can't claim to live more than the concrete type,
555-
// so outlives predicates will always hold.
556-
.cloned()
557-
.filter(|(p, _)| p.to_opt_type_outlives().is_none())
558-
.any(|pred| contains_illegal_self_type_reference(tcx, trait_def_id, pred))
559-
{
550+
if tcx.predicates_of(method.def_id).predicates.iter().any(|&(pred, span)| {
551+
// dyn Trait is okay:
552+
//
553+
// trait Trait {
554+
// fn f(&self) where Self: 'static;
555+
// }
556+
//
557+
// because a trait object can't claim to live longer than the concrete
558+
// type. If the lifetime bound holds on dyn Trait then it's guaranteed
559+
// to hold as well on the concrete type.
560+
if pred.to_opt_type_outlives().is_some() {
561+
return false;
562+
}
563+
564+
// dyn Trait is okay:
565+
//
566+
// auto trait AutoTrait {}
567+
//
568+
// trait Trait {
569+
// fn f(&self) where Self: AutoTrait;
570+
// }
571+
//
572+
// because `impl AutoTrait for dyn Trait` is disallowed by coherence.
573+
// Traits with a default impl are implemented for a trait object if and
574+
// only if the autotrait is one of the trait object's trait bounds, like
575+
// in `dyn Trait + AutoTrait`. This guarantees that trait objects only
576+
// implement auto traits if the underlying type does as well.
577+
if let ty::PredicateKind::Clause(ty::Clause::Trait(ty::TraitPredicate {
578+
trait_ref: pred_trait_ref,
579+
constness: ty::BoundConstness::NotConst,
580+
polarity: ty::ImplPolarity::Positive,
581+
})) = pred.kind().skip_binder()
582+
&& pred_trait_ref.self_ty() == tcx.types.self_param
583+
&& tcx.trait_is_auto(pred_trait_ref.def_id)
584+
{
585+
// Consider bounds like `Self: Bound<Self>`. Auto traits are not
586+
// allowed to have generic parameters so `auto trait Bound<T> {}`
587+
// would already have reported an error at the definition of the
588+
// auto trait.
589+
if pred_trait_ref.substs.len() != 1 {
590+
tcx.sess.diagnostic().delay_span_bug(
591+
span,
592+
"auto traits cannot have generic parameters",
593+
);
594+
}
595+
return false;
596+
}
597+
598+
contains_illegal_self_type_reference(tcx, trait_def_id, pred.clone())
599+
}) {
560600
return Some(MethodViolationCode::WhereClauseReferencesSelf);
561601
}
562602

‎tests/ui/coherence/coherence-impl-trait-for-marker-trait-negative.rs

+15-8
Original file line numberDiff line numberDiff line change
@@ -12,19 +12,26 @@ auto trait Marker2 {}
1212
trait Object: Marker1 {}
1313

1414
// A supertrait marker is illegal...
15-
impl !Marker1 for dyn Object + Marker2 { } //~ ERROR E0371
15+
impl !Marker1 for dyn Object + Marker2 {} //~ ERROR E0371
16+
//~^ ERROR 0321
1617
// ...and also a direct component.
17-
impl !Marker2 for dyn Object + Marker2 { } //~ ERROR E0371
18-
19-
// But implementing a marker if it is not present is OK.
20-
impl !Marker2 for dyn Object {} // OK
18+
impl !Marker2 for dyn Object + Marker2 {} //~ ERROR E0371
19+
//~^ ERROR 0321
2120

2221
// A non-principal trait-object type is orphan even in its crate.
2322
impl !Send for dyn Marker2 {} //~ ERROR E0117
2423

25-
// And impl'ing a remote marker for a local trait object is forbidden
26-
// by one of these special orphan-like rules.
24+
// Implementing a marker for a local trait object is forbidden by a special
25+
// orphan-like rule.
26+
impl !Marker2 for dyn Object {} //~ ERROR E0321
2727
impl !Send for dyn Object {} //~ ERROR E0321
2828
impl !Send for dyn Object + Marker2 {} //~ ERROR E0321
2929

30-
fn main() { }
30+
// Blanket impl that applies to dyn Object is equally problematic.
31+
auto trait Marker3 {}
32+
impl<T: ?Sized> !Marker3 for T {} //~ ERROR E0321
33+
34+
auto trait Marker4 {}
35+
impl<T> !Marker4 for T {} // okay
36+
37+
fn main() {}

0 commit comments

Comments
 (0)