Skip to content

make evaluation track whether outlives relationships mattered #54401

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

Closed
Closed
Show file tree
Hide file tree
Changes from 1 commit
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 src/librustc/infer/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1390,7 +1390,7 @@ impl<'a, 'gcx, 'tcx> InferCtxt<'a, 'gcx, 'tcx> {
// rightly refuses to work with inference variables, but
// moves_by_default has a cache, which we want to use in other
// cases.
!traits::type_known_to_meet_bound(self, param_env, ty, copy_def_id, span)
!traits::type_known_to_meet_bound_modulo_regions(self, param_env, ty, copy_def_id, span)
}

/// Obtains the latest type of the given closure; this may be a
Expand Down
4 changes: 2 additions & 2 deletions src/librustc/traits/fulfill.rs
Original file line number Diff line number Diff line change
Expand Up @@ -293,10 +293,10 @@ impl<'a, 'b, 'gcx, 'tcx> ObligationProcessor for FulfillProcessor<'a, 'b, 'gcx,
ty::Predicate::Trait(ref data) => {
let trait_obligation = obligation.with(data.clone());

if data.is_global() && !data.has_late_bound_regions() {
if data.is_global() {
// no type variables present, can use evaluation for better caching.
// FIXME: consider caching errors too.
if self.selcx.infcx().predicate_must_hold(&obligation) {
if self.selcx.infcx().predicate_must_hold_considering_regions(&obligation) {
debug!("selecting trait `{:?}` at depth {} evaluated to holds",
data, obligation.recursion_depth);
return ProcessResult::Changed(vec![])
Expand Down
22 changes: 11 additions & 11 deletions src/librustc/traits/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -564,14 +564,14 @@ pub fn predicates_for_generics<'tcx>(cause: ObligationCause<'tcx>,
/// `bound` or is not known to meet bound (note that this is
/// conservative towards *no impl*, which is the opposite of the
/// `evaluate` methods).
pub fn type_known_to_meet_bound<'a, 'gcx, 'tcx>(infcx: &InferCtxt<'a, 'gcx, 'tcx>,
param_env: ty::ParamEnv<'tcx>,
ty: Ty<'tcx>,
def_id: DefId,
span: Span)
-> bool
{
debug!("type_known_to_meet_bound(ty={:?}, bound={:?})",
pub fn type_known_to_meet_bound_modulo_regions<'a, 'gcx, 'tcx>(
infcx: &InferCtxt<'a, 'gcx, 'tcx>,
param_env: ty::ParamEnv<'tcx>,
ty: Ty<'tcx>,
def_id: DefId,
span: Span,
) -> bool {
debug!("type_known_to_meet_bound_modulo_regions(ty={:?}, bound={:?})",
ty,
infcx.tcx.item_path_str(def_id));

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

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

Expand All @@ -613,13 +613,13 @@ pub fn type_known_to_meet_bound<'a, 'gcx, 'tcx>(infcx: &InferCtxt<'a, 'gcx, 'tcx
// assume it is move; linear is always ok.
match fulfill_cx.select_all_or_error(infcx) {
Ok(()) => {
debug!("type_known_to_meet_bound: ty={:?} bound={} success",
debug!("type_known_to_meet_bound_modulo_regions: ty={:?} bound={} success",
ty,
infcx.tcx.item_path_str(def_id));
true
}
Err(e) => {
debug!("type_known_to_meet_bound: ty={:?} bound={} errors={:?}",
debug!("type_known_to_meet_bound_modulo_regions: ty={:?} bound={} errors={:?}",
ty,
infcx.tcx.item_path_str(def_id),
e);
Expand Down
19 changes: 17 additions & 2 deletions src/librustc/traits/query/evaluate_obligation.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,11 +26,26 @@ impl<'cx, 'gcx, 'tcx> InferCtxt<'cx, 'gcx, 'tcx> {
/// Evaluates whether the predicate can be satisfied in the given
/// `ParamEnv`, and returns `false` if not certain. However, this is
/// not entirely accurate if inference variables are involved.
pub fn predicate_must_hold(
///
/// This version may conservatively fail when outlives obligations
/// are required.
pub fn predicate_must_hold_considering_regions(
&self,
obligation: &PredicateObligation<'tcx>,
) -> bool {
self.evaluate_obligation(obligation) == EvaluationResult::EvaluatedToOk
self.evaluate_obligation(obligation).must_apply_considering_regions()
}

/// Evaluates whether the predicate can be satisfied in the given
/// `ParamEnv`, and returns `false` if not certain. However, this is
/// not entirely accurate if inference variables are involved.
///
/// This version ignores all outlives constraints.
pub fn predicate_must_hold_modulo_regions(
&self,
obligation: &PredicateObligation<'tcx>,
) -> bool {
self.evaluate_obligation(obligation).must_apply_modulo_regions()
}

// Helper function that canonicalizes and runs the query, as well as handles
Expand Down
33 changes: 28 additions & 5 deletions src/librustc/traits/select.rs
Original file line number Diff line number Diff line change
Expand Up @@ -315,7 +315,7 @@ enum BuiltinImplConditions<'tcx> {
/// evaluations.
///
/// The evaluation results are ordered:
/// - `EvaluatedToOk` implies `EvaluatedToAmbig` implies `EvaluatedToUnknown`
/// - `EvaluatedToOk` implies `EvaluatedToOkModuloRegions` implies `EvaluatedToAmbig` implies `EvaluatedToUnknown`
/// - `EvaluatedToErr` implies `EvaluatedToRecur`
/// - the "union" of evaluation results is equal to their maximum -
/// all the "potential success" candidates can potentially succeed,
Expand All @@ -324,6 +324,8 @@ enum BuiltinImplConditions<'tcx> {
pub enum EvaluationResult {
/// Evaluation successful
EvaluatedToOk,
/// Evaluation successful, but there were unevaluated region obligations
EvaluatedToOkModuloRegions,
/// Evaluation is known to be ambiguous - it *might* hold for some
/// assignment of inference variables, but it might not.
///
Expand Down Expand Up @@ -387,9 +389,22 @@ pub enum EvaluationResult {
}

impl EvaluationResult {
/// True if this evaluation result is known to apply, even
/// considering outlives constraints.
pub fn must_apply_considering_regions(self) -> bool {
self == EvaluatedToOk
}

/// True if this evaluation result is known to apply, ignoring
/// outlives constraints.
pub fn must_apply_modulo_regions(self) -> bool {
self <= EvaluatedToOkModuloRegions
}

pub fn may_apply(self) -> bool {
match self {
EvaluatedToOk |
EvaluatedToOkModuloRegions |
EvaluatedToAmbig |
EvaluatedToUnknown => true,

Expand All @@ -404,6 +419,7 @@ impl EvaluationResult {
EvaluatedToRecur => true,

EvaluatedToOk |
EvaluatedToOkModuloRegions |
EvaluatedToAmbig |
EvaluatedToErr => false,
}
Expand All @@ -412,6 +428,7 @@ impl EvaluationResult {

impl_stable_hash_for!(enum self::EvaluationResult {
EvaluatedToOk,
EvaluatedToOkModuloRegions,
EvaluatedToAmbig,
EvaluatedToUnknown,
EvaluatedToRecur,
Expand Down Expand Up @@ -693,7 +710,7 @@ impl<'cx, 'gcx, 'tcx> SelectionContext<'cx, 'gcx, 'tcx> {
ty::Predicate::TypeOutlives(..) | ty::Predicate::RegionOutlives(..) => {
// we do not consider region relationships when
// evaluating trait matches
Ok(EvaluatedToOk)
Ok(EvaluatedToOkModuloRegions)
}

ty::Predicate::ObjectSafe(trait_def_id) => {
Expand Down Expand Up @@ -900,7 +917,13 @@ impl<'cx, 'gcx, 'tcx> SelectionContext<'cx, 'gcx, 'tcx> {
{
debug!("evaluate_stack({:?}) --> recursive",
stack.fresh_trait_ref);
let cycle = stack.iter().skip(1).take(rec_index + 1);

// Subtle: when checking for a coinductive cycle, we do
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you give an example where this subtle thing is true? It certainly seems like we are comparing freshened regions, and that should be OK: freshening does not touch LBRs (even escaping LBRs), and trait evaluation pretty much works with the staticized version of a type, ignoring non-late-bound regions.

// not compare using the "freshened trait refs" (which
// have erased regions) but rather the fully explicit
// trait refs. This is important because it's only a cycle
// if the regions match exactly.
let cycle = stack.iter().skip(1).take(rec_index+1);
let cycle = cycle.map(|stack| ty::Predicate::Trait(stack.obligation.predicate));
if self.coinductive_match(cycle) {
debug!("evaluate_stack({:?}) --> recursive, coinductive",
Expand Down Expand Up @@ -2095,7 +2118,7 @@ impl<'cx, 'gcx, 'tcx> SelectionContext<'cx, 'gcx, 'tcx> {
// See if we can toss out `victim` based on specialization.
// This requires us to know *for sure* that the `other` impl applies
// i.e. EvaluatedToOk:
if other.evaluation == EvaluatedToOk {
if other.evaluation.must_apply_modulo_regions() {
match victim.candidate {
ImplCandidate(victim_def) => {
let tcx = self.tcx().global_tcx();
Expand All @@ -2122,7 +2145,7 @@ impl<'cx, 'gcx, 'tcx> SelectionContext<'cx, 'gcx, 'tcx> {
ParamCandidate(ref cand) => {
// Prefer these to a global where-clause bound
// (see issue #50825)
is_global(cand) && other.evaluation == EvaluatedToOk
is_global(cand) && other.evaluation.must_apply_modulo_regions()
}
_ => false,
}
Expand Down
36 changes: 21 additions & 15 deletions src/librustc/ty/util.rs
Original file line number Diff line number Diff line change
Expand Up @@ -862,11 +862,13 @@ fn is_copy_raw<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>,
let (param_env, ty) = query.into_parts();
let trait_def_id = tcx.require_lang_item(lang_items::CopyTraitLangItem);
tcx.infer_ctxt()
.enter(|infcx| traits::type_known_to_meet_bound(&infcx,
param_env,
ty,
trait_def_id,
DUMMY_SP))
.enter(|infcx| traits::type_known_to_meet_bound_modulo_regions(
&infcx,
param_env,
ty,
trait_def_id,
DUMMY_SP,
))
}

fn is_sized_raw<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>,
Expand All @@ -876,11 +878,13 @@ fn is_sized_raw<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>,
let (param_env, ty) = query.into_parts();
let trait_def_id = tcx.require_lang_item(lang_items::SizedTraitLangItem);
tcx.infer_ctxt()
.enter(|infcx| traits::type_known_to_meet_bound(&infcx,
param_env,
ty,
trait_def_id,
DUMMY_SP))
.enter(|infcx| traits::type_known_to_meet_bound_modulo_regions(
&infcx,
param_env,
ty,
trait_def_id,
DUMMY_SP,
))
}

fn is_freeze_raw<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>,
Expand All @@ -890,11 +894,13 @@ fn is_freeze_raw<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>,
let (param_env, ty) = query.into_parts();
let trait_def_id = tcx.require_lang_item(lang_items::FreezeTraitLangItem);
tcx.infer_ctxt()
.enter(|infcx| traits::type_known_to_meet_bound(&infcx,
param_env,
ty,
trait_def_id,
DUMMY_SP))
.enter(|infcx| traits::type_known_to_meet_bound_modulo_regions(
&infcx,
param_env,
ty,
trait_def_id,
DUMMY_SP,
))
}

fn needs_drop_raw<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>,
Expand Down
8 changes: 4 additions & 4 deletions src/librustc_typeck/check/cast.rs
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> {
return Err(ErrorReported);
}

if self.type_is_known_to_be_sized(t, span) {
if self.type_is_known_to_be_sized_modulo_regions(t, span) {
return Ok(Some(PointerKind::Thin));
}

Expand Down Expand Up @@ -406,7 +406,7 @@ impl<'a, 'gcx, 'tcx> CastCheck<'tcx> {
self.expr_ty,
self.cast_ty);

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

impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> {
fn type_is_known_to_be_sized(&self, ty: Ty<'tcx>, span: Span) -> bool {
fn type_is_known_to_be_sized_modulo_regions(&self, ty: Ty<'tcx>, span: Span) -> bool {
let lang_item = self.tcx.require_lang_item(lang_items::SizedTraitLangItem);
traits::type_known_to_meet_bound(self, self.param_env, ty, lang_item, span)
traits::type_known_to_meet_bound_modulo_regions(self, self.param_env, ty, lang_item, span)
}
}
24 changes: 24 additions & 0 deletions src/test/ui/hrtb/hrtb-cache-issue-54302.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
// Regression test for #54302.
//
// We were incorrectly using the "evaluation cache" (which ignored
// region results) to conclude that `&'static str: Deserialize`, even
// though it would require that `for<'de> 'de: 'static`, which is
// clearly false.

trait Deserialize<'de> {}

trait DeserializeOwned: for<'de> Deserialize<'de> {}
impl<T> DeserializeOwned for T where T: for<'de> Deserialize<'de> {}

// Based on this impl, `&'static str` only implements Deserialize<'static>.
// It does not implement for<'de> Deserialize<'de>.
impl<'de: 'a, 'a> Deserialize<'de> for &'a str {}

fn main() {
fn assert_deserialize_owned<T: DeserializeOwned>() {}
assert_deserialize_owned::<&'static str>(); //~ ERROR

// It correctly does not implement for<'de> Deserialize<'de>.
// fn assert_hrtb<T: for<'de> Deserialize<'de>>() {}
// assert_hrtb::<&'static str>();
}
17 changes: 17 additions & 0 deletions src/test/ui/hrtb/hrtb-cache-issue-54302.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
error[E0279]: the requirement `for<'de> 'de : ` is not satisfied (`expected bound lifetime parameter 'de, found concrete lifetime`)
--> $DIR/hrtb-cache-issue-54302.rs:19:5
|
LL | assert_deserialize_owned::<&'static str>(); //~ ERROR
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
= note: required because of the requirements on the impl of `for<'de> Deserialize<'de>` for `&'static str`
= note: required because of the requirements on the impl of `DeserializeOwned` for `&'static str`
note: required by `main::assert_deserialize_owned`
--> $DIR/hrtb-cache-issue-54302.rs:18:5
|
LL | fn assert_deserialize_owned<T: DeserializeOwned>() {}
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

error: aborting due to previous error

For more information about this error, try `rustc --explain E0279`.