Skip to content

Merge method, type and const object safety checks #112318

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

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
6 changes: 5 additions & 1 deletion compiler/rustc_hir_analysis/src/astconv/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1601,7 +1601,7 @@ impl<'o, 'tcx> dyn AstConv<'tcx> + 'o {
tcx.associated_items(pred.def_id())
.in_definition_order()
.filter(|item| item.kind == ty::AssocKind::Type)
.filter(|item| tcx.opt_rpitit_info(item.def_id).is_none())
.filter(|item| item.opt_rpitit_info.is_none())
.map(|item| item.def_id),
);
}
Expand Down Expand Up @@ -1643,6 +1643,10 @@ impl<'o, 'tcx> dyn AstConv<'tcx> + 'o {
}
}

// `dyn Trait<Assoc = Foo>` desugars to (not Rust syntax) `dyn Trait where <Self as Trait>::Assoc = Foo`.
// So every `Projection` clause is an `Assoc = Foo` bound. `associated_types` contains all associated
// types's `DefId`, so the following loop removes all the `DefIds` of the associated types that have a
// corresponding `Projection` clause
for (projection_bound, _) in &projection_bounds {
for def_ids in associated_types.values_mut() {
def_ids.remove(&projection_bound.projection_def_id());
Expand Down
4 changes: 2 additions & 2 deletions compiler/rustc_hir_analysis/src/check/compare_impl_item.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1216,7 +1216,7 @@ fn compare_number_of_generics<'tcx>(
// has mismatched type or const generic arguments, then the method that it's
// inheriting the generics from will also have mismatched arguments, and
// we'll report an error for that instead. Delay a bug for safety, though.
if tcx.opt_rpitit_info(trait_.def_id).is_some() {
if trait_.opt_rpitit_info.is_some() {
return Err(tcx.sess.delay_span_bug(
rustc_span::DUMMY_SP,
"errors comparing numbers of generics of trait/impl functions were not emitted",
Expand Down Expand Up @@ -2006,7 +2006,7 @@ pub(super) fn check_type_bounds<'tcx>(
// A synthetic impl Trait for RPITIT desugaring has no HIR, which we currently use to get the
// span for an impl's associated type. Instead, for these, use the def_span for the synthesized
// associated type.
let impl_ty_span = if tcx.opt_rpitit_info(impl_ty.def_id).is_some() {
let impl_ty_span = if impl_ty.opt_rpitit_info.is_some() {
tcx.def_span(impl_ty_def_id)
} else {
match tcx.hir().get_by_def_id(impl_ty_def_id) {
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_hir_analysis/src/check/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -188,7 +188,7 @@ fn missing_items_err(
full_impl_span: Span,
) {
let missing_items =
missing_items.iter().filter(|trait_item| tcx.opt_rpitit_info(trait_item.def_id).is_none());
missing_items.iter().filter(|trait_item| trait_item.opt_rpitit_info.is_none());

let missing_items_msg = missing_items
.clone()
Expand Down
94 changes: 43 additions & 51 deletions compiler/rustc_trait_selection/src/traits/object_safety.rs
Original file line number Diff line number Diff line change
Expand Up @@ -115,15 +115,11 @@ fn object_safety_violations_for_trait(
tcx: TyCtxt<'_>,
trait_def_id: DefId,
) -> Vec<ObjectSafetyViolation> {
// Check methods for violations.
// Check assoc items for violations.
let mut violations: Vec<_> = tcx
.associated_items(trait_def_id)
.in_definition_order()
.filter(|item| item.kind == ty::AssocKind::Fn)
.filter_map(|&item| {
object_safety_violation_for_method(tcx, trait_def_id, item)
.map(|(code, span)| ObjectSafetyViolation::Method(item.name, code, span))
})
.filter_map(|&item| object_safety_violation_for_assoc_item(tcx, trait_def_id, item))
.collect();

// Check the trait itself.
Expand All @@ -145,30 +141,6 @@ fn object_safety_violations_for_trait(
violations.push(ObjectSafetyViolation::SupertraitNonLifetimeBinder(spans));
}

violations.extend(
tcx.associated_items(trait_def_id)
.in_definition_order()
.filter(|item| item.kind == ty::AssocKind::Const)
.map(|item| {
let ident = item.ident(tcx);
ObjectSafetyViolation::AssocConst(ident.name, ident.span)
}),
);

if !tcx.features().generic_associated_types_extended {
violations.extend(
tcx.associated_items(trait_def_id)
.in_definition_order()
.filter(|item| item.kind == ty::AssocKind::Type)
.filter(|item| !tcx.generics_of(item.def_id).params.is_empty())
.filter(|item| tcx.opt_rpitit_info(item.def_id).is_none())
.map(|item| {
let ident = item.ident(tcx);
ObjectSafetyViolation::GAT(ident.name, ident.span)
}),
);
}

debug!(
"object_safety_violations_for_trait(trait_def_id={:?}) = {:?}",
trait_def_id, violations
Expand Down Expand Up @@ -401,34 +373,54 @@ fn generics_require_sized_self(tcx: TyCtxt<'_>, def_id: DefId) -> bool {
})
}

/// Returns `Some(_)` if this method makes the containing trait not object safe.
fn object_safety_violation_for_method(
/// Returns `Some(_)` if this item makes the containing trait not object safe.
#[instrument(level = "debug", skip(tcx), ret)]
fn object_safety_violation_for_assoc_item(
tcx: TyCtxt<'_>,
trait_def_id: DefId,
method: ty::AssocItem,
) -> Option<(MethodViolationCode, Span)> {
debug!("object_safety_violation_for_method({:?}, {:?})", trait_def_id, method);
// Any method that has a `Self : Sized` requisite is otherwise
item: ty::AssocItem,
) -> Option<ObjectSafetyViolation> {
// Any item that has a `Self : Sized` requisite is otherwise
// exempt from the regulations.
if generics_require_sized_self(tcx, method.def_id) {
if generics_require_sized_self(tcx, item.def_id) {
return None;
}

let violation = virtual_call_violation_for_method(tcx, trait_def_id, method);
// Get an accurate span depending on the violation.
violation.map(|v| {
let node = tcx.hir().get_if_local(method.def_id);
let span = match (&v, node) {
(MethodViolationCode::ReferencesSelfInput(Some(span)), _) => *span,
(MethodViolationCode::UndispatchableReceiver(Some(span)), _) => *span,
(MethodViolationCode::ReferencesImplTraitInTrait(span), _) => *span,
(MethodViolationCode::ReferencesSelfOutput, Some(node)) => {
node.fn_decl().map_or(method.ident(tcx).span, |decl| decl.output.span())
match item.kind {
// Associated consts are never object safe, as they can't have `where` bounds yet at all,
// and associated const bounds in trait objects aren't a thing yet either.
ty::AssocKind::Const => {
Some(ObjectSafetyViolation::AssocConst(item.name, item.ident(tcx).span))
}
ty::AssocKind::Fn => virtual_call_violation_for_method(tcx, trait_def_id, item).map(|v| {
let node = tcx.hir().get_if_local(item.def_id);
// Get an accurate span depending on the violation.
let span = match (&v, node) {
(MethodViolationCode::ReferencesSelfInput(Some(span)), _) => *span,
(MethodViolationCode::UndispatchableReceiver(Some(span)), _) => *span,
(MethodViolationCode::ReferencesImplTraitInTrait(span), _) => *span,
(MethodViolationCode::ReferencesSelfOutput, Some(node)) => {
node.fn_decl().map_or(item.ident(tcx).span, |decl| decl.output.span())
}
_ => item.ident(tcx).span,
};

ObjectSafetyViolation::Method(item.name, v, span)
}),
// Associated types can only be object safe if they have `Self: Sized` bounds.
ty::AssocKind::Type => {
if !tcx.features().generic_associated_types_extended
&& !tcx.generics_of(item.def_id).params.is_empty()
&& item.opt_rpitit_info.is_none()
{
Some(ObjectSafetyViolation::GAT(item.name, item.ident(tcx).span))
} else {
// We will permit associated types if they are explicitly mentioned in the trait object.
// We can't check this here, as here we only check if it is guaranteed to not be possible.
None
}
_ => method.ident(tcx).span,
};
(v, span)
})
}
}
}

/// Returns `Some(_)` if this method cannot be called on a trait
Expand Down