Skip to content

Commit 90d6255

Browse files
borsbors
committed
Auto merge of #125380 - compiler-errors:wc-obj-safety, r=oli-obk
Make `WHERE_CLAUSES_OBJECT_SAFETY` a regular object safety violation #### The issue In #50781, we have known about unsound `where` clauses in function arguments: ```rust trait Impossible {} trait Foo { fn impossible(&self) where Self: Impossible; } impl Foo for &() { fn impossible(&self) where Self: Impossible, {} } // `where` clause satisfied for the object, meaning that the function now *looks* callable. impl Impossible for dyn Foo {} fn main() { let x: &dyn Foo = &&(); x.impossible(); } ``` ... which currently segfaults at runtime because we try to call a method in the vtable that doesn't exist. :( #### What did u change This PR removes the `WHERE_CLAUSES_OBJECT_SAFETY` lint and instead makes it a regular object safety violation. I choose to make this into a hard error immediately rather than a `deny` because of the time that has passed since this lint was authored, and the single (1) regression (see below). That means that it's OK to mention `where Self: Trait` where clauses in your trait, but making such a trait into a `dyn Trait` object will report an object safety violation just like `where Self: Sized`, etc. ```rust trait Impossible {} trait Foo { fn impossible(&self) where Self: Impossible; // <~ This definition is valid, just not object-safe. } impl Foo for &() { fn impossible(&self) where Self: Impossible, {} } fn main() { let x: &dyn Foo = &&(); // <~ THIS is where we emit an error. } ``` #### Regressions From a recent crater run, there's only one crate that relies on this behavior: #124305 (comment). The crate looks unmaintained and there seems to be no dependents. #### Further We may later choose to relax this (e.g. when the where clause is implied by the supertraits of the trait or something), but this is not something I propose to do in this FCP. For example, given: ``` trait Tr { fn f(&self) where Self: Blanket; } impl<T: ?Sized> Blanket for T {} ``` Proving that some placeholder `S` implements `S: Blanket` would be sufficient to prove that the same (blanket) impl applies for both `Concerete: Blanket` and `dyn Trait: Blanket`. Repeating here that I don't think we need to implement this behavior right now. ---- r? lcnr
2 parents 1689a5a + 511f1cf commit 90d6255

File tree

28 files changed

+92
-209
lines changed

28 files changed

+92
-209
lines changed

‎compiler/rustc_hir_analysis/src/check/wfcheck.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -881,7 +881,7 @@ fn check_object_unsafe_self_trait_by_name(tcx: TyCtxt<'_>, item: &hir::TraitItem
881881
_ => {}
882882
}
883883
if !trait_should_be_self.is_empty() {
884-
if tcx.check_is_object_safe(trait_def_id) {
884+
if tcx.is_object_safe(trait_def_id) {
885885
return;
886886
}
887887
let sugg = trait_should_be_self.iter().map(|span| (*span, "Self".to_string())).collect();

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

+1-1
Original file line numberDiff line numberDiff line change
@@ -191,7 +191,7 @@ fn check_object_overlap<'tcx>(
191191
});
192192

193193
for component_def_id in component_def_ids {
194-
if !tcx.check_is_object_safe(component_def_id) {
194+
if !tcx.is_object_safe(component_def_id) {
195195
// Without the 'object_safe_for_dispatch' feature this is an error
196196
// which will be reported by wfcheck. Ignore it here.
197197
// This is tested by `coherence-impl-trait-for-trait-object-safe.rs`.

‎compiler/rustc_hir_analysis/src/hir_ty_lowering/lint.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -182,7 +182,7 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ {
182182
// For recursive traits, don't downgrade the error. (#119652)
183183
is_downgradable = false;
184184
}
185-
tcx.check_is_object_safe(id)
185+
tcx.is_object_safe(id)
186186
}
187187
_ => false,
188188
})

