Skip to content

Commit 69b13e4

Browse files
authored
Rollup merge of #141347 - lcnr:lets-make-it-unsound-3, r=compiler-errors
incorrectly prefer builtin `dyn` impls :3 This makes #57893 slightly more exploitable with the new solver. It's still strictly better than the old solver and the underlying unsoundness persists in the new one even without this preference. Properly fixing #57893 is something we've been looking at more deeply recently and discussed at the [Types Meetup during the All-Hands](https://hackmd.io/rz-4ghMzTb2wXOkdLKHaHw#Dyn-traits). Whatever approach we'll end up deciding on will likely require a fairly long transition period and some significant further design work. This should not block `-Znext-solver`. fixesrust-lang/trait-system-refactor-initiative#183 r? `@compiler-errors` cc `@rust-lang/types`
2 parents b9c6b33 + 5d9141c commit 69b13e4

2 files changed

Lines changed: 75 additions & 2 deletions

File tree

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

Lines changed: 42 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,15 +9,15 @@ use rustc_type_ir::{
99
selfas ty,Interner,Movability,TraitPredicate,TypeVisitableExtas _,TypingMode,
1010
Upcastas _, elaborate,
1111
};
12-
use tracing::{instrument, trace};
12+
use tracing::{debug,instrument, trace};
1313

1414
usecrate::delegate::SolverDelegate;
1515
usecrate::solve::assembly::structural_traits::{self,AsyncCallableRelevantTypes};
1616
usecrate::solve::assembly::{self,AllowInferenceConstraints,AssembleCandidatesFrom,Candidate};
1717
usecrate::solve::inspect::ProbeKind;
1818
usecrate::solve::{
1919
BuiltinImplSource,CandidateSource,Certainty,EvalCtxt,Goal,GoalSource,MaybeCause,
20-
NoSolution,ParamEnvSource,QueryResult,
20+
NoSolution,ParamEnvSource,QueryResult, has_only_region_constraints,
2121
};
2222

2323
impl<D,I> assembly::GoalKind<D>forTraitPredicate<I>
@@ -1253,6 +1253,45 @@ where
12531253
D:SolverDelegate<Interner = I>,
12541254
I:Interner,
12551255
{
1256+
/// FIXME(#57893): For backwards compatability with the old trait solver implementation,
1257+
/// we need to handle overlap between builtin and user-written impls for trait objects.
1258+
///
1259+
/// This overlap is unsound in general and something which we intend to fix separately.
1260+
/// To avoid blocking the stabilization of the trait solver, we add this hack to avoid
1261+
/// breakage in cases which are *mostly fine*™. Importantly, this preference is strictly
1262+
/// weaker than the old behavior.
1263+
///
1264+
/// We only prefer builtin over user-written impls if there are no inference constraints.
1265+
/// Importantly, we also only prefer the builtin impls for trait goals, and not during
1266+
/// normalization. This means the only case where this special-case results in exploitable
1267+
/// unsoundness should be lifetime dependent user-written impls.
1268+
pub(super)fnunsound_prefer_builtin_dyn_impl(&mutself,candidates:&mutVec<Candidate<I>>){
1269+
matchself.typing_mode(){
1270+
TypingMode::Coherence => return,
1271+
TypingMode::Analysis{ .. }
1272+
| TypingMode::Borrowck{ .. }
1273+
| TypingMode::PostBorrowckAnalysis{ .. }
1274+
| TypingMode::PostAnalysis => {}
1275+
}
1276+
1277+
if candidates
1278+
.iter()
1279+
.find(|c| {
1280+
matches!(c.source,CandidateSource::BuiltinImpl(BuiltinImplSource::Object(_)))
1281+
})
1282+
.is_some_and(|c| has_only_region_constraints(c.result))
1283+
{
1284+
candidates.retain(|c| {
1285+
ifmatches!(c.source,CandidateSource::Impl(_)){
1286+
debug!(?c,"unsoundly dropping impl in favor of builtin dyn-candidate");
1287+
false
1288+
}else{
1289+
true
1290+
}
1291+
});
1292+
}
1293+
}
1294+
12561295
#[instrument(level = "debug", skip(self), ret)]
12571296
pub(super)fnmerge_trait_candidates(
12581297
&mutself,
@@ -1313,6 +1352,7 @@ where
13131352
}
13141353

13151354
self.filter_specialized_impls(AllowInferenceConstraints::No,&mut candidates);
1355+
self.unsound_prefer_builtin_dyn_impl(&mut candidates);
13161356

13171357
// If there are *only* global where bounds, then make sure to return that this
13181358
// is still reported as being proven-via the param-env so that rigid projections
Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
1+
//@ check-pass
2+
//@ revisions: current next
3+
//@[next] compile-flags: -Znext-solver
4+
//@ ignore-compare-mode-next-solver (explicit revisions)
5+
6+
// A regression test for trait-system-refactor-initiative#183. While
7+
// this concrete instance is likely not practically unsound, the general
8+
// pattern is, see #57893.
9+
10+
use std::any::TypeId;
11+
12+
unsafetraitTidAble<'a>:Tid<'a>{}
13+
traitTidExt<'a>:Tid<'a>{
14+
fndowncast_box(self:Box<Self>){
15+
loop{}
16+
}
17+
}
18+
19+
impl<'a,X: ?Sized + Tid<'a>>TidExt<'a>forX{}
20+
21+
unsafetraitTid<'a>:'a{}
22+
23+
unsafeimpl<'a,T: ?Sized + TidAble<'a>>Tid<'a>forT{}
24+
25+
impl<'a>dynTid<'a> + 'a{
26+
fndowncast_any_box(self:Box<Self>){
27+
self.downcast_box();
28+
}
29+
}
30+
31+
unsafeimpl<'a>TidAble<'a>fordynTid<'a> + 'a{}
32+
33+
fnmain(){}

0 commit comments

Comments
 (0)