Skip to content

Commit

Permalink
Auto merge of #18205 - noahmbright:object_safety, r=HKalbasi
Browse files Browse the repository at this point in the history
Rename object_safety

First PR here (yay!), so I read some of the getting started docs. There are a couple references to `handlers.rs`, which as far as I can tell has been refactored into `handlers/*.rs`. I made some tweaks to that in one commit. There is one fixme about a function called `to_lsp_runnable`, which I can't find anywhere at all. I can update that if I get some more info there.

Otherwise I changed references to object safety, is object safe, etc., trying to match case/style as I went. There was one case I found where there's a trait from somewhere else called `is_object_safe`, which I found defined in my cargo registry. I didn't touch that for now, just marked it with a fixme
  • Loading branch information
bors committed Sep 29, 2024
2 parents 792e795 + 4255cae commit 822644d
Show file tree
Hide file tree
Showing 16 changed files with 117 additions and 109 deletions.
3 changes: 2 additions & 1 deletion crates/hir-ty/src/chalk_db.rs
Original file line number Diff line number Diff line change
Expand Up @@ -382,8 +382,9 @@ impl chalk_solve::RustIrDatabase<Interner> for ChalkContext<'_> {
}

fn is_object_safe(&self, trait_id: chalk_ir::TraitId<Interner>) -> bool {
// FIXME: When cargo is updated, change to dyn_compatibility
let trait_ = from_chalk_trait_id(trait_id);
crate::object_safety::object_safety(self.db, trait_).is_none()
crate::dyn_compatibility::dyn_compatibility(self.db, trait_).is_none()
}

