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.

Sign up for GitHub

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

Jump to bottom

Autotrait bounds on dyn-safe trait methods #107082

Merged
merged 2 commits into from
Feb 3, 2023
Merged

Autotrait bounds on dyn-safe trait methods #107082

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
9e1c600
Disallow impl autotrait for trait object
dtolnay Jan 19, 2023
4501d3a
Autotrait bounds on dyn-safe trait methods
dtolnay Jan 19, 2023
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
4 changes: 2 additions & 2 deletions compiler/rustc_data_structures/src/sync.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,8 +31,8 @@ cfg_if! {
pub auto trait Send {}
pub auto trait Sync {}

impl<T: ?Sized> Send for T {}
impl<T: ?Sized> Sync for T {}
impl<T> Send for T {}
impl<T> Sync for T {}

#[macro_export]
macro_rules! rustc_erase_owner {
Expand Down
223 changes: 183 additions & 40 deletions compiler/rustc_hir_analysis/src/coherence/orphan.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ use rustc_hir as hir;
use rustc_middle::ty::subst::InternalSubsts;
use rustc_middle::ty::util::IgnoreRegions;
use rustc_middle::ty::{
self, ImplPolarity, Ty, TyCtxt, TypeSuperVisitable, TypeVisitable, TypeVisitor,
self, AliasKind, ImplPolarity, Ty, TyCtxt, TypeSuperVisitable, TypeVisitable, TypeVisitor,
};
use rustc_session::lint;
use rustc_span::def_id::{DefId, LocalDefId};
Expand Down Expand Up @@ -86,7 +86,7 @@ fn do_orphan_check_impl<'tcx>(
// struct B { }
// impl Foo for A { }
// impl Foo for B { }
// impl !Send for (A, B) { }
// impl !Foo for (A, B) { }
// ```
//
// This final impl is legal according to the orphan
Expand All @@ -99,50 +99,193 @@ fn do_orphan_check_impl<'tcx>(
tcx.trait_is_auto(trait_def_id)
);

if tcx.trait_is_auto(trait_def_id) && !trait_def_id.is_local() {
if tcx.trait_is_auto(trait_def_id) {
let self_ty = trait_ref.self_ty();
let opt_self_def_id = match *self_ty.kind() {
ty::Adt(self_def, _) => Some(self_def.did()),
ty::Foreign(did) => Some(did),
_ => None,
};

let msg = match opt_self_def_id {
// We only want to permit nominal types, but not *all* nominal types.
// They must be local to the current crate, so that people
// can't do `unsafe impl Send for Rc<SomethingLocal>` or
// `impl !Send for Box<SomethingLocalAndSend>`.
Some(self_def_id) => {
if self_def_id.is_local() {
None
// If the impl is in the same crate as the auto-trait, almost anything
// goes.
//
// impl MyAuto for Rc<Something> {} // okay
// impl<T> !MyAuto for *const T {} // okay
// impl<T> MyAuto for T {} // okay
//
// But there is one important exception: implementing for a trait object
// is not allowed.
//
// impl MyAuto for dyn Trait {} // NOT OKAY
// impl<T: ?Sized> MyAuto for T {} // NOT OKAY
//
// With this restriction, it's guaranteed that an auto-trait is
// implemented for a trait object if and only if the auto-trait 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.
//
// This is necessary in order for autotrait bounds on methods of trait
// objects to be sound.
//
// auto trait AutoTrait {}
//
// trait ObjectSafeTrait {
// fn f(&self) where Self: AutoTrait;
// }
//
// We can allow f to be called on `dyn ObjectSafeTrait + AutoTrait`.
//
// If we didn't deny `impl AutoTrait for dyn Trait`, it would be unsound
// for the ObjectSafeTrait shown above to be object safe because someone
// could take some type implementing ObjectSafeTrait but not AutoTrait,
// unsize it to `dyn ObjectSafeTrait`, and call .f() which has no
// concrete implementation (issue #50781).
enum LocalImpl {
Allow,
Disallow { problematic_kind: &'static str },
}

// If the auto-trait is from a dependency, it must only be getting
// implemented for a nominal type, and specifically one local to the
// current crate.
//
// impl<T> Sync for MyStruct<T> {} // okay
//
// impl Sync for Rc<MyStruct> {} // NOT OKAY
enum NonlocalImpl {
Allow,
DisallowBecauseNonlocal,
DisallowOther,
}

// Exhaustive match considering that this logic is essential for
// soundness.
let (local_impl, nonlocal_impl) = match self_ty.kind() {
// struct Struct<T>;
// impl AutoTrait for Struct<Foo> {}
ty::Adt(self_def, _) => (
LocalImpl::Allow,
if self_def.did().is_local() {
NonlocalImpl::Allow
} else {
NonlocalImpl::DisallowBecauseNonlocal
},
),

// extern { type OpaqueType; }
// impl AutoTrait for OpaqueType {}
ty::Foreign(did) => (
LocalImpl::Allow,
if did.is_local() {
NonlocalImpl::Allow
} else {
Some((
format!(
"cross-crate traits with a default impl, like `{}`, \
can only be implemented for a struct/enum type \
defined in the current crate",
tcx.def_path_str(trait_def_id)
),
"can't implement cross-crate trait for type in another crate",
))
NonlocalImpl::DisallowBecauseNonlocal
},
),

// impl AutoTrait for dyn Trait {}
ty::Dynamic(..) => (
LocalImpl::Disallow { problematic_kind: "trait object" },
NonlocalImpl::DisallowOther,
),

// impl<T> AutoTrait for T {}
// impl<T: ?Sized> AutoTrait for T {}
ty::Param(..) => (
if self_ty.is_sized(tcx, tcx.param_env(def_id)) {
LocalImpl::Allow
} else {
LocalImpl::Disallow { problematic_kind: "generic type" }
},
NonlocalImpl::DisallowOther,
),

// trait Id { type This: ?Sized; }
// impl<T: ?Sized> Id for T {
// type This = T;
// }
// impl<T: ?Sized> AutoTrait for <T as Id>::This {}
ty::Alias(AliasKind::Projection, _) => (
LocalImpl::Disallow { problematic_kind: "associated type" },
NonlocalImpl::DisallowOther,
),

// type Opaque = impl Trait;
// impl AutoTrait for Opaque {}
ty::Alias(AliasKind::Opaque, _) => (
LocalImpl::Disallow { problematic_kind: "opaque type" },
NonlocalImpl::DisallowOther,
),

ty::Bool
| ty::Char
| ty::Int(..)
| ty::Uint(..)
| ty::Float(..)
| ty::Str
| ty::Array(..)
| ty::Slice(..)
| ty::RawPtr(..)
| ty::Ref(..)
| ty::FnDef(..)
| ty::FnPtr(..)
| ty::Never
| ty::Tuple(..) => (LocalImpl::Allow, NonlocalImpl::DisallowOther),

ty::Closure(..)
| ty::Generator(..)
| ty::GeneratorWitness(..)
| ty::GeneratorWitnessMIR(..)
| ty::Bound(..)
| ty::Placeholder(..)
| ty::Infer(..) => span_bug!(sp, "weird self type for autotrait impl"),

ty::Error(..) => (LocalImpl::Allow, NonlocalImpl::Allow),
};

if trait_def_id.is_local() {
match local_impl {
LocalImpl::Allow => {}
LocalImpl::Disallow { problematic_kind } => {
let msg = format!(
"traits with a default impl, like `{trait}`, \
cannot be implemented for {problematic_kind} `{self_ty}`",
trait = tcx.def_path_str(trait_def_id),
);
let label = format!(
"a trait object implements `{trait}` if and only if `{trait}` \
is one of the trait object's trait bounds",
trait = tcx.def_path_str(trait_def_id),
);
let reported =
struct_span_err!(tcx.sess, sp, E0321, "{}", msg).note(label).emit();
return Err(reported);
}
}
_ => Some((
format!(
"cross-crate traits with a default impl, like `{}`, can \
} else {
if let Some((msg, label)) = match nonlocal_impl {
NonlocalImpl::Allow => None,
NonlocalImpl::DisallowBecauseNonlocal => Some((
format!(
"cross-crate traits with a default impl, like `{}`, \
can only be implemented for a struct/enum type \
defined in the current crate",
tcx.def_path_str(trait_def_id)
),
"can't implement cross-crate trait for type in another crate",
)),
NonlocalImpl::DisallowOther => Some((
format!(
"cross-crate traits with a default impl, like `{}`, can \
only be implemented for a struct/enum type, not `{}`",
tcx.def_path_str(trait_def_id),
self_ty
),
"can't implement cross-crate trait with a default impl for \
non-struct/enum type",
)),
};

if let Some((msg, label)) = msg {
let reported =
struct_span_err!(tcx.sess, sp, E0321, "{}", msg).span_label(sp, label).emit();
return Err(reported);
tcx.def_path_str(trait_def_id),
self_ty
),
"can't implement cross-crate trait with a default impl for \
non-struct/enum type",
)),
} {
let reported =
struct_span_err!(tcx.sess, sp, E0321, "{}", msg).span_label(sp, label).emit();
return Err(reported);
}
}
}

Expand Down
60 changes: 50 additions & 10 deletions compiler/rustc_trait_selection/src/traits/object_safety.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -547,16 +547,56 @@ fn virtual_call_violation_for_method<'tcx>(

// NOTE: This check happens last, because it results in a lint, and not a
// hard error.
if tcx
.predicates_of(method.def_id)
.predicates
.iter()
// A trait object can't claim to live more than the concrete type,
// so outlives predicates will always hold.
.cloned()
.filter(|(p, _)| p.to_opt_type_outlives().is_none())
.any(|pred| contains_illegal_self_type_reference(tcx, trait_def_id, pred))
{
if tcx.predicates_of(method.def_id).predicates.iter().any(|&(pred, span)| {
// dyn Trait is okay:
//
// trait Trait {
// fn f(&self) where Self: 'static;
// }
//
// because a trait object can't claim to live longer than the concrete
// type. If the lifetime bound holds on dyn Trait then it's guaranteed
// to hold as well on the concrete type.
if pred.to_opt_type_outlives().is_some() {
return false;
}

// dyn Trait is okay:
//
// auto trait AutoTrait {}
//
// trait Trait {
// fn f(&self) where Self: AutoTrait;
// }
//
// because `impl AutoTrait for dyn Trait` is disallowed by coherence.
// 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, like
// in `dyn Trait + AutoTrait`. This guarantees that trait objects only
// implement auto traits if the underlying type does as well.
if let ty::PredicateKind::Clause(ty::Clause::Trait(ty::TraitPredicate {
trait_ref: pred_trait_ref,
constness: ty::BoundConstness::NotConst,
polarity: ty::ImplPolarity::Positive,
})) = pred.kind().skip_binder()
&& pred_trait_ref.self_ty() == tcx.types.self_param
&& tcx.trait_is_auto(pred_trait_ref.def_id)
{
// Consider bounds like `Self: Bound<Self>`. Auto traits are not
// allowed to have generic parameters so `auto trait Bound<T> {}`
// would already have reported an error at the definition of the
// auto trait.
if pred_trait_ref.substs.len() != 1 {
tcx.sess.diagnostic().delay_span_bug(
span,
"auto traits cannot have generic parameters",
);
}
return false;
}

contains_illegal_self_type_reference(tcx, trait_def_id, pred.clone())
}) {
return Some(MethodViolationCode::WhereClauseReferencesSelf);
}

Expand Down
23 changes: 15 additions & 8 deletions tests/ui/coherence/coherence-impl-trait-for-marker-trait-negative.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,19 +12,26 @@ auto trait Marker2 {}
trait Object: Marker1 {}

// A supertrait marker is illegal...
impl !Marker1 for dyn Object + Marker2 { } //~ ERROR E0371
impl !Marker1 for dyn Object + Marker2 {} //~ ERROR E0371
//~^ ERROR 0321
// ...and also a direct component.
impl !Marker2 for dyn Object + Marker2 { } //~ ERROR E0371

// But implementing a marker if it is not present is OK.
impl !Marker2 for dyn Object {} // OK
impl !Marker2 for dyn Object + Marker2 {} //~ ERROR E0371
//~^ ERROR 0321

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

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

fn main() { }
// Blanket impl that applies to dyn Object is equally problematic.
auto trait Marker3 {}
impl<T: ?Sized> !Marker3 for T {} //~ ERROR E0321

auto trait Marker4 {}
impl<T> !Marker4 for T {} // okay

fn main() {}
Loading