stop special-casing 'static in evaluate

This commit is contained in:
lcnr 2023-03-22 11:37:57 +01:00
parent 9bdb4881c7
commit 73c79cd806
7 changed files with 151 additions and 102 deletions

View file

@ -43,18 +43,16 @@ pub struct TypeFreshener<'a, 'tcx> {
const_freshen_count: u32,
ty_freshen_map: FxHashMap<ty::InferTy, Ty<'tcx>>,
const_freshen_map: FxHashMap<ty::InferConst<'tcx>, ty::Const<'tcx>>,
keep_static: bool,
}
impl<'a, 'tcx> TypeFreshener<'a, 'tcx> {
pub fn new(infcx: &'a InferCtxt<'tcx>, keep_static: bool) -> TypeFreshener<'a, 'tcx> {
pub fn new(infcx: &'a InferCtxt<'tcx>) -> TypeFreshener<'a, 'tcx> {
TypeFreshener {
infcx,
ty_freshen_count: 0,
const_freshen_count: 0,
ty_freshen_map: Default::default(),
const_freshen_map: Default::default(),
keep_static,
}
}
@ -121,18 +119,9 @@ fn fold_region(&mut self, r: ty::Region<'tcx>) -> ty::Region<'tcx> {
| ty::ReFree(_)
| ty::ReVar(_)
| ty::RePlaceholder(..)
| ty::ReStatic
| ty::ReError(_)
| ty::ReErased => {
// replace all free regions with 'erased
self.interner().lifetimes.re_erased
}
ty::ReStatic => {
if self.keep_static {
r
} else {
self.interner().lifetimes.re_erased
}
}
| ty::ReErased => self.interner().lifetimes.re_erased,
}
}

View file

@ -706,12 +706,7 @@ pub fn type_var_origin(&self, ty: Ty<'tcx>) -> Option<TypeVariableOrigin> {
}
pub fn freshener<'b>(&'b self) -> TypeFreshener<'b, 'tcx> {
freshen::TypeFreshener::new(self, false)
}
/// Like `freshener`, but does not replace `'static` regions.
pub fn freshener_keep_static<'b>(&'b self) -> TypeFreshener<'b, 'tcx> {
freshen::TypeFreshener::new(self, true)
freshen::TypeFreshener::new(self)
}
pub fn unsolved_variables(&self) -> Vec<Ty<'tcx>> {

View file