fn closure_kind(
Expand Down
10 changes: 5 additions & 5 deletions crates/hir-ty/src/db.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,11 +20,11 @@ use triomphe::Arc;
use crate::{
chalk_db,
consteval::ConstEvalError,
dyn_compatibility::DynCompatibilityViolation,
layout::{Layout, LayoutError},
lower::{GenericDefaults, GenericPredicates},
method_resolution::{InherentImpls, TraitImpls, TyFingerprint},
mir::{BorrowckResult, MirBody, MirLowerError},
object_safety::ObjectSafetyViolation,
Binders, ClosureId, Const, FnDefId, ImplTraitId, ImplTraits, InferenceResult, Interner,
PolyFnSig, Substitution, TraitEnvironment, TraitRef, Ty, TyDefId, ValueTyDefId,
};
Expand Down Expand Up @@ -108,8 +108,8 @@ pub trait HirDatabase: DefDatabase + Upcast<dyn DefDatabase> {
#[salsa::invoke(crate::layout::target_data_layout_query)]
fn target_data_layout(&self, krate: CrateId) -> Result<Arc<TargetDataLayout>, Arc<str>>;

#[salsa::invoke(crate::object_safety::object_safety_of_trait_query)]
fn object_safety_of_trait(&self, trait_: TraitId) -> Option<ObjectSafetyViolation>;
#[salsa::invoke(crate::dyn_compatibility::dyn_compatibility_of_trait_query)]
fn dyn_compatibility_of_trait(&self, trait_: TraitId) -> Option<DynCompatibilityViolation>;

#[salsa::invoke(crate::lower::ty_query)]
#[salsa::cycle(crate::lower::ty_recover)]
Expand Down Expand Up @@ -280,8 +280,8 @@ pub trait HirDatabase: DefDatabase + Upcast<dyn DefDatabase> {
}

#[test]
fn hir_database_is_object_safe() {
fn _assert_object_safe(_: &dyn HirDatabase) {}
fn hir_database_is_dyn_compatible() {
fn _assert_dyn_compatible(_: &dyn HirDatabase) {}
}

#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)]
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
//! Compute the object-safety of a trait
//! Compute the dyn-compatibility of a trait

use std::ops::ControlFlow;

Expand Down Expand Up @@ -28,14 +28,14 @@ use crate::{
};

#[derive(Debug, Clone, PartialEq, Eq, Hash)]
pub enum ObjectSafetyViolation {
pub enum DynCompatibilityViolation {
SizedSelf,
SelfReferential,
Method(FunctionId, MethodViolationCode),
AssocConst(ConstId),
GAT(TypeAliasId),
// This doesn't exist in rustc, but added for better visualization
HasNonSafeSuperTrait(TraitId),
HasNonCompatibleSuperTrait(TraitId),
}

#[derive(Debug, Clone, PartialEq, Eq, Hash)]
Expand All @@ -50,70 +50,73 @@ pub enum MethodViolationCode {
UndispatchableReceiver,
}

pub fn object_safety(db: &dyn HirDatabase, trait_: TraitId) -> Option<ObjectSafetyViolation> {
pub fn dyn_compatibility(
db: &dyn HirDatabase,
trait_: TraitId,
) -> Option<DynCompatibilityViolation> {
for super_trait in all_super_traits(db.upcast(), trait_).into_iter().skip(1).rev() {
if db.object_safety_of_trait(super_trait).is_some() {
return Some(ObjectSafetyViolation::HasNonSafeSuperTrait(super_trait));
if db.dyn_compatibility_of_trait(super_trait).is_some() {
return Some(DynCompatibilityViolation::HasNonCompatibleSuperTrait(super_trait));
}
}

db.object_safety_of_trait(trait_)
db.dyn_compatibility_of_trait(trait_)
}

pub fn object_safety_with_callback<F>(
pub fn dyn_compatibility_with_callback<F>(
db: &dyn HirDatabase,
trait_: TraitId,
cb: &mut F,
) -> ControlFlow<()>
where
F: FnMut(ObjectSafetyViolation) -> ControlFlow<()>,
F: FnMut(DynCompatibilityViolation) -> ControlFlow<()>,
{
for super_trait in all_super_traits(db.upcast(), trait_).into_iter().skip(1).rev() {
if db.object_safety_of_trait(super_trait).is_some() {
cb(ObjectSafetyViolation::HasNonSafeSuperTrait(trait_))?;
if db.dyn_compatibility_of_trait(super_trait).is_some() {
cb(DynCompatibilityViolation::HasNonCompatibleSuperTrait(trait_))?;
}
}

object_safety_of_trait_with_callback(db, trait_, cb)
dyn_compatibility_of_trait_with_callback(db, trait_, cb)
}

pub fn object_safety_of_trait_with_callback<F>(
pub fn dyn_compatibility_of_trait_with_callback<F>(
db: &dyn HirDatabase,
trait_: TraitId,
cb: &mut F,
) -> ControlFlow<()>
where
F: FnMut(ObjectSafetyViolation) -> ControlFlow<()>,
F: FnMut(DynCompatibilityViolation) -> ControlFlow<()>,
{
// Check whether this has a `Sized` bound
if generics_require_sized_self(db, trait_.into()) {
cb(ObjectSafetyViolation::SizedSelf)?;
cb(DynCompatibilityViolation::SizedSelf)?;
}

// Check if there exist bounds that referencing self
if predicates_reference_self(db, trait_) {
cb(ObjectSafetyViolation::SelfReferential)?;
cb(DynCompatibilityViolation::SelfReferential)?;
}
if bounds_reference_self(db, trait_) {
cb(ObjectSafetyViolation::SelfReferential)?;
cb(DynCompatibilityViolation::SelfReferential)?;
}

// rustc checks for non-lifetime binders here, but we don't support HRTB yet

let trait_data = db.trait_data(trait_);
for (_, assoc_item) in &trait_data.items {
object_safety_violation_for_assoc_item(db, trait_, *assoc_item, cb)?;
dyn_compatibility_violation_for_assoc_item(db, trait_, *assoc_item, cb)?;
}

ControlFlow::Continue(())
}

pub fn object_safety_of_trait_query(
pub fn dyn_compatibility_of_trait_query(
db: &dyn HirDatabase,
trait_: TraitId,
) -> Option<ObjectSafetyViolation> {
) -> Option<DynCompatibilityViolation> {
let mut res = None;
object_safety_of_trait_with_callback(db, trait_, &mut |osv| {
dyn_compatibility_of_trait_with_callback(db, trait_, &mut |osv| {
res = Some(osv);
ControlFlow::Break(())
});
Expand Down Expand Up @@ -321,14 +324,14 @@ fn contains_illegal_self_type_reference<T: TypeVisitable<Interner>>(
t.visit_with(visitor.as_dyn(), outer_binder).is_break()
}

fn object_safety_violation_for_assoc_item<F>(
fn dyn_compatibility_violation_for_assoc_item<F>(
db: &dyn HirDatabase,
trait_: TraitId,
item: AssocItemId,
cb: &mut F,
) -> ControlFlow<()>
where
F: FnMut(ObjectSafetyViolation) -> ControlFlow<()>,
F: FnMut(DynCompatibilityViolation) -> ControlFlow<()>,
{
// Any item that has a `Self : Sized` requisite is otherwise
// exempt from the regulations.
Expand All @@ -337,10 +340,10 @@ where
}

match item {
AssocItemId::ConstId(it) => cb(ObjectSafetyViolation::AssocConst(it)),
AssocItemId::ConstId(it) => cb(DynCompatibilityViolation::AssocConst(it)),
AssocItemId::FunctionId(it) => {
virtual_call_violations_for_method(db, trait_, it, &mut |mvc| {
cb(ObjectSafetyViolation::Method(it, mvc))
cb(DynCompatibilityViolation::Method(it, mvc))
})
}
AssocItemId::TypeAliasId(it) => {
Expand All @@ -350,7 +353,7 @@ where
} else {
let generic_params = db.generic_params(item.into());
if !generic_params.is_empty() {
cb(ObjectSafetyViolation::GAT(it))
cb(DynCompatibilityViolation::GAT(it))
} else {
ControlFlow::Continue(())
}
Expand Down Expand Up @@ -469,7 +472,7 @@ fn receiver_is_dispatchable(
return false;
};

// `self: Self` can't be dispatched on, but this is already considered object safe.
// `self: Self` can't be dispatched on, but this is already considered dyn compatible
// See rustc's comment on https://github.com/rust-lang/rust/blob/3f121b9461cce02a703a0e7e450568849dfaa074/compiler/rustc_trait_selection/src/traits/object_safety.rs#L433-L437
if sig
.skip_binders()
Expand Down
Loading

0 comments on commit 822644d

Please sign in to comment.