Skip to content
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.

Commit 643b173

Browse files
committedSep 25, 2018
Auto merge of #54401 - nikomatsakis:issue-54302-hrtb-fail, r=<try>
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. Fixes #54302 r? @arielb1
2 parents ae36663 + 563ba10 commit 643b173

File tree

9 files changed

+1466
-1143
lines changed

9 files changed

+1466
-1143
lines changed
 

‎src/librustc/infer/mod.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -1390,7 +1390,7 @@ impl<'a, 'gcx, 'tcx> InferCtxt<'a, 'gcx, 'tcx> {
13901390
// rightly refuses to work with inference variables, but
13911391
// moves_by_default has a cache, which we want to use in other
13921392
// cases.
1393-
!traits::type_known_to_meet_bound(self, param_env, ty, copy_def_id, span)
1393+
!traits::type_known_to_meet_bound_modulo_regions(self, param_env, ty, copy_def_id, span)
13941394
}
13951395

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

‎src/librustc/traits/fulfill.rs

+2-2
Original file line numberDiff line numberDiff line change
@@ -293,10 +293,10 @@ impl<'a, 'b, 'gcx, 'tcx> ObligationProcessor for FulfillProcessor<'a, 'b, 'gcx,
293293
ty::Predicate::Trait(ref data) => {
294294
let trait_obligation = obligation.with(data.clone());
295295

296-
if data.is_global() && !data.has_late_bound_regions() {
296+
if data.is_global() {
297297
// no type variables present, can use evaluation for better caching.
298298
// FIXME: consider caching errors too.
299-
if self.selcx.infcx().predicate_must_hold(&obligation) {
299+
if self.selcx.infcx().predicate_must_hold_considering_regions(&obligation) {
300300
debug!("selecting trait `{:?}` at depth {} evaluated to holds",
301301
data, obligation.recursion_depth);
302302
return ProcessResult::Changed(vec![])

‎src/librustc/traits/mod.rs

+11-11
Original file line numberDiff line numberDiff line change
@@ -564,14 +564,14 @@ pub fn predicates_for_generics<'tcx>(cause: ObligationCause<'tcx>,
564564
/// `bound` or is not known to meet bound (note that this is
565565
/// conservative towards *no impl*, which is the opposite of the
566566
/// `evaluate` methods).
567-
pub fn type_known_to_meet_bound<'a, 'gcx, 'tcx>(infcx: &InferCtxt<'a, 'gcx, 'tcx>,
568-
param_env: ty::ParamEnv<'tcx>,
569-
ty: Ty<'tcx>,
570-
def_id: DefId,
571-
span: Span)
572-
-> bool
573-
{
574-
debug!("type_known_to_meet_bound(ty={:?}, bound={:?})",
567+
pub fn type_known_to_meet_bound_modulo_regions<'a, 'gcx, 'tcx>(
568+
infcx: &InferCtxt<'a, 'gcx, 'tcx>,
569+
param_env: ty::ParamEnv<'tcx>,
570+
ty: Ty<'tcx>,
571+
def_id: DefId,
572+
span: Span,
573+
) -> bool {
574+
debug!("type_known_to_meet_bound_modulo_regions(ty={:?}, bound={:?})",
575575
ty,
576576
infcx.tcx.item_path_str(def_id));
577577

@@ -586,7 +586,7 @@ pub fn type_known_to_meet_bound<'a, 'gcx, 'tcx>(infcx: &InferCtxt<'a, 'gcx, 'tcx
586586
predicate: trait_ref.to_predicate(),
587587
};
588588

589-
let result = infcx.predicate_must_hold(&obligation);
589+
let result = infcx.predicate_must_hold_modulo_regions(&obligation);
590590
debug!("type_known_to_meet_ty={:?} bound={} => {:?}",
591591
ty, infcx.tcx.item_path_str(def_id), result);
592592

@@ -613,13 +613,13 @@ pub fn type_known_to_meet_bound<'a, 'gcx, 'tcx>(infcx: &InferCtxt<'a, 'gcx, 'tcx
613613
// assume it is move; linear is always ok.
614614
match fulfill_cx.select_all_or_error(infcx) {
615615
Ok(()) => {
616-
debug!("type_known_to_meet_bound: ty={:?} bound={} success",
616+
debug!("type_known_to_meet_bound_modulo_regions: ty={:?} bound={} success",
617617
ty,
618618
infcx.tcx.item_path_str(def_id));
619619
true
620620
}
621621
Err(e) => {
622-
debug!("type_known_to_meet_bound: ty={:?} bound={} errors={:?}",
622+
debug!("type_known_to_meet_bound_modulo_regions: ty={:?} bound={} errors={:?}",
623623
ty,
624624
infcx.tcx.item_path_str(def_id),
625625
e);

‎src/librustc/traits/query/evaluate_obligation.rs

+17-2
Original file line numberDiff line numberDiff line change
@@ -26,11 +26,26 @@ impl<'cx, 'gcx, 'tcx> InferCtxt<'cx, 'gcx, 'tcx> {
2626
/// Evaluates whether the predicate can be satisfied in the given
2727
/// `ParamEnv`, and returns `false` if not certain. However, this is
2828
/// not entirely accurate if inference variables are involved.
29-
pub fn predicate_must_hold(
29+
///
30+
/// This version may conservatively fail when outlives obligations
31+
/// are required.
32+
pub fn predicate_must_hold_considering_regions(
3033
&self,
3134
obligation: &PredicateObligation<'tcx>,
3235
) -> bool {
33-
self.evaluate_obligation(obligation) == EvaluationResult::EvaluatedToOk
36+
self.evaluate_obligation(obligation).must_apply_considering_regions()
37+
}
38+
39+
/// Evaluates whether the predicate can be satisfied in the given
40+
/// `ParamEnv`, and returns `false` if not certain. However, this is
41+
/// not entirely accurate if inference variables are involved.
42+
///
43+
/// This version ignores all outlives constraints.
44+
pub fn predicate_must_hold_modulo_regions(
45+
&self,
46+
obligation: &PredicateObligation<'tcx>,
47+
) -> bool {
48+
self.evaluate_obligation(obligation).must_apply_modulo_regions()
3449
}
3550

3651
// Helper function that canonicalizes and runs the query, as well as handles

‎src/librustc/traits/select.rs

+1,369-1,108
Large diffs are not rendered by default.

‎src/librustc/ty/util.rs

+21-15
Original file line numberDiff line numberDiff line change
@@ -862,11 +862,13 @@ fn is_copy_raw<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>,
862862
let (param_env, ty) = query.into_parts();
863863
let trait_def_id = tcx.require_lang_item(lang_items::CopyTraitLangItem);
864864
tcx.infer_ctxt()
865-
.enter(|infcx| traits::type_known_to_meet_bound(&infcx,
866-
param_env,
867-
ty,
868-
trait_def_id,
869-
DUMMY_SP))
865+
.enter(|infcx| traits::type_known_to_meet_bound_modulo_regions(
866+
&infcx,
867+
param_env,
868+
ty,
869+
trait_def_id,
870+
DUMMY_SP,
871+
))
870872
}
871873

872874
fn is_sized_raw<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>,
@@ -876,11 +878,13 @@ fn is_sized_raw<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>,
876878
let (param_env, ty) = query.into_parts();
877879
let trait_def_id = tcx.require_lang_item(lang_items::SizedTraitLangItem);
878880
tcx.infer_ctxt()
879-
.enter(|infcx| traits::type_known_to_meet_bound(&infcx,
880-
param_env,
881-
ty,
882-
trait_def_id,
883-
DUMMY_SP))
881+
.enter(|infcx| traits::type_known_to_meet_bound_modulo_regions(
882+
&infcx,
883+
param_env,
884+
ty,
885+
trait_def_id,
886+
DUMMY_SP,
887+
))
884888
}
885889

886890
fn is_freeze_raw<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>,
@@ -890,11 +894,13 @@ fn is_freeze_raw<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>,
890894
let (param_env, ty) = query.into_parts();
891895
let trait_def_id = tcx.require_lang_item(lang_items::FreezeTraitLangItem);
892896
tcx.infer_ctxt()
893-
.enter(|infcx| traits::type_known_to_meet_bound(&infcx,
894-
param_env,
895-
ty,
896-
trait_def_id,
897-
DUMMY_SP))
897+
.enter(|infcx| traits::type_known_to_meet_bound_modulo_regions(
898+
&infcx,
899+
param_env,
900+
ty,
901+
trait_def_id,
902+
DUMMY_SP,
903+
))
898904
}
899905

900906
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
@@ -98,7 +98,7 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> {
9898
return Err(ErrorReported);
9999
}
100100

101-
if self.type_is_known_to_be_sized(t, span) {
101+
if self.type_is_known_to_be_sized_modulo_regions(t, span) {
102102
return Ok(Some(PointerKind::Thin));
103103
}
104104

@@ -406,7 +406,7 @@ impl<'a, 'gcx, 'tcx> CastCheck<'tcx> {
406406
self.expr_ty,
407407
self.cast_ty);
408408

409-
if !fcx.type_is_known_to_be_sized(self.cast_ty, self.span) {
409+
if !fcx.type_is_known_to_be_sized_modulo_regions(self.cast_ty, self.span) {
410410
self.report_cast_to_unsized_type(fcx);
411411
} else if self.expr_ty.references_error() || self.cast_ty.references_error() {
412412
// No sense in giving duplicate error messages
@@ -627,8 +627,8 @@ impl<'a, 'gcx, 'tcx> CastCheck<'tcx> {
627627
}
628628

629629
impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> {
630-
fn type_is_known_to_be_sized(&self, ty: Ty<'tcx>, span: Span) -> bool {
630+
fn type_is_known_to_be_sized_modulo_regions(&self, ty: Ty<'tcx>, span: Span) -> bool {
631631
let lang_item = self.tcx.require_lang_item(lang_items::SizedTraitLangItem);
632-
traits::type_known_to_meet_bound(self, self.param_env, ty, lang_item, span)
632+
traits::type_known_to_meet_bound_modulo_regions(self, self.param_env, ty, lang_item, span)
633633
}
634634
}
+24
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
// Regression test for #54302.
2+
//
3+
// We were incorrectly using the "evaluation cache" (which ignored
4+
// region results) to conclude that `&'static str: Deserialize`, even
5+
// though it would require that `for<'de> 'de: 'static`, which is
6+
// clearly false.
7+
8+
trait Deserialize<'de> {}
9+
10+
trait DeserializeOwned: for<'de> Deserialize<'de> {}
11+
impl<T> DeserializeOwned for T where T: for<'de> Deserialize<'de> {}
12+
13+
// Based on this impl, `&'static str` only implements Deserialize<'static>.
14+
// It does not implement for<'de> Deserialize<'de>.
15+
impl<'de: 'a, 'a> Deserialize<'de> for &'a str {}
16+
17+
fn main() {
18+
fn assert_deserialize_owned<T: DeserializeOwned>() {}
19+
assert_deserialize_owned::<&'static str>(); //~ ERROR
20+
21+
// It correctly does not implement for<'de> Deserialize<'de>.
22+
// fn assert_hrtb<T: for<'de> Deserialize<'de>>() {}
23+
// assert_hrtb::<&'static str>();
24+
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
error[E0279]: the requirement `for<'de> 'de : ` is not satisfied (`expected bound lifetime parameter 'de, found concrete lifetime`)
2+
--> $DIR/hrtb-cache-issue-54302.rs:19:5
3+
|
4+
LL | assert_deserialize_owned::<&'static str>(); //~ ERROR
5+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
6+
|
7+
= note: required because of the requirements on the impl of `for<'de> Deserialize<'de>` for `&'static str`
8+
= note: required because of the requirements on the impl of `DeserializeOwned` for `&'static str`
9+
note: required by `main::assert_deserialize_owned`
10+
--> $DIR/hrtb-cache-issue-54302.rs:18:5
11+
|
12+
LL | fn assert_deserialize_owned<T: DeserializeOwned>() {}
13+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
14+
15+
error: aborting due to previous error
16+
17+
For more information about this error, try `rustc --explain E0279`.

0 commit comments

Comments
 (0)
Please sign in to comment.