@ -211,7 +211,7 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
pub fn new(infcx: &'cx InferCtxt<'tcx>) -> SelectionContext<'cx, 'tcx> {
SelectionContext {
infcx,
freshener: infcx.freshener_keep_static(),
freshener: infcx.freshener(),
intercrate_ambiguity_causes: None,
query_mode: TraitQueryMode::Standard,
}
@ -769,14 +769,16 @@ fn evaluate_predicate_recursively<'o>(
}
ty::PredicateKind::Clause(ty::Clause::TypeOutlives(pred)) => {
// A global type with no late-bound regions can only
// contain the "'static" lifetime (any other lifetime
// would either be late-bound or local), so it is guaranteed
// to outlive any other lifetime
if pred.0.is_global() && !pred.0.has_late_bound_vars() {
Ok(EvaluatedToOk)
} else {
// A global type with no free lifetimes or generic parameters
// outlives anything.
if pred.0.has_free_regions()
|| pred.0.has_late_bound_regions()
|| pred.0.has_non_region_infer()
|| pred.0.has_non_region_infer()
{
Ok(EvaluatedToOkModuloRegions)
} else {
Ok(EvaluatedToOk)
}
}
@ -1824,6 +1826,12 @@ enum DropVictim {
No,
}
impl DropVictim {
fn drop_if(should_drop: bool) -> DropVictim {
if should_drop { DropVictim::Yes } else { DropVictim::No }
}
}
/// ## Winnowing
///
/// Winnowing is the process of attempting to resolve ambiguity by
@ -1889,11 +1897,7 @@ fn candidate_should_be_dropped_in_favor_of(
// or the current one if tied (they should both evaluate to the same answer). This is
// probably best characterized as a "hack", since we might prefer to just do our
// best to *not* create essentially duplicate candidates in the first place.
if other.bound_vars().len() <= victim.bound_vars().len() {
DropVictim::Yes
} else {
DropVictim::No
}
DropVictim::drop_if(other.bound_vars().len() <= victim.bound_vars().len())
} else if other.skip_binder().trait_ref == victim.skip_binder().trait_ref
&& victim.skip_binder().constness == ty::BoundConstness::NotConst
&& other.skip_binder().polarity == victim.skip_binder().polarity
@ -1923,17 +1927,13 @@ fn candidate_should_be_dropped_in_favor_of(
| ObjectCandidate(_)
| ProjectionCandidate(..),
) => {
if is_global(other_cand) {
DropVictim::No
} else {
// We have a where clause so don't go around looking
// for impls. Arbitrarily give param candidates priority
// over projection and object candidates.
//
// Global bounds from the where clause should be ignored
// here (see issue #50825).
DropVictim::Yes
}
// We have a where clause so don't go around looking
// for impls. Arbitrarily give param candidates priority
// over projection and object candidates.
//
// Global bounds from the where clause should be ignored
// here (see issue #50825).
DropVictim::drop_if(!is_global(other_cand))
}
(ObjectCandidate(_) | ProjectionCandidate(..), ParamCandidate(ref victim_cand)) => {
// Prefer these to a global where-clause bound
@ -1955,18 +1955,16 @@ fn candidate_should_be_dropped_in_favor_of(
) => {
// Prefer these to a global where-clause bound
// (see issue #50825).
if is_global(victim_cand) && other.evaluation.must_apply_modulo_regions() {
DropVictim::Yes
} else {
DropVictim::No
}
DropVictim::drop_if(
is_global(victim_cand) && other.evaluation.must_apply_modulo_regions(),
)
}
(ProjectionCandidate(i, _), ProjectionCandidate(j, _))
| (ObjectCandidate(i), ObjectCandidate(j)) => {
// Arbitrarily pick the lower numbered candidate for backwards
// compatibility reasons. Don't let this affect inference.
if i < j && !needs_infer { DropVictim::Yes } else { DropVictim::No }
DropVictim::drop_if(i < j && !needs_infer)
}
(ObjectCandidate(_), ProjectionCandidate(..))
| (ProjectionCandidate(..), ObjectCandidate(_)) => {
@ -2017,55 +2015,65 @@ fn candidate_should_be_dropped_in_favor_of(
}
}
if other.evaluation.must_apply_considering_regions() {
match tcx.impls_are_allowed_to_overlap(other_def, victim_def) {
Some(ty::ImplOverlapKind::Permitted { marker: true }) => {
// Subtle: If the predicate we are evaluating has inference
// variables, do *not* allow discarding candidates due to
// marker trait impls.
//
// Without this restriction, we could end up accidentally
// constraining inference variables based on an arbitrarily
// chosen trait impl.
//
// Imagine we have the following code:
//
// ```rust
// #[marker] trait MyTrait {}
// impl MyTrait for u8 {}
// impl MyTrait for bool {}
// ```
//
// And we are evaluating the predicate `<_#0t as MyTrait>`.
//
// During selection, we will end up with one candidate for each
// impl of `MyTrait`. If we were to discard one impl in favor
// of the other, we would be left with one candidate, causing
// us to "successfully" select the predicate, unifying
// _#0t with (for example) `u8`.
//
// However, we have no reason to believe that this unification
// is correct - we've essentially just picked an arbitrary
// *possibility* for _#0t, and required that this be the *only*
// possibility.
//
// Eventually, we will either:
// 1) Unify all inference variables in the predicate through
// some other means (e.g. type-checking of a function). We will
// then be in a position to drop marker trait candidates
// without constraining inference variables (since there are
// none left to constrain)
// 2) Be left with some unconstrained inference variables. We
// will then correctly report an inference error, since the
// existence of multiple marker trait impls tells us nothing
// about which one should actually apply.
if needs_infer { DropVictim::No } else { DropVictim::Yes }
}
Some(_) => DropVictim::Yes,
None => DropVictim::No,
match tcx.impls_are_allowed_to_overlap(other_def, victim_def) {
// For #33140 the impl headers must be exactly equal, the trait must not have
// any associated items and there are no where-clauses.
//
// We can just arbitrarily drop one of the impls.
Some(ty::ImplOverlapKind::Issue33140) => {
assert_eq!(other.evaluation, victim.evaluation);
DropVictim::Yes
}
} else {
DropVictim::No
// For candidates which already reference errors it doesn't really
// matter what we do 🤷
Some(ty::ImplOverlapKind::Permitted { marker: false }) => {
DropVictim::drop_if(other.evaluation.must_apply_considering_regions())
}
Some(ty::ImplOverlapKind::Permitted { marker: true }) => {
// Subtle: If the predicate we are evaluating has inference
// variables, do *not* allow discarding candidates due to
// marker trait impls.
//
// Without this restriction, we could end up accidentally
// constraining inference variables based on an arbitrarily
// chosen trait impl.
//
// Imagine we have the following code:
//
// ```rust
// #[marker] trait MyTrait {}
// impl MyTrait for u8 {}
// impl MyTrait for bool {}
// ```
//
// And we are evaluating the predicate `<_#0t as MyTrait>`.
//
// During selection, we will end up with one candidate for each
// impl of `MyTrait`. If we were to discard one impl in favor
// of the other, we would be left with one candidate, causing
// us to "successfully" select the predicate, unifying
// _#0t with (for example) `u8`.
//
// However, we have no reason to believe that this unification
// is correct - we've essentially just picked an arbitrary
// *possibility* for _#0t, and required that this be the *only*
// possibility.
//
// Eventually, we will either:
// 1) Unify all inference variables in the predicate through
// some other means (e.g. type-checking of a function). We will
// then be in a position to drop marker trait candidates
// without constraining inference variables (since there are
// none left to constrain)
// 2) Be left with some unconstrained inference variables. We
// will then correctly report an inference error, since the
// existence of multiple marker trait impls tells us nothing
// about which one should actually apply.
DropVictim::drop_if(
!needs_infer && other.evaluation.must_apply_considering_regions(),
)
}
None => DropVictim::No,
}
}

View file

@ -1,4 +1,8 @@
// check-pass
// known-bug: #89515
//
// The trait solver cannot deal with ambiguous marker trait impls
// if there are lifetimes involved. As we must not special-case any
// regions this does not work, even with 'static
#![feature(marker_trait_attr)]
#[marker]

View file

@ -0,0 +1,31 @@
error[E0283]: type annotations needed: cannot satisfy `&'static (): Marker`
--> $DIR/overlap-marker-trait-with-static-lifetime.rs:11:17
|
LL | impl Marker for &'static () {}
| ^^^^^^^^^^^
|
note: multiple `impl`s satisfying `&'static (): Marker` found
--> $DIR/overlap-marker-trait-with-static-lifetime.rs:11:1
|
LL | impl Marker for &'static () {}
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^
LL | impl Marker for &'static () {}
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^
error[E0283]: type annotations needed: cannot satisfy `&'static (): Marker`
--> $DIR/overlap-marker-trait-with-static-lifetime.rs:12:17
|
LL | impl Marker for &'static () {}
| ^^^^^^^^^^^
|
note: multiple `impl`s satisfying `&'static (): Marker` found
--> $DIR/overlap-marker-trait-with-static-lifetime.rs:11:1
|
LL | impl Marker for &'static () {}
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^
LL | impl Marker for &'static () {}
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^
error: aborting due to 2 previous errors
For more information about this error, try `rustc --explain E0283`.

View file

@ -1,9 +1,17 @@
// check-pass
// known-bug: #109481
//
// While the `T: Copy` is always applicable when checking
// that the impl `impl<T: Copy> F for T {}` is well formed,
// the old trait solver can only approximate this by checking
// that there are no inference variables in the obligation and
// no region constraints in the evaluation result.
//
// Because of this we end up with ambiguity here.
#![feature(marker_trait_attr)]
#[marker]
pub trait F {}
impl<T> F for T where T: Copy {}
impl<T> F for T where T: 'static {}
impl<T: Copy> F for T {}
impl<T: 'static> F for T {}
fn main() {}

View file

@ -0,0 +1,14 @@
error[E0310]: the parameter type `T` may not live long enough
--> $DIR/overlapping-impl-1-modulo-regions.rs:14:21
|
LL | impl<T: Copy> F for T {}
| ^ ...so that the type `T` will meet its required lifetime bounds
|
help: consider adding an explicit lifetime bound...
|
LL | impl<T: Copy + 'static> F for T {}
| +++++++++
error: aborting due to previous error
For more information about this error, try `rustc --explain E0310`.