-
Notifications
You must be signed in to change notification settings - Fork 13.1k
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
Don't use lift to detect local types #61871
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -465,9 +465,11 @@ impl<'a, 'tcx> MemCategorizationContext<'a, 'tcx> { | |
) -> bool { | ||
self.infcx.map(|infcx| infcx.type_is_copy_modulo_regions(param_env, ty, span)) | ||
.or_else(|| { | ||
self.tcx.lift_to_global(&(param_env, ty)).map(|(param_env, ty)| { | ||
ty.is_copy_modulo_regions(self.tcx.global_tcx(), param_env, span) | ||
}) | ||
if (param_env, ty).has_local_value() { | ||
None | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hmm, if There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I agree. |
||
} else { | ||
Some(ty.is_copy_modulo_regions(self.tcx, param_env, span)) | ||
} | ||
}) | ||
.unwrap_or(true) | ||
} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -461,16 +461,13 @@ impl<'a, 'b, 'tcx> ObligationProcessor for FulfillProcessor<'a, 'b, 'tcx> { | |
} | ||
|
||
ty::Predicate::ConstEvaluatable(def_id, substs) => { | ||
match self.selcx.tcx().lift_to_global(&obligation.param_env) { | ||
None => { | ||
if obligation.param_env.has_local_value() { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same as the other |
||
ProcessResult::Unchanged | ||
} | ||
Some(param_env) => { | ||
match self.selcx.tcx().lift_to_global(&substs) { | ||
Some(substs) => { | ||
} else { | ||
if !substs.has_local_value() { | ||
let instance = ty::Instance::resolve( | ||
self.selcx.tcx().global_tcx(), | ||
param_env, | ||
obligation.param_env, | ||
def_id, | ||
substs, | ||
); | ||
|
@@ -480,7 +477,7 @@ impl<'a, 'b, 'tcx> ObligationProcessor for FulfillProcessor<'a, 'b, 'tcx> { | |
promoted: None, | ||
}; | ||
match self.selcx.tcx().at(obligation.cause.span) | ||
.const_eval(param_env.and(cid)) { | ||
.const_eval(obligation.param_env.and(cid)) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I can't wait for this to use erasure and canonicalization instead (@oli-obk and @nikomatsakis are working on that, I think?). |
||
Ok(_) => ProcessResult::Changed(vec![]), | ||
Err(err) => ProcessResult::Error( | ||
CodeSelectionError(ConstEvalFailure(err))) | ||
|
@@ -490,17 +487,14 @@ impl<'a, 'b, 'tcx> ObligationProcessor for FulfillProcessor<'a, 'b, 'tcx> { | |
ConstEvalFailure(ErrorHandled::TooGeneric) | ||
)) | ||
} | ||
}, | ||
None => { | ||
} else { | ||
pending_obligation.stalled_on = substs.types().collect(); | ||
ProcessResult::Unchanged | ||
} | ||
} | ||
} | ||
} | ||
} | ||
} | ||
} | ||
|
||
fn process_backedge<'c, I>(&mut self, cycle: I, | ||
_marker: PhantomData<&'c PendingPredicateObligation<'tcx>>) | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -457,6 +457,16 @@ pub enum SelectionError<'tcx> { | |
Overflow, | ||
} | ||
|
||
EnumTypeFoldableImpl! { | ||
impl<'tcx> TypeFoldable<'tcx> for SelectionError<'tcx> { | ||
(SelectionError::Unimplemented), | ||
(SelectionError::OutputTypeParameterMismatch)(a, b, c), | ||
(SelectionError::TraitNotObjectSafe)(a), | ||
(SelectionError::ConstEvalFailure)(a), | ||
(SelectionError::Overflow), | ||
} | ||
} | ||
|
||
pub struct FulfillmentError<'tcx> { | ||
pub obligation: PredicateObligation<'tcx>, | ||
pub code: FulfillmentErrorCode<'tcx> | ||
|
@@ -782,13 +792,11 @@ fn do_normalize_predicates<'tcx>( | |
return Err(ErrorReported) | ||
} | ||
}; | ||
|
||
match tcx.lift_to_global(&predicates) { | ||
Some(predicates) => Ok(predicates), | ||
None => { | ||
if predicates.has_local_value() { | ||
// FIXME: shouldn't we, you know, actually report an error here? or an ICE? | ||
Err(ErrorReported) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @rust-lang/wg-traits This is scary, can we do a crater run with a There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @eddyb What happened to this? |
||
} | ||
} else { | ||
Ok(predicates) | ||
} | ||
}) | ||
} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -328,6 +328,23 @@ impl<'a, 'tcx> ty::Lift<'tcx> for SelectionCandidate<'a> { | |
} | ||
} | ||
|
||
EnumTypeFoldableImpl! { | ||
impl<'tcx> TypeFoldable<'tcx> for SelectionCandidate<'tcx> { | ||
(SelectionCandidate::BuiltinCandidate) { has_nested }, | ||
(SelectionCandidate::ParamCandidate)(poly_trait_ref), | ||
(SelectionCandidate::ImplCandidate)(def_id), | ||
(SelectionCandidate::AutoImplCandidate)(def_id), | ||
(SelectionCandidate::ProjectionCandidate), | ||
(SelectionCandidate::ClosureCandidate), | ||
(SelectionCandidate::GeneratorCandidate), | ||
(SelectionCandidate::FnPointerCandidate), | ||
(SelectionCandidate::TraitAliasCandidate)(def_id), | ||
(SelectionCandidate::ObjectCandidate), | ||
(SelectionCandidate::BuiltinObjectCandidate), | ||
(SelectionCandidate::BuiltinUnsizeCandidate), | ||
} | ||
} | ||
|
||
struct SelectionCandidateSet<'tcx> { | ||
// a list of candidates that definitely apply to the current | ||
// obligation (meaning: types unify). | ||
|
@@ -818,10 +835,10 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> { | |
|
||
ty::Predicate::ConstEvaluatable(def_id, substs) => { | ||
let tcx = self.tcx(); | ||
match tcx.lift_to_global(&(obligation.param_env, substs)) { | ||
Some((param_env, substs)) => { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'd prefer if one of two things happened here (and in other places
That is, I would prefer if indentation didn't change (and it only does because of the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why do you not want indention to change? I like less rightward drift, and I definitely don't like matching on bools. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It's fine to change, I just want to be able to read the diff without mentally pretending nothing changed without checking, where indentation is involved. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I changed the indentation to reduce the diffs in this PR. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @eddyb you can append There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nice. I also reordered some branches which that can't deal with though =P |
||
if !(obligation.param_env, substs).has_local_value() { | ||
let param_env = obligation.param_env; | ||
let instance = | ||
ty::Instance::resolve(tcx.global_tcx(), param_env, def_id, substs); | ||
ty::Instance::resolve(tcx, param_env, def_id, substs); | ||
if let Some(instance) = instance { | ||
let cid = GlobalId { | ||
instance, | ||
|
@@ -834,15 +851,13 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> { | |
} else { | ||
Ok(EvaluatedToErr) | ||
} | ||
} | ||
None => { | ||
} else { | ||
// Inference variables still left in param_env or substs. | ||
Ok(EvaluatedToAmbig) | ||
} | ||
} | ||
} | ||
} | ||
} | ||
|
||
fn evaluate_trait_predicate_recursively<'o>( | ||
&mut self, | ||
|
@@ -1172,7 +1187,7 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> { | |
} | ||
|
||
if self.can_use_global_caches(param_env) { | ||
if let Some(trait_ref) = self.tcx().lift_to_global(&trait_ref) { | ||
if !trait_ref.has_local_value() { | ||
debug!( | ||
"insert_evaluation_cache(trait_ref={:?}, candidate={:?}) global", | ||
trait_ref, result, | ||
|
@@ -1645,8 +1660,8 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> { | |
if let Err(Overflow) = candidate { | ||
// Don't cache overflow globally; we only produce this | ||
// in certain modes. | ||
} else if let Some(trait_ref) = tcx.lift_to_global(&trait_ref) { | ||
if let Some(candidate) = tcx.lift_to_global(&candidate) { | ||
} else if !trait_ref.has_local_value() { | ||
if !candidate.has_local_value() { | ||
debug!( | ||
"insert_candidate_cache(trait_ref={:?}, candidate={:?}) global", | ||
trait_ref, candidate, | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
cc @rust-lang/compiler I think it might be better if we stop referring to these as "local values" and instead use something more like
.has_infer_vars()
?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
has_infer_types
already exist and is a different property.