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.

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

Make WHERE_CLAUSES_OBJECT_SAFETY a regular object safety violation #125380

Merged
merged 2 commits into from
Jun 4, 2024
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
2 changes: 1 addition & 1 deletion compiler/rustc_hir_analysis/src/check/wfcheck.rs
Original file line number Diff line number Diff line change
Expand Up @@ -881,7 +881,7 @@ fn check_object_unsafe_self_trait_by_name(tcx: TyCtxt<'_>, item: &hir::TraitItem
_ => {}
}
if !trait_should_be_self.is_empty() {
if tcx.check_is_object_safe(trait_def_id) {
if tcx.is_object_safe(trait_def_id) {
return;
}
let sugg = trait_should_be_self.iter().map(|span| (*span, "Self".to_string())).collect();
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_hir_analysis/src/coherence/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -191,7 +191,7 @@ fn check_object_overlap<'tcx>(
});

for component_def_id in component_def_ids {
if !tcx.check_is_object_safe(component_def_id) {
if !tcx.is_object_safe(component_def_id) {
// Without the 'object_safe_for_dispatch' feature this is an error
// which will be reported by wfcheck. Ignore it here.
// This is tested by `coherence-impl-trait-for-trait-object-safe.rs`.
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_hir_analysis/src/hir_ty_lowering/lint.rs
Original file line number Diff line number Diff line change
Expand Up @@ -182,7 +182,7 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ {
// For recursive traits, don't downgrade the error. (#119652)
is_downgradable = false;
}
tcx.check_is_object_safe(id)
tcx.is_object_safe(id)
}
_ => false,
})
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_interface/src/passes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -832,7 +832,7 @@ fn analysis(tcx: TyCtxt<'_>, (): ()) -> Result<()> {
let traits = tcx.traits(LOCAL_CRATE);

for &tr in traits {
if !tcx.check_is_object_safe(tr) {
if !tcx.is_object_safe(tr) {
continue;
}

Expand Down
4 changes: 2 additions & 2 deletions compiler/rustc_lint/src/multiple_supertrait_upcastable.rs
Original file line number Diff line number Diff line change
Expand Up @@ -38,10 +38,10 @@ declare_lint_pass!(MultipleSupertraitUpcastable => [MULTIPLE_SUPERTRAIT_UPCASTAB
impl<'tcx> LateLintPass<'tcx> for MultipleSupertraitUpcastable {
fn check_item(&mut self, cx: &LateContext<'tcx>, item: &'tcx hir::Item<'tcx>) {
let def_id = item.owner_id.to_def_id();
// NOTE(nbdd0121): use `object_safety_violations` instead of `check_is_object_safe` because
// NOTE(nbdd0121): use `object_safety_violations` instead of `is_object_safe` because
// the latter will report `where_clause_object_safety` lint.
if let hir::ItemKind::Trait(_, _, _, _, _) = item.kind
&& cx.tcx.object_safety_violations(def_id).is_empty()
&& cx.tcx.is_object_safe(def_id)
{
let direct_super_traits_iter = cx
.tcx
Expand Down
42 changes: 0 additions & 42 deletions compiler/rustc_lint_defs/src/builtin.rs
Original file line number Diff line number Diff line change
Expand Up @@ -136,7 +136,6 @@ declare_lint_pass! {
USELESS_DEPRECATED,
WARNINGS,
WASM_C_ABI,
WHERE_CLAUSES_OBJECT_SAFETY,
WRITES_THROUGH_IMMUTABLE_POINTER,
// tidy-alphabetical-end
]
Expand Down Expand Up @@ -2093,47 +2092,6 @@ declare_lint! {
"detects labels that are never used"
}

declare_lint! {
/// The `where_clauses_object_safety` lint detects for [object safety] of
/// [where clauses].
///
/// [object safety]: https://doc.rust-lang.org/reference/items/traits.html#object-safety
/// [where clauses]: https://doc.rust-lang.org/reference/items/generics.html#where-clauses
///
/// ### Example
///
/// ```rust,no_run
/// trait Trait {}
///
/// trait X { fn foo(&self) where Self: Trait; }
///
/// impl X for () { fn foo(&self) {} }
///
/// impl Trait for dyn X {}
///
/// // Segfault at opt-level 0, SIGILL otherwise.
/// pub fn main() { <dyn X as X>::foo(&()); }
/// ```
///
/// {{produces}}
///
/// ### Explanation
///
/// The compiler previously allowed these object-unsafe bounds, which was
/// incorrect. This is a [future-incompatible] lint to transition this to
/// a hard error in the future. See [issue #51443] for more details.
///
/// [issue #51443]: https://github.com/rust-lang/rust/issues/51443
/// [future-incompatible]: ../index.md#future-incompatible-lints
pub WHERE_CLAUSES_OBJECT_SAFETY,
Warn,
"checks the object safety of where clauses",
@future_incompatible = FutureIncompatibleInfo {
reason: FutureIncompatibilityReason::FutureReleaseErrorDontReportInDeps,
reference: "issue #51443 <https://github.com/rust-lang/rust/issues/51443>",
};
}

declare_lint! {
/// The `proc_macro_derive_resolution_fallback` lint detects proc macro
/// derives using inaccessible names from parent modules.
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_middle/src/query/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1309,7 +1309,7 @@ rustc_queries! {
query object_safety_violations(trait_id: DefId) -> &'tcx [ObjectSafetyViolation] {
desc { |tcx| "determining object safety of trait `{}`", tcx.def_path_str(trait_id) }
}
query check_is_object_safe(trait_id: DefId) -> bool {
query is_object_safe(trait_id: DefId) -> bool {
desc { |tcx| "checking if trait `{}` is object safe", tcx.def_path_str(trait_id) }
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -367,7 +367,7 @@ pub fn transform_instance<'tcx>(
let trait_method = tcx.associated_item(method_id);
let trait_id = trait_ref.skip_binder().def_id;
if traits::is_vtable_safe_method(tcx, trait_id, trait_method)
&& tcx.object_safety_violations(trait_id).is_empty()
&& tcx.is_object_safe(trait_id)
{
// Trait methods will have a Self polymorphic parameter, where the concreteized
// implementatation will not. We need to walk back to the more general trait method
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_trait_selection/src/solve/assembly/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -714,7 +714,7 @@ impl<'tcx> EvalCtxt<'_, InferCtxt<'tcx>> {
};

// Do not consider built-in object impls for non-object-safe types.
if bounds.principal_def_id().is_some_and(|def_id| !tcx.check_is_object_safe(def_id)) {
if bounds.principal_def_id().is_some_and(|def_id| !tcx.is_object_safe(def_id)) {
return;
}

Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_trait_selection/src/solve/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -133,7 +133,7 @@ impl<'a, 'tcx> EvalCtxt<'a, InferCtxt<'tcx>> {
}

fn compute_object_safe_goal(&mut self, trait_def_id: DefId) -> QueryResult<'tcx> {
if self.interner().check_is_object_safe(trait_def_id) {
if self.interner().is_object_safe(trait_def_id) {
self.evaluate_added_goals_and_make_canonical_response(Certainty::Yes)
} else {
Err(NoSolution)
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_trait_selection/src/solve/trait_goals.rs
Original file line number Diff line number Diff line change
Expand Up @@ -789,7 +789,7 @@ impl<'tcx> EvalCtxt<'_, InferCtxt<'tcx>> {
let Goal { predicate: (a_ty, _), .. } = goal;

// Can only unsize to an object-safe trait.
if b_data.principal_def_id().is_some_and(|def_id| !tcx.check_is_object_safe(def_id)) {
if b_data.principal_def_id().is_some_and(|def_id| !tcx.is_object_safe(def_id)) {
return Err(NoSolution);
}

Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_trait_selection/src/traits/fulfill.rs
Original file line number Diff line number Diff line change
Expand Up @@ -411,7 +411,7 @@ impl<'a, 'tcx> ObligationProcessor for FulfillProcessor<'a, 'tcx> {
}

ty::PredicateKind::ObjectSafe(trait_def_id) => {
if !self.selcx.tcx().check_is_object_safe(trait_def_id) {
if !self.selcx.tcx().is_object_safe(trait_def_id) {
ProcessResult::Error(FulfillmentErrorCode::Select(Unimplemented))
} else {
ProcessResult::Changed(vec![])
Expand Down
85 changes: 5 additions & 80 deletions compiler/rustc_trait_selection/src/traits/object_safety.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ use super::elaborate;
use crate::infer::TyCtxtInferExt;
use crate::traits::query::evaluate_obligation::InferCtxtExt;
use crate::traits::{self, Obligation, ObligationCause};
use rustc_errors::{FatalError, MultiSpan};
use rustc_errors::FatalError;
use rustc_hir as hir;
use rustc_hir::def_id::DefId;
use rustc_middle::query::Providers;
Expand All @@ -23,7 +23,6 @@ use rustc_middle::ty::{
};
use rustc_middle::ty::{GenericArg, GenericArgs};
use rustc_middle::ty::{TypeVisitableExt, Upcast};
use rustc_session::lint::builtin::WHERE_CLAUSES_OBJECT_SAFETY;
use rustc_span::symbol::Symbol;
use rustc_span::Span;
use rustc_target::abi::Abi;
Expand Down Expand Up @@ -65,45 +64,14 @@ fn object_safety_violations(tcx: TyCtxt<'_>, trait_def_id: DefId) -> &'_ [Object
)
}

fn check_is_object_safe(tcx: TyCtxt<'_>, trait_def_id: DefId) -> bool {
let violations = tcx.object_safety_violations(trait_def_id);

if violations.is_empty() {
return true;
}

// If the trait contains any other violations, then let the error reporting path
// report it instead of emitting a warning here.
if violations.iter().all(|violation| {
matches!(
violation,
ObjectSafetyViolation::Method(_, MethodViolationCode::WhereClauseReferencesSelf, _)
)
}) {
for violation in violations {
if let ObjectSafetyViolation::Method(
_,
MethodViolationCode::WhereClauseReferencesSelf,
span,
) = violation
{
lint_object_unsafe_trait(tcx, *span, trait_def_id, violation);
}
}
return true;
}

false
fn is_object_safe(tcx: TyCtxt<'_>, trait_def_id: DefId) -> bool {
tcx.object_safety_violations(trait_def_id).is_empty()
}

/// We say a method is *vtable safe* if it can be invoked on a trait
/// object. Note that object-safe traits can have some
/// non-vtable-safe methods, so long as they require `Self: Sized` or
/// otherwise ensure that they cannot be used when `Self = Trait`.
///
/// [`MethodViolationCode::WhereClauseReferencesSelf`] is considered object safe due to backwards
/// compatibility, see <https://github.com/rust-lang/rust/issues/51443> and
/// [`WHERE_CLAUSES_OBJECT_SAFETY`].
pub fn is_vtable_safe_method(tcx: TyCtxt<'_>, trait_def_id: DefId, method: ty::AssocItem) -> bool {
debug_assert!(tcx.generics_of(trait_def_id).has_self);
debug!("is_vtable_safe_method({:?}, {:?})", trait_def_id, method);
Expand All @@ -112,9 +80,7 @@ pub fn is_vtable_safe_method(tcx: TyCtxt<'_>, trait_def_id: DefId, method: ty::A
return false;
}

virtual_call_violations_for_method(tcx, trait_def_id, method)
.iter()
.all(|v| matches!(v, MethodViolationCode::WhereClauseReferencesSelf))
virtual_call_violations_for_method(tcx, trait_def_id, method).is_empty()
}

fn object_safety_violations_for_trait(
Expand Down Expand Up @@ -163,47 +129,6 @@ fn object_safety_violations_for_trait(
violations
}

/// Lint object-unsafe trait.
fn lint_object_unsafe_trait(
tcx: TyCtxt<'_>,
span: Span,
trait_def_id: DefId,
violation: &ObjectSafetyViolation,
) {
// Using `CRATE_NODE_ID` is wrong, but it's hard to get a more precise id.
// It's also hard to get a use site span, so we use the method definition span.
tcx.node_span_lint(WHERE_CLAUSES_OBJECT_SAFETY, hir::CRATE_HIR_ID, span, |err| {
err.primary_message(format!(
"the trait `{}` cannot be made into an object",
tcx.def_path_str(trait_def_id)
));
let node = tcx.hir().get_if_local(trait_def_id);
let mut spans = MultiSpan::from_span(span);
if let Some(hir::Node::Item(item)) = node {
spans.push_span_label(item.ident.span, "this trait cannot be made into an object...");
spans.push_span_label(span, format!("...because {}", violation.error_msg()));
} else {
spans.push_span_label(
span,
format!(
"the trait cannot be made into an object because {}",
violation.error_msg()
),
);
};
err.span_note(
spans,
"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>",
);
if node.is_some() {
// Only provide the help if its a local trait, otherwise it's not
violation.solution().add_to(err);
}
});
}

fn sized_trait_bound_spans<'tcx>(
tcx: TyCtxt<'tcx>,
bounds: hir::GenericBounds<'tcx>,
Expand Down Expand Up @@ -929,7 +854,7 @@ pub fn contains_illegal_impl_trait_in_trait<'tcx>(
pub fn provide(providers: &mut Providers) {
*providers = Providers {
object_safety_violations,
check_is_object_safe,
is_object_safe,
generics_require_sized_self,
..*providers
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -870,7 +870,7 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
if let Some(principal) = data.principal() {
if !self.infcx.tcx.features().object_safe_for_dispatch {
principal.with_self_ty(self.tcx(), self_ty)
} else if self.tcx().check_is_object_safe(principal.def_id()) {
} else if self.tcx().is_object_safe(principal.def_id()) {
principal.with_self_ty(self.tcx(), self_ty)
} else {
return;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1222,7 +1222,7 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
// `T` -> `Trait`
(_, &ty::Dynamic(data, r, ty::Dyn)) => {
let mut object_dids = data.auto_traits().chain(data.principal_def_id());
if let Some(did) = object_dids.find(|did| !tcx.check_is_object_safe(*did)) {
if let Some(did) = object_dids.find(|did| !tcx.is_object_safe(*did)) {
return Err(TraitNotObjectSafe(did));
}

Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_trait_selection/src/traits/select/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -798,7 +798,7 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
}

ty::PredicateKind::ObjectSafe(trait_def_id) => {
if self.tcx().check_is_object_safe(trait_def_id) {
if self.tcx().is_object_safe(trait_def_id) {
Ok(EvaluatedToOk)
} else {
Ok(EvaluatedToErr)
Expand Down
2 changes: 1 addition & 1 deletion src/librustdoc/clean/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1451,7 +1451,7 @@ impl Trait {
tcx.trait_def(self.def_id).safety
}
pub(crate) fn is_object_safe(&self, tcx: TyCtxt<'_>) -> bool {
tcx.check_is_object_safe(self.def_id)
tcx.is_object_safe(self.def_id)
}
}

Expand Down
19 changes: 0 additions & 19 deletions src/tools/miri/tests/fail/issue-miri-2432.rs

This file was deleted.

15 changes: 0 additions & 15 deletions src/tools/miri/tests/fail/issue-miri-2432.stderr

This file was deleted.

Original file line number Diff line number Diff line change
@@ -1,22 +1,21 @@
#![feature(generic_const_exprs)]
#![allow(incomplete_features)]
#![deny(where_clauses_object_safety)]


const fn bar<T: ?Sized>() -> usize { 7 }

trait Foo {
fn test(&self) where [u8; bar::<Self>()]: Sized;
//~^ ERROR the trait `Foo` cannot be made into an object
//~| WARN this was previously accepted by the compiler but is being phased out
}

impl Foo for () {
fn test(&self) where [u8; bar::<Self>()]: Sized {}
}

fn use_dyn(v: &dyn Foo) {
//~^ ERROR the trait `Foo` cannot be made into an object
v.test();
//~^ ERROR the trait `Foo` cannot be made into an object
}

fn main() {}
Loading
Loading