Skip to content

Commit 4b5f274

Browse files
committed
make evaluation track whether outlives relationships mattered
Previously, evaluation ignored outlives relationships. Since we using evaluation to skip the "normal" trait selection (which enforces outlives relationships) this led to incorrect results in some cases.
1 parent 79efed8 commit 4b5f274

File tree

10 files changed

+129
-126
lines changed

10 files changed

+129
-126
lines changed

src/librustc/infer/mod.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -1455,7 +1455,7 @@ impl<'a, 'gcx, 'tcx> InferCtxt<'a, 'gcx, 'tcx> {
14551455
// rightly refuses to work with inference variables, but
14561456
// moves_by_default has a cache, which we want to use in other
14571457
// cases.
1458-
!traits::type_known_to_meet_bound(self, param_env, ty, copy_def_id, span)
1458+
!traits::type_known_to_meet_bound_modulo_regions(self, param_env, ty, copy_def_id, span)
14591459
}
14601460

14611461
/// Obtains the latest type of the given closure; this may be a

src/librustc/traits/fulfill.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -282,7 +282,7 @@ impl<'a, 'b, 'gcx, 'tcx> ObligationProcessor for FulfillProcessor<'a, 'b, 'gcx,
282282
if data.is_global() {
283283
// no type variables present, can use evaluation for better caching.
284284
// FIXME: consider caching errors too.
285-
if self.selcx.infcx().predicate_must_hold(&obligation) {
285+
if self.selcx.infcx().predicate_must_hold_considering_regions(&obligation) {
286286
debug!("selecting trait `{:?}` at depth {} evaluated to holds",
287287
data, obligation.recursion_depth);
288288
return ProcessResult::Changed(vec![])

src/librustc/traits/mod.rs

+11-11
Original file line numberDiff line numberDiff line change
@@ -628,14 +628,14 @@ pub fn predicates_for_generics<'tcx>(cause: ObligationCause<'tcx>,
628628
/// `bound` or is not known to meet bound (note that this is
629629
/// conservative towards *no impl*, which is the opposite of the
630630
/// `evaluate` methods).
631-
pub fn type_known_to_meet_bound<'a, 'gcx, 'tcx>(infcx: &InferCtxt<'a, 'gcx, 'tcx>,
632-
param_env: ty::ParamEnv<'tcx>,
633-
ty: Ty<'tcx>,
634-
def_id: DefId,
635-
span: Span)
636-
-> bool
637-
{
638-
debug!("type_known_to_meet_bound(ty={:?}, bound={:?})",
631+
pub fn type_known_to_meet_bound_modulo_regions<'a, 'gcx, 'tcx>(
632+
infcx: &InferCtxt<'a, 'gcx, 'tcx>,
633+
param_env: ty::ParamEnv<'tcx>,
634+
ty: Ty<'tcx>,
635+
def_id: DefId,
636+
span: Span,
637+
) -> bool {
638+
debug!("type_known_to_meet_bound_modulo_regions(ty={:?}, bound={:?})",
639639
ty,
640640
infcx.tcx.item_path_str(def_id));
641641

@@ -650,7 +650,7 @@ pub fn type_known_to_meet_bound<'a, 'gcx, 'tcx>(infcx: &InferCtxt<'a, 'gcx, 'tcx
650650
predicate: trait_ref.to_predicate(),
651651
};
652652

653-
let result = infcx.predicate_must_hold(&obligation);
653+
let result = infcx.predicate_must_hold_modulo_regions(&obligation);
654654
debug!("type_known_to_meet_ty={:?} bound={} => {:?}",
655655
ty, infcx.tcx.item_path_str(def_id), result);
656656

@@ -677,13 +677,13 @@ pub fn type_known_to_meet_bound<'a, 'gcx, 'tcx>(infcx: &InferCtxt<'a, 'gcx, 'tcx
677677
// assume it is move; linear is always ok.
678678
match fulfill_cx.select_all_or_error(infcx) {
679679
Ok(()) => {
680-
debug!("type_known_to_meet_bound: ty={:?} bound={} success",
680+
debug!("type_known_to_meet_bound_modulo_regions: ty={:?} bound={} success",
681681
ty,
682682
infcx.tcx.item_path_str(def_id));
683683
true
684684
}
685685
Err(e) => {
686-
debug!("type_known_to_meet_bound: ty={:?} bound={} errors={:?}",
686+
debug!("type_known_to_meet_bound_modulo_regions: ty={:?} bound={} errors={:?}",
687687
ty,
688688
infcx.tcx.item_path_str(def_id),
689689
e);

src/librustc/traits/object_safety.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -568,7 +568,7 @@ impl<'a, 'tcx> TyCtxt<'a, 'tcx, 'tcx> {
568568

569569
self.infer_ctxt().enter(|ref infcx| {
570570
// the receiver is dispatchable iff the obligation holds
571-
infcx.predicate_must_hold(&obligation)
571+
infcx.predicate_must_hold_modulo_regions(&obligation)
572572
})
573573
}
574574

src/librustc/traits/query/evaluate_obligation.rs

+17-2
Original file line numberDiff line numberDiff line change
@@ -16,11 +16,26 @@ impl<'cx, 'gcx, 'tcx> InferCtxt<'cx, 'gcx, 'tcx> {
1616
/// Evaluates whether the predicate can be satisfied in the given
1717
/// `ParamEnv`, and returns `false` if not certain. However, this is
1818
/// not entirely accurate if inference variables are involved.
19-
pub fn predicate_must_hold(
19+
///
20+
/// This version may conservatively fail when outlives obligations
21+
/// are required.
22+
pub fn predicate_must_hold_considering_regions(
2023
&self,
2124
obligation: &PredicateObligation<'tcx>,
2225
) -> bool {
23-
self.evaluate_obligation_no_overflow(obligation) == EvaluationResult::EvaluatedToOk
26+
self.evaluate_obligation_no_overflow(obligation).must_apply_considering_regions()
27+
}
28+
29+
/// Evaluates whether the predicate can be satisfied in the given
30+
/// `ParamEnv`, and returns `false` if not certain. However, this is
31+
/// not entirely accurate if inference variables are involved.
32+
///
33+
/// This version ignores all outlives constraints.
34+
pub fn predicate_must_hold_modulo_regions(
35+
&self,
36+
obligation: &PredicateObligation<'tcx>,
37+
) -> bool {
38+
self.evaluate_obligation_no_overflow(obligation).must_apply_modulo_regions()
2439
}
2540

2641
/// Evaluate a given predicate, capturing overflow and propagating it back.

src/librustc/traits/select.rs

+32-91
Original file line numberDiff line numberDiff line change
@@ -327,7 +327,8 @@ enum BuiltinImplConditions<'tcx> {
327327
/// evaluations.
328328
///
329329
/// The evaluation results are ordered:
330-
/// - `EvaluatedToOk` implies `EvaluatedToAmbig` implies `EvaluatedToUnknown`
330+
/// - `EvaluatedToOk` implies `EvaluatedToOkModuloRegions`
331+
/// implies `EvaluatedToAmbig` implies `EvaluatedToUnknown`
331332
/// - `EvaluatedToErr` implies `EvaluatedToRecur`
332333
/// - the "union" of evaluation results is equal to their maximum -
333334
/// all the "potential success" candidates can potentially succeed,
@@ -336,6 +337,8 @@ enum BuiltinImplConditions<'tcx> {
336337
pub enum EvaluationResult {
337338
/// Evaluation successful
338339
EvaluatedToOk,
340+
/// Evaluation successful, but there were unevaluated region obligations
341+
EvaluatedToOkModuloRegions,
339342
/// Evaluation is known to be ambiguous - it *might* hold for some
340343
/// assignment of inference variables, but it might not.
341344
///
@@ -399,9 +402,23 @@ pub enum EvaluationResult {
399402
}
400403

401404
impl EvaluationResult {
405+
/// True if this evaluation result is known to apply, even
406+
/// considering outlives constraints.
407+
pub fn must_apply_considering_regions(self) -> bool {
408+
self == EvaluatedToOk
409+
}
410+
411+
/// True if this evaluation result is known to apply, ignoring
412+
/// outlives constraints.
413+
pub fn must_apply_modulo_regions(self) -> bool {
414+
self <= EvaluatedToOkModuloRegions
415+
}
416+
402417
pub fn may_apply(self) -> bool {
403418
match self {
404-
EvaluatedToOk | EvaluatedToAmbig | EvaluatedToUnknown => true,
419+
EvaluatedToOk | EvaluatedToOkModuloRegions | EvaluatedToAmbig | EvaluatedToUnknown => {
420+
true
421+
}
405422

406423
EvaluatedToErr | EvaluatedToRecur => false,
407424
}
@@ -411,13 +428,14 @@ impl EvaluationResult {
411428
match self {
412429
EvaluatedToUnknown | EvaluatedToRecur => true,
413430

414-
EvaluatedToOk | EvaluatedToAmbig | EvaluatedToErr => false,
431+
EvaluatedToOk | EvaluatedToOkModuloRegions | EvaluatedToAmbig | EvaluatedToErr => false,
415432
}
416433
}
417434
}
418435

419436
impl_stable_hash_for!(enum self::EvaluationResult {
420437
EvaluatedToOk,
438+
EvaluatedToOkModuloRegions,
421439
EvaluatedToAmbig,
422440
EvaluatedToUnknown,
423441
EvaluatedToRecur,
@@ -686,92 +704,10 @@ impl<'cx, 'gcx, 'tcx> SelectionContext<'cx, 'gcx, 'tcx> {
686704
None => Ok(EvaluatedToAmbig),
687705
},
688706

689-
ty::Predicate::TypeOutlives(ref binder) => {
690-
assert!(!binder.has_escaping_bound_vars());
691-
// Check if the type has higher-ranked vars.
692-
if binder.skip_binder().0.has_escaping_bound_vars() {
693-
// If so, this obligation is an error (for now). Eventually we should be
694-
// able to support additional cases here, like `for<'a> &'a str: 'a`.
695-
696-
// NOTE: this hack is implemented in both trait fulfillment and
697-
// evaluation. If you fix it in one place, make sure you fix it
698-
// in the other.
699-
700-
// We don't want to allow this sort of reasoning in intercrate
701-
// mode, for backwards-compatibility reasons.
702-
if self.intercrate.is_some() {
703-
Ok(EvaluatedToAmbig)
704-
} else {
705-
Ok(EvaluatedToErr)
706-
}
707-
} else {
708-
// If the type has no late bound vars, then if we assign all
709-
// the inference variables in it to be 'static, then the type
710-
// will be 'static itself.
711-
//
712-
// Therefore, `staticize(T): 'a` holds for any `'a`, so this
713-
// obligation is fulfilled. Because evaluation works with
714-
// staticized types (yes I know this is involved with #21974),
715-
// we are 100% OK here.
716-
Ok(EvaluatedToOk)
717-
}
718-
}
719-
720-
ty::Predicate::RegionOutlives(ref binder) => {
721-
let ty::OutlivesPredicate(r_a, r_b) = binder.skip_binder();
722-
723-
if r_a == r_b {
724-
// for<'a> 'a: 'a. OK
725-
Ok(EvaluatedToOk)
726-
} else if **r_a == ty::ReStatic {
727-
// 'static: 'x always holds.
728-
//
729-
// This special case is handled somewhat inconsistently - if we
730-
// have an inference variable that is supposed to be equal to
731-
// `'static`, then we don't allow it to be equated to an LBR,
732-
// but if we have a literal `'static`, then we *do*.
733-
//
734-
// This is actually consistent with how our region inference works.
735-
//
736-
// It would appear that this sort of inconsistency would
737-
// cause "instability" problems with evaluation caching. However,
738-
// evaluation caching is only for trait predicates, and when
739-
// trait predicates create nested obligations, they contain
740-
// inference variables for all the regions in the trait - the
741-
// only way this codepath can be reached from trait predicate
742-
// evaluation is when the user typed an explicit `where 'static: 'a`
743-
// lifetime bound (in which case we want to return EvaluatedToOk).
744-
//
745-
// If we ever want to handle inference variables that might be
746-
// equatable with ReStatic, we need to make sure we are not confused by
747-
// technically-allowed-by-RFC-447-but-probably-should-not-be
748-
// impls such as
749-
// ```Rust
750-
// impl<'a, 's, T> X<'s> for T where T: Debug + 'a, 'a: 's
751-
// ```
752-
Ok(EvaluatedToOk)
753-
} else if r_a.is_late_bound() || r_b.is_late_bound() {
754-
// There is no current way to prove `for<'a> 'a: 'x`
755-
// unless `'a = 'x`, because there are no bounds involving
756-
// lifetimes.
757-
758-
// It might be possible to prove `for<'a> 'x: 'a` by forcing `'x`
759-
// to be `'static`. However, this is not currently done by type
760-
// inference unless `'x` is literally ReStatic. See the comment
761-
// above.
762-
763-
// We don't want to allow this sort of reasoning in intercrate
764-
// mode, for backwards-compatibility reasons.
765-
if self.intercrate.is_some() {
766-
Ok(EvaluatedToAmbig)
767-
} else {
768-
Ok(EvaluatedToErr)
769-
}
770-
} else {
771-
// Relating 2 inference variable regions. These will
772-
// always hold if our query is "staticized".
773-
Ok(EvaluatedToOk)
774-
}
707+
ty::Predicate::TypeOutlives(..) | ty::Predicate::RegionOutlives(..) => {
708+
// we do not consider region relationships when
709+
// evaluating trait matches
710+
Ok(EvaluatedToOkModuloRegions)
775711
}
776712

777713
ty::Predicate::ObjectSafe(trait_def_id) => {
@@ -985,6 +921,11 @@ impl<'cx, 'gcx, 'tcx> SelectionContext<'cx, 'gcx, 'tcx> {
985921
{
986922
debug!("evaluate_stack({:?}) --> recursive", stack.fresh_trait_ref);
987923

924+
// Subtle: when checking for a coinductive cycle, we do
925+
// not compare using the "freshened trait refs" (which
926+
// have erased regions) but rather the fully explicit
927+
// trait refs. This is important because it's only a cycle
928+
// if the regions match exactly.
988929
let cycle = stack.iter().skip(1).take(rec_index + 1);
989930
let cycle = cycle.map(|stack| ty::Predicate::Trait(stack.obligation.predicate));
990931
if self.coinductive_match(cycle) {
@@ -2324,7 +2265,7 @@ impl<'cx, 'gcx, 'tcx> SelectionContext<'cx, 'gcx, 'tcx> {
23242265
// See if we can toss out `victim` based on specialization.
23252266
// This requires us to know *for sure* that the `other` impl applies
23262267
// i.e., EvaluatedToOk:
2327-
if other.evaluation == EvaluatedToOk {
2268+
if other.evaluation.must_apply_modulo_regions() {
23282269
match victim.candidate {
23292270
ImplCandidate(victim_def) => {
23302271
let tcx = self.tcx().global_tcx();
@@ -2351,7 +2292,7 @@ impl<'cx, 'gcx, 'tcx> SelectionContext<'cx, 'gcx, 'tcx> {
23512292
ParamCandidate(ref cand) => {
23522293
// Prefer these to a global where-clause bound
23532294
// (see issue #50825)
2354-
is_global(cand) && other.evaluation == EvaluatedToOk
2295+
is_global(cand) && other.evaluation.must_apply_modulo_regions()
23552296
}
23562297
_ => false,
23572298
}

src/librustc/ty/util.rs

+21-15
Original file line numberDiff line numberDiff line change
@@ -851,11 +851,13 @@ fn is_copy_raw<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>,
851851
let (param_env, ty) = query.into_parts();
852852
let trait_def_id = tcx.require_lang_item(lang_items::CopyTraitLangItem);
853853
tcx.infer_ctxt()
854-
.enter(|infcx| traits::type_known_to_meet_bound(&infcx,
855-
param_env,
856-
ty,
857-
trait_def_id,
858-
DUMMY_SP))
854+
.enter(|infcx| traits::type_known_to_meet_bound_modulo_regions(
855+
&infcx,
856+
param_env,
857+
ty,
858+
trait_def_id,
859+
DUMMY_SP,
860+
))
859861
}
860862

861863
fn is_sized_raw<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>,
@@ -865,11 +867,13 @@ fn is_sized_raw<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>,
865867
let (param_env, ty) = query.into_parts();
866868
let trait_def_id = tcx.require_lang_item(lang_items::SizedTraitLangItem);
867869
tcx.infer_ctxt()
868-
.enter(|infcx| traits::type_known_to_meet_bound(&infcx,
869-
param_env,
870-
ty,
871-
trait_def_id,
872-
DUMMY_SP))
870+
.enter(|infcx| traits::type_known_to_meet_bound_modulo_regions(
871+
&infcx,
872+
param_env,
873+
ty,
874+
trait_def_id,
875+
DUMMY_SP,
876+
))
873877
}
874878

875879
fn is_freeze_raw<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>,
@@ -879,11 +883,13 @@ fn is_freeze_raw<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>,
879883
let (param_env, ty) = query.into_parts();
880884
let trait_def_id = tcx.require_lang_item(lang_items::FreezeTraitLangItem);
881885
tcx.infer_ctxt()
882-
.enter(|infcx| traits::type_known_to_meet_bound(&infcx,
883-
param_env,
884-
ty,
885-
trait_def_id,
886-
DUMMY_SP))
886+
.enter(|infcx| traits::type_known_to_meet_bound_modulo_regions(
887+
&infcx,
888+
param_env,
889+
ty,
890+
trait_def_id,
891+
DUMMY_SP,
892+
))
887893
}
888894

889895
fn needs_drop_raw<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>,

src/librustc_typeck/check/cast.rs

+4-4
Original file line numberDiff line numberDiff line change
@@ -88,7 +88,7 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> {
8888
return Err(ErrorReported);
8989
}
9090

91-
if self.type_is_known_to_be_sized(t, span) {
91+
if self.type_is_known_to_be_sized_modulo_regions(t, span) {
9292
return Ok(Some(PointerKind::Thin));
9393
}
9494

@@ -397,7 +397,7 @@ impl<'a, 'gcx, 'tcx> CastCheck<'tcx> {
397397
self.expr_ty,
398398
self.cast_ty);
399399

400-
if !fcx.type_is_known_to_be_sized(self.cast_ty, self.span) {
400+
if !fcx.type_is_known_to_be_sized_modulo_regions(self.cast_ty, self.span) {
401401
self.report_cast_to_unsized_type(fcx);
402402
} else if self.expr_ty.references_error() || self.cast_ty.references_error() {
403403
// No sense in giving duplicate error messages
@@ -618,8 +618,8 @@ impl<'a, 'gcx, 'tcx> CastCheck<'tcx> {
618618
}
619619

620620
impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> {
621-
fn type_is_known_to_be_sized(&self, ty: Ty<'tcx>, span: Span) -> bool {
621+
fn type_is_known_to_be_sized_modulo_regions(&self, ty: Ty<'tcx>, span: Span) -> bool {
622622
let lang_item = self.tcx.require_lang_item(lang_items::SizedTraitLangItem);
623-
traits::type_known_to_meet_bound(self, self.param_env, ty, lang_item, span)
623+
traits::type_known_to_meet_bound_modulo_regions(self, self.param_env, ty, lang_item, span)
624624
}
625625
}

0 commit comments

Comments
 (0)