‎compiler/rustc_interface/src/passes.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -832,7 +832,7 @@ fn analysis(tcx: TyCtxt<'_>, (): ()) -> Result<()> {
832832
let traits = tcx.traits(LOCAL_CRATE);
833833

834834
for &tr in traits {
835-
if !tcx.check_is_object_safe(tr) {
835+
if !tcx.is_object_safe(tr) {
836836
continue;
837837
}
838838

‎compiler/rustc_lint/src/multiple_supertrait_upcastable.rs

+2-2
Original file line numberDiff line numberDiff line change
@@ -38,10 +38,10 @@ declare_lint_pass!(MultipleSupertraitUpcastable => [MULTIPLE_SUPERTRAIT_UPCASTAB
3838
impl<'tcx> LateLintPass<'tcx> for MultipleSupertraitUpcastable {
3939
fn check_item(&mut self, cx: &LateContext<'tcx>, item: &'tcx hir::Item<'tcx>) {
4040
let def_id = item.owner_id.to_def_id();
41-
// NOTE(nbdd0121): use `object_safety_violations` instead of `check_is_object_safe` because
41+
// NOTE(nbdd0121): use `object_safety_violations` instead of `is_object_safe` because
4242
// the latter will report `where_clause_object_safety` lint.
4343
if let hir::ItemKind::Trait(_, _, _, _, _) = item.kind
44-
&& cx.tcx.object_safety_violations(def_id).is_empty()
44+
&& cx.tcx.is_object_safe(def_id)
4545
{
4646
let direct_super_traits_iter = cx
4747
.tcx

‎compiler/rustc_lint_defs/src/builtin.rs

-42
Original file line numberDiff line numberDiff line change
@@ -136,7 +136,6 @@ declare_lint_pass! {
136136
USELESS_DEPRECATED,
137137
WARNINGS,
138138
WASM_C_ABI,
139-
WHERE_CLAUSES_OBJECT_SAFETY,
140139
WRITES_THROUGH_IMMUTABLE_POINTER,
141140
// tidy-alphabetical-end
142141
]
@@ -2093,47 +2092,6 @@ declare_lint! {
20932092
"detects labels that are never used"
20942093
}
20952094

2096-
declare_lint! {
2097-
/// The `where_clauses_object_safety` lint detects for [object safety] of
2098-
/// [where clauses].
2099-
///
2100-
/// [object safety]: https://doc.rust-lang.org/reference/items/traits.html#object-safety
2101-
/// [where clauses]: https://doc.rust-lang.org/reference/items/generics.html#where-clauses
2102-
///
2103-
/// ### Example
2104-
///
2105-
/// ```rust,no_run
2106-
/// trait Trait {}
2107-
///
2108-
/// trait X { fn foo(&self) where Self: Trait; }
2109-
///
2110-
/// impl X for () { fn foo(&self) {} }
2111-
///
2112-
/// impl Trait for dyn X {}
2113-
///
2114-
/// // Segfault at opt-level 0, SIGILL otherwise.
2115-
/// pub fn main() { <dyn X as X>::foo(&()); }
2116-
/// ```
2117-
///
2118-
/// {{produces}}
2119-
///
2120-
/// ### Explanation
2121-
///
2122-
/// The compiler previously allowed these object-unsafe bounds, which was
2123-
/// incorrect. This is a [future-incompatible] lint to transition this to
2124-
/// a hard error in the future. See [issue #51443] for more details.
2125-
///
2126-
/// [issue #51443]: https://github.com/rust-lang/rust/issues/51443
2127-
/// [future-incompatible]: ../index.md#future-incompatible-lints
2128-
pub WHERE_CLAUSES_OBJECT_SAFETY,
2129-
Warn,
2130-
"checks the object safety of where clauses",
2131-
@future_incompatible = FutureIncompatibleInfo {
2132-
reason: FutureIncompatibilityReason::FutureReleaseErrorDontReportInDeps,
2133-
reference: "issue #51443 <https://github.com/rust-lang/rust/issues/51443>",
2134-
};
2135-
}
2136-
21372095
declare_lint! {
21382096
/// The `proc_macro_derive_resolution_fallback` lint detects proc macro
21392097
/// derives using inaccessible names from parent modules.

‎compiler/rustc_middle/src/query/mod.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -1309,7 +1309,7 @@ rustc_queries! {
13091309
query object_safety_violations(trait_id: DefId) -> &'tcx [ObjectSafetyViolation] {
13101310
desc { |tcx| "determining object safety of trait `{}`", tcx.def_path_str(trait_id) }
13111311
}
1312-
query check_is_object_safe(trait_id: DefId) -> bool {
1312+
query is_object_safe(trait_id: DefId) -> bool {
13131313
desc { |tcx| "checking if trait `{}` is object safe", tcx.def_path_str(trait_id) }
13141314
}
13151315

‎compiler/rustc_sanitizers/src/cfi/typeid/itanium_cxx_abi/transform.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -367,7 +367,7 @@ pub fn transform_instance<'tcx>(
367367
let trait_method = tcx.associated_item(method_id);
368368
let trait_id = trait_ref.skip_binder().def_id;
369369
if traits::is_vtable_safe_method(tcx, trait_id, trait_method)
370-
&& tcx.object_safety_violations(trait_id).is_empty()
370+
&& tcx.is_object_safe(trait_id)
371371
{
372372
// Trait methods will have a Self polymorphic parameter, where the concreteized
373373
// implementatation will not. We need to walk back to the more general trait method

‎compiler/rustc_trait_selection/src/solve/assembly/mod.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -714,7 +714,7 @@ impl<'tcx> EvalCtxt<'_, InferCtxt<'tcx>> {
714714
};
715715

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

‎compiler/rustc_trait_selection/src/solve/mod.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -133,7 +133,7 @@ impl<'a, 'tcx> EvalCtxt<'a, InferCtxt<'tcx>> {
133133
}
134134

135135
fn compute_object_safe_goal(&mut self, trait_def_id: DefId) -> QueryResult<'tcx> {
136-
if self.interner().check_is_object_safe(trait_def_id) {
136+
if self.interner().is_object_safe(trait_def_id) {
137137
self.evaluate_added_goals_and_make_canonical_response(Certainty::Yes)
138138
} else {
139139
Err(NoSolution)

‎compiler/rustc_trait_selection/src/solve/trait_goals.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -789,7 +789,7 @@ impl<'tcx> EvalCtxt<'_, InferCtxt<'tcx>> {
789789
let Goal { predicate: (a_ty, _), .. } = goal;
790790

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

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

+1-1
Original file line numberDiff line numberDiff line change
@@ -421,7 +421,7 @@ impl<'a, 'tcx> ObligationProcessor for FulfillProcessor<'a, 'tcx> {
421421
}
422422

423423
ty::PredicateKind::ObjectSafe(trait_def_id) => {
424-
if !self.selcx.tcx().check_is_object_safe(trait_def_id) {
424+
if !self.selcx.tcx().is_object_safe(trait_def_id) {
425425
ProcessResult::Error(FulfillmentErrorCode::Select(Unimplemented))
426426
} else {
427427
ProcessResult::Changed(vec![])

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

+5-80
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ use super::elaborate;
1313
use crate::infer::TyCtxtInferExt;
1414
use crate::traits::query::evaluate_obligation::InferCtxtExt;
1515
use crate::traits::{self, Obligation, ObligationCause};
16-
use rustc_errors::{FatalError, MultiSpan};
16+
use rustc_errors::FatalError;
1717
use rustc_hir as hir;
1818
use rustc_hir::def_id::DefId;
1919
use rustc_middle::query::Providers;
@@ -23,7 +23,6 @@ use rustc_middle::ty::{
2323
};
2424
use rustc_middle::ty::{GenericArg, GenericArgs};
2525
use rustc_middle::ty::{TypeVisitableExt, Upcast};
26-
use rustc_session::lint::builtin::WHERE_CLAUSES_OBJECT_SAFETY;
2726
use rustc_span::symbol::Symbol;
2827
use rustc_span::Span;
2928
use rustc_target::abi::Abi;
@@ -65,45 +64,14 @@ fn object_safety_violations(tcx: TyCtxt<'_>, trait_def_id: DefId) -> &'_ [Object
6564
)
6665
}
6766

68-
fn check_is_object_safe(tcx: TyCtxt<'_>, trait_def_id: DefId) -> bool {
69-
let violations = tcx.object_safety_violations(trait_def_id);
70-
71-
if violations.is_empty() {
72-
return true;
73-
}
74-
75-
// If the trait contains any other violations, then let the error reporting path
76-
// report it instead of emitting a warning here.
77-
if violations.iter().all(|violation| {
78-
matches!(
79-
violation,
80-
ObjectSafetyViolation::Method(_, MethodViolationCode::WhereClauseReferencesSelf, _)
81-
)
82-
}) {
83-
for violation in violations {
84-
if let ObjectSafetyViolation::Method(
85-
_,
86-
MethodViolationCode::WhereClauseReferencesSelf,
87-
span,
88-
) = violation
89-
{
90-
lint_object_unsafe_trait(tcx, *span, trait_def_id, violation);
91-
}
92-
}
93-
return true;
94-
}
95-
96-
false
67+
fn is_object_safe(tcx: TyCtxt<'_>, trait_def_id: DefId) -> bool {
68+
tcx.object_safety_violations(trait_def_id).is_empty()
9769
}
9870

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

115-
virtual_call_violations_for_method(tcx, trait_def_id, method)
116-
.iter()
117-
.all(|v| matches!(v, MethodViolationCode::WhereClauseReferencesSelf))
83+
virtual_call_violations_for_method(tcx, trait_def_id, method).is_empty()
11884
}
11985

12086
fn object_safety_violations_for_trait(
@@ -163,47 +129,6 @@ fn object_safety_violations_for_trait(
163129
violations
164130
}
165131

166-
/// Lint object-unsafe trait.
167-
fn lint_object_unsafe_trait(
168-
tcx: TyCtxt<'_>,
169-
span: Span,
170-
trait_def_id: DefId,
171-
violation: &ObjectSafetyViolation,
172-
) {
173-
// Using `CRATE_NODE_ID` is wrong, but it's hard to get a more precise id.
174-
// It's also hard to get a use site span, so we use the method definition span.
175-
tcx.node_span_lint(WHERE_CLAUSES_OBJECT_SAFETY, hir::CRATE_HIR_ID, span, |err| {
176-
err.primary_message(format!(
177-
"the trait `{}` cannot be made into an object",
178-
tcx.def_path_str(trait_def_id)
179-
));
180-
let node = tcx.hir().get_if_local(trait_def_id);
181-
let mut spans = MultiSpan::from_span(span);
182-
if let Some(hir::Node::Item(item)) = node {
183-
spans.push_span_label(item.ident.span, "this trait cannot be made into an object...");
184-
spans.push_span_label(span, format!("...because {}", violation.error_msg()));
185-
} else {
186-
spans.push_span_label(
187-
span,
188-
format!(
189-
"the trait cannot be made into an object because {}",
190-
violation.error_msg()
191-
),
192-
);
193-
};
194-
err.span_note(
195-
spans,
196-
"for a trait to be \"object safe\" it needs to allow building a vtable to allow the \
197-
call to be resolvable dynamically; for more information visit \
198-
<https://doc.rust-lang.org/reference/items/traits.html#object-safety>",
199-
);
200-
if node.is_some() {
201-
// Only provide the help if its a local trait, otherwise it's not
202-
violation.solution().add_to(err);
203-
}
204-
});
205-
}
206-
207132
fn sized_trait_bound_spans<'tcx>(
208133
tcx: TyCtxt<'tcx>,
209134
bounds: hir::GenericBounds<'tcx>,
@@ -929,7 +854,7 @@ pub fn contains_illegal_impl_trait_in_trait<'tcx>(
929854
pub fn provide(providers: &mut Providers) {
930855
*providers = Providers {
931856
object_safety_violations,
932-
check_is_object_safe,
857+
is_object_safe,
933858
generics_require_sized_self,
934859
..*providers
935860
};

‎compiler/rustc_trait_selection/src/traits/select/candidate_assembly.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -870,7 +870,7 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
870870
if let Some(principal) = data.principal() {
871871
if !self.infcx.tcx.features().object_safe_for_dispatch {
872872
principal.with_self_ty(self.tcx(), self_ty)
873-
} else if self.tcx().check_is_object_safe(principal.def_id()) {
873+
} else if self.tcx().is_object_safe(principal.def_id()) {
874874
principal.with_self_ty(self.tcx(), self_ty)
875875
} else {
876876
return;

‎compiler/rustc_trait_selection/src/traits/select/confirmation.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -1222,7 +1222,7 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
12221222
// `T` -> `Trait`
12231223
(_, &ty::Dynamic(data, r, ty::Dyn)) => {
12241224
let mut object_dids = data.auto_traits().chain(data.principal_def_id());
1225-
if let Some(did) = object_dids.find(|did| !tcx.check_is_object_safe(*did)) {
1225+
if let Some(did) = object_dids.find(|did| !tcx.is_object_safe(*did)) {
12261226
return Err(TraitNotObjectSafe(did));
12271227
}
12281228

‎compiler/rustc_trait_selection/src/traits/select/mod.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -798,7 +798,7 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
798798
}
799799

800800
ty::PredicateKind::ObjectSafe(trait_def_id) => {
801-
if self.tcx().check_is_object_safe(trait_def_id) {
801+
if self.tcx().is_object_safe(trait_def_id) {
802802
Ok(EvaluatedToOk)
803803
} else {
804804
Ok(EvaluatedToErr)

‎src/librustdoc/clean/types.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -1451,7 +1451,7 @@ impl Trait {
14511451
tcx.trait_def(self.def_id).safety
14521452
}
14531453
pub(crate) fn is_object_safe(&self, tcx: TyCtxt<'_>) -> bool {
1454-
tcx.check_is_object_safe(self.def_id)
1454+
tcx.is_object_safe(self.def_id)
14551455
}
14561456
}
14571457

‎src/tools/miri/tests/fail/issue-miri-2432.rs

-19
This file was deleted.

‎src/tools/miri/tests/fail/issue-miri-2432.stderr

-15
This file was deleted.

‎tests/ui/const-generics/generic_const_exprs/object-safety-err-where-bounds.rs

+2-3
Original file line numberDiff line numberDiff line change
@@ -1,22 +1,21 @@
11
#![feature(generic_const_exprs)]
22
#![allow(incomplete_features)]
3-
#![deny(where_clauses_object_safety)]
43

54

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

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

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

1815
fn use_dyn(v: &dyn Foo) {
16+
//~^ ERROR the trait `Foo` cannot be made into an object
1917
v.test();
18+
//~^ ERROR the trait `Foo` cannot be made into an object
2019
}
2120

2221
fn main() {}

0 commit comments

Comments
 (0)