Skip to content

Commit

Permalink
Fix incosistent usage of push_lifetime_outlives
Browse files Browse the repository at this point in the history
The code that relates Adt's was using `push_lifetime_outlives_goals` in
a way that assumed it would push a outlives b when the variance is
covariant. The code that relates two references assumed that it would
push a constraint b outlives a when the variance was covariant. This
latter behavior was what the function actually did, despite its name
implying the opposite.

This commit changes the behavior of `push_lifetime_outlives_goals` to
push a outlives b when the variance is covariant, and updates the code
that relates references.

We also updated an incorrect test case where a reference was used in a
struct. This test case would have failed (because
`push_lifetime_outlives_goals` was inverted), but the constraints in the
expected output were also incorrectly inverted.

The output of two additional test cases also were updated because the
constraints were emitted in a different order after making the update.

Co-authored-by: David Ross <daboross@daboross.net>
  • Loading branch information
super-tuple and daboross committed Nov 1, 2020
1 parent c7c1d9d commit d5b0ffc
Show file tree
Hide file tree
Showing 5 changed files with 13 additions and 18 deletions.
4 changes: 2 additions & 2 deletions chalk-solve/src/infer/test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -413,10 +413,10 @@ fn lifetime_constraint_indirect() {
assert_eq!(goals.len(), 2);
assert_eq!(
format!("{:?}", goals[0]),
"InEnvironment { environment: Env([]), goal: \'?2: \'!1_0 }",
"InEnvironment { environment: Env([]), goal: \'!1_0: \'?2 }",
);
assert_eq!(
format!("{:?}", goals[1]),
"InEnvironment { environment: Env([]), goal: \'!1_0: \'?2 }",
"InEnvironment { environment: Env([]), goal: \'?2: \'!1_0 }",
);
}
15 changes: 5 additions & 10 deletions chalk-solve/src/infer/unify.rs
Original file line number Diff line number Diff line change
Expand Up @@ -275,13 +275,8 @@ impl<'t, I: Interner> Unifier<'t, I> {
if mutability_a != mutability_b {
return Err(NoSolution);
}
// The lifetime is `Contravariant`
Zip::zip_with(
self,
variance.xform(Variance::Contravariant),
lifetime_a,
lifetime_b,
)?;
// Zipping lifetimes implies a is a subtype of b, meaning a outlives b
Zip::zip_with(self, variance, lifetime_a, lifetime_b)?;
// The type is `Covariant` when not mut, `Invariant` otherwise
let output_variance = match mutability_a {
Mutability::Not => Variance::Covariant,
Expand Down Expand Up @@ -1035,16 +1030,16 @@ impl<'t, I: Interner> Unifier<'t, I> {
self.goals.push(InEnvironment::new(
self.environment,
WhereClause::LifetimeOutlives(LifetimeOutlives {
a: a.clone(),
b: b.clone(),
a: b.clone(),
b: a.clone(),
})
.cast(self.interner),
));
}
if matches!(variance, Variance::Invariant | Variance::Covariant) {
self.goals.push(InEnvironment::new(
self.environment,
WhereClause::LifetimeOutlives(LifetimeOutlives { a: b, b: a }).cast(self.interner),
WhereClause::LifetimeOutlives(LifetimeOutlives { a, b }).cast(self.interner),
));
}
}
Expand Down
4 changes: 2 additions & 2 deletions tests/test/misc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -259,8 +259,8 @@ fn basic_region_constraint_from_positive_impl() {
forall<'a, 'b, T> { Ref<'a, 'b, T>: Foo }
} yields_all[SolverChoice::slg(3, None)] {
"substitution [], lifetime constraints [\
InEnvironment { environment: Env([]), goal: '!1_0: '!1_1 }, \
InEnvironment { environment: Env([]), goal: '!1_1: '!1_0 } \
InEnvironment { environment: Env([]), goal: '!1_1: '!1_0 }, \
InEnvironment { environment: Env([]), goal: '!1_0: '!1_1 } \
]"
}
}
Expand Down
6 changes: 3 additions & 3 deletions tests/test/projection.rs
Original file line number Diff line number Diff line change
Expand Up @@ -799,10 +799,10 @@ fn normalize_under_binder_multi() {
} yields_all {
"substitution [?0 := I32], lifetime constraints []",
"for<?U0,?U0> { substitution [?0 := (Deref::Item)<Ref<'^0.0, I32>, '^0.1>], lifetime constraints [\
InEnvironment { environment: Env([]), goal: '!1_0: '^0.1 }, \
InEnvironment { environment: Env([]), goal: '^0.1: '!1_0 }, \
InEnvironment { environment: Env([]), goal: '!1_0: '^0.0 }, \
InEnvironment { environment: Env([]), goal: '^0.0: '!1_0 }] }"
InEnvironment { environment: Env([]), goal: '!1_0: '^0.1 }, \
InEnvironment { environment: Env([]), goal: '^0.0: '!1_0 }, \
InEnvironment { environment: Env([]), goal: '!1_0: '^0.0 }] }"
}

goal {
Expand Down
2 changes: 1 addition & 1 deletion tests/test/subtype.rs
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ fn struct_lifetime_variance() {
}
} yields {
"Unique; substitution [], lifetime constraints [\
InEnvironment { environment: Env([]), goal: '!1_1: '!1_0 } \
InEnvironment { environment: Env([]), goal: '!1_0: '!1_1 } \
]"
}
}
Expand Down

0 comments on commit d5b0ffc

Please sign in to comment.