Skip to content

Commit

Permalink
Auto merge of #61775 - nikomatsakis:issue-56238-multiple-lifetimes-as…
Browse files Browse the repository at this point in the history
…ync-fn-region-solver, r=MatthewJasper

generalize impl trait to permit multiple lifetime bounds

Generalizes the region solver to support "pick constraints". These have the form:

```
pick R0 from [R1..Rn]
```

where `R1..Rn` are called the "option regions". The idea is that `R0` must be equal to *some* region in the set `R1..Rn`. These constraints are then used to handle cases like this:

```rust
fn foo<'a, 'b>(...) -> impl Trait<'a, 'b> { .. }
```

The problem here is that every region R in the hidden type must be equal to *either* `'a` *or* `'b` (or `'static`) -- in the past, the only kinds of constraints we had were outlives constraints, and since `'a` and `'b` are unrelated, there was no outlives constraint we could issue that would enforce that (`R: 'a` and `R: 'b` are both too strict, for example). But now we can issue a pick constraint: `pick R from ['a, 'b]`.

In general, solving pick constraints is tricky. We integrate them into the solver as follows. In general, during the propagation phase, we are monotonically growing a set of inference regions. To handle a case like `pick R from [O...]`, where `O...` represents the option regions, we do the following:

- Look for all the *lower bounds* of the region R -- that is, every region LB such that `R: LB` must hold.
- Look for all the *upper bounds* of the region R -- that is, every region UB such that `UB: R` must hold.
- Let the *viable options* be each option region O such that `UB: O` and `O: LB` for each UB, LB bound.
- Find the *minimal viable option* M, where `O: M` holds for every option region O.

If there is such a *minimal viable option*, then we make `R: M`. (This may in turn influence other bits of inference.) If there is no minimal viable option, either because all options were eliminated or because none of the remaining options are minimal, we do nothing. Ultimately, if the pick constraint is not satisfied, an error is reported.

For this logic, we currently require that the option regions O are always lifetime parameters. To determine the bounds, we walk the various outlives edges that were otherwise introduced.

r? @matthewjasper
cc @cramertj

Fixes #56238

TODO:

- [ ] Error messages include region variable info sometimes, how to fix?
- [ ] Tests for bare `existential type`  and other impl Trait usage
  • Loading branch information
bors committed Jul 3, 2019
2 parents 0beb2ba + f7e00a5 commit 8301de1
Show file tree
Hide file tree
Showing 71 changed files with 2,580 additions and 548 deletions.
29 changes: 29 additions & 0 deletions src/doc/unstable-book/src/language-features/member-constraints.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
# `member_constraints`

The tracking issue for this feature is: [#61977]

[#61977]: https://github.com/rust-lang/rust/issues/61977

------------------------

The `member_constraints` feature gate lets you use `impl Trait` syntax with
multiple unrelated lifetime parameters.

A simple example is:

```rust
#![feature(member_constraints)]

trait Trait<'a, 'b> { }
impl<T> Trait<'_, '_> for T {}

fn foo<'a, 'b>(x: &'a u32, y: &'b u32) -> impl Trait<'a, 'b> {
(x, y)
}

fn main() { }
```

Without the `member_constraints` feature gate, the above example is an
error because both `'a` and `'b` appear in the impl Trait bounds, but
neither outlives the other.
33 changes: 31 additions & 2 deletions src/librustc/infer/canonical/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@

use crate::infer::{InferCtxt, RegionVariableOrigin, TypeVariableOrigin, TypeVariableOriginKind};
use crate::infer::{ConstVariableOrigin, ConstVariableOriginKind};
use crate::infer::region_constraints::MemberConstraint;
use crate::mir::interpret::ConstValue;
use rustc_data_structures::indexed_vec::IndexVec;
use rustc_macros::HashStable;
Expand Down Expand Up @@ -189,11 +190,25 @@ pub enum CanonicalTyVarKind {
#[derive(Clone, Debug, HashStable)]
pub struct QueryResponse<'tcx, R> {
pub var_values: CanonicalVarValues<'tcx>,
pub region_constraints: Vec<QueryRegionConstraint<'tcx>>,
pub region_constraints: QueryRegionConstraints<'tcx>,
pub certainty: Certainty,
pub value: R,
}

#[derive(Clone, Debug, Default, HashStable)]
pub struct QueryRegionConstraints<'tcx> {
pub outlives: Vec<QueryOutlivesConstraint<'tcx>>,
pub member_constraints: Vec<MemberConstraint<'tcx>>,
}

impl QueryRegionConstraints<'_> {
/// Represents an empty (trivially true) set of region
/// constraints.
pub fn is_empty(&self) -> bool {
self.outlives.is_empty() && self.member_constraints.is_empty()
}
}

pub type Canonicalized<'tcx, V> = Canonical<'tcx, V>;

pub type CanonicalizedQueryResponse<'tcx, T> =
Expand Down Expand Up @@ -292,7 +307,8 @@ impl<'tcx, V> Canonical<'tcx, V> {
}
}

pub type QueryRegionConstraint<'tcx> = ty::Binder<ty::OutlivesPredicate<Kind<'tcx>, Region<'tcx>>>;
pub type QueryOutlivesConstraint<'tcx> =
ty::Binder<ty::OutlivesPredicate<Kind<'tcx>, Region<'tcx>>>;

impl<'cx, 'tcx> InferCtxt<'cx, 'tcx> {
/// Creates a substitution S for the canonical value with fresh
Expand Down Expand Up @@ -540,6 +556,19 @@ BraceStructLiftImpl! {
} where R: Lift<'tcx>
}

BraceStructTypeFoldableImpl! {
impl<'tcx> TypeFoldable<'tcx> for QueryRegionConstraints<'tcx> {
outlives, member_constraints
}
}

BraceStructLiftImpl! {
impl<'a, 'tcx> Lift<'tcx> for QueryRegionConstraints<'a> {
type Lifted = QueryRegionConstraints<'tcx>;
outlives, member_constraints
}
}

impl<'tcx> Index<BoundVar> for CanonicalVarValues<'tcx> {
type Output = Kind<'tcx>;

Expand Down
46 changes: 28 additions & 18 deletions src/librustc/infer/canonical/query_response.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ use crate::arena::ArenaAllocatable;
use crate::infer::canonical::substitute::substitute_value;
use crate::infer::canonical::{
Canonical, CanonicalVarValues, CanonicalizedQueryResponse, Certainty,
OriginalQueryValues, QueryRegionConstraint, QueryResponse,
OriginalQueryValues, QueryRegionConstraints, QueryOutlivesConstraint, QueryResponse,
};
use crate::infer::region_constraints::{Constraint, RegionConstraintData};
use crate::infer::InferCtxtBuilder;
Expand Down Expand Up @@ -132,7 +132,7 @@ impl<'cx, 'tcx> InferCtxt<'cx, 'tcx> {
{
self.canonicalize_response(&QueryResponse {
var_values: inference_vars,
region_constraints: vec![],
region_constraints: QueryRegionConstraints::default(),
certainty: Certainty::Proven, // Ambiguities are OK!
value: answer,
})
Expand Down Expand Up @@ -174,7 +174,7 @@ impl<'cx, 'tcx> InferCtxt<'cx, 'tcx> {

let region_obligations = self.take_registered_region_obligations();
let region_constraints = self.with_region_constraints(|region_constraints| {
make_query_outlives(
make_query_region_constraints(
tcx,
region_obligations
.iter()
Expand Down Expand Up @@ -222,10 +222,10 @@ impl<'cx, 'tcx> InferCtxt<'cx, 'tcx> {
mut obligations,
} = self.query_response_substitution(cause, param_env, original_values, query_response)?;

obligations.extend(self.query_region_constraints_into_obligations(
obligations.extend(self.query_outlives_constraints_into_obligations(
cause,
param_env,
&query_response.value.region_constraints,
&query_response.value.region_constraints.outlives,
&result_subst,
));

Expand All @@ -248,9 +248,9 @@ impl<'cx, 'tcx> InferCtxt<'cx, 'tcx> {
/// that come out of these queries, which it wants to convert into
/// MIR-based constraints and solve. Therefore, it is most
/// convenient for the NLL Type Checker to **directly consume**
/// the `QueryRegionConstraint` values that arise from doing a
/// the `QueryOutlivesConstraint` values that arise from doing a
/// query. This is contrast to other parts of the compiler, which
/// would prefer for those `QueryRegionConstraint` to be converted
/// would prefer for those `QueryOutlivesConstraint` to be converted
/// into the older infcx-style constraints (e.g., calls to
/// `sub_regions` or `register_region_obligation`).
///
Expand All @@ -263,7 +263,7 @@ impl<'cx, 'tcx> InferCtxt<'cx, 'tcx> {
/// result. If any errors arise, they are propagated back as an
/// `Err` result.
/// - In the case of a successful substitution, we will append
/// `QueryRegionConstraint` values onto the
/// `QueryOutlivesConstraint` values onto the
/// `output_query_region_constraints` vector for the solver to
/// use (if an error arises, some values may also be pushed, but
/// they should be ignored).
Expand All @@ -279,15 +279,15 @@ impl<'cx, 'tcx> InferCtxt<'cx, 'tcx> {
param_env: ty::ParamEnv<'tcx>,
original_values: &OriginalQueryValues<'tcx>,
query_response: &Canonical<'tcx, QueryResponse<'tcx, R>>,
output_query_region_constraints: &mut Vec<QueryRegionConstraint<'tcx>>,
output_query_region_constraints: &mut QueryRegionConstraints<'tcx>,
) -> InferResult<'tcx, R>
where
R: Debug + TypeFoldable<'tcx>,
{
let result_subst =
self.query_response_substitution_guess(cause, original_values, query_response);

// Compute `QueryRegionConstraint` values that unify each of
// Compute `QueryOutlivesConstraint` values that unify each of
// the original values `v_o` that was canonicalized into a
// variable...
let mut obligations = vec![];
Expand All @@ -306,8 +306,10 @@ impl<'cx, 'tcx> InferCtxt<'cx, 'tcx> {
// To make `v_o = v_r`, we emit `v_o: v_r` and `v_r: v_o`.
if v_o != v_r {
output_query_region_constraints
.outlives
.push(ty::Binder::dummy(ty::OutlivesPredicate(v_o.into(), v_r)));
output_query_region_constraints
.outlives
.push(ty::Binder::dummy(ty::OutlivesPredicate(v_r.into(), v_o)));
}
}
Expand All @@ -333,12 +335,12 @@ impl<'cx, 'tcx> InferCtxt<'cx, 'tcx> {
}

// ...also include the other query region constraints from the query.
output_query_region_constraints.extend(
query_response.value.region_constraints.iter().filter_map(|r_c| {
output_query_region_constraints.outlives.extend(
query_response.value.region_constraints.outlives.iter().filter_map(|r_c| {
let r_c = substitute_value(self.tcx, &result_subst, r_c);

// Screen out `'a: 'a` cases -- we skip the binder here but
// only care the inner values to one another, so they are still at
// only compare the inner values to one another, so they are still at
// consistent binding levels.
let &ty::OutlivesPredicate(k1, r2) = r_c.skip_binder();
if k1 != r2.into() {
Expand All @@ -349,6 +351,13 @@ impl<'cx, 'tcx> InferCtxt<'cx, 'tcx> {
})
);

// ...also include the query member constraints.
output_query_region_constraints.member_constraints.extend(
query_response.value.region_constraints.member_constraints.iter().map(|p_c| {
substitute_value(self.tcx, &result_subst, p_c)
})
);

let user_result: R =
query_response.substitute_projected(self.tcx, &result_subst, |q_r| &q_r.value);

Expand Down Expand Up @@ -560,11 +569,11 @@ impl<'cx, 'tcx> InferCtxt<'cx, 'tcx> {

/// Converts the region constraints resulting from a query into an
/// iterator of obligations.
fn query_region_constraints_into_obligations<'a>(
fn query_outlives_constraints_into_obligations<'a>(
&'a self,
cause: &'a ObligationCause<'tcx>,
param_env: ty::ParamEnv<'tcx>,
unsubstituted_region_constraints: &'a [QueryRegionConstraint<'tcx>],
unsubstituted_region_constraints: &'a [QueryOutlivesConstraint<'tcx>],
result_subst: &'a CanonicalVarValues<'tcx>,
) -> impl Iterator<Item = PredicateObligation<'tcx>> + 'a + Captures<'tcx> {
unsubstituted_region_constraints
Expand Down Expand Up @@ -645,15 +654,16 @@ impl<'cx, 'tcx> InferCtxt<'cx, 'tcx> {

/// Given the region obligations and constraints scraped from the infcx,
/// creates query region constraints.
pub fn make_query_outlives<'tcx>(
pub fn make_query_region_constraints<'tcx>(
tcx: TyCtxt<'tcx>,
outlives_obligations: impl Iterator<Item = (Ty<'tcx>, ty::Region<'tcx>)>,
region_constraints: &RegionConstraintData<'tcx>,
) -> Vec<QueryRegionConstraint<'tcx>> {
) -> QueryRegionConstraints<'tcx> {
let RegionConstraintData {
constraints,
verifys,
givens,
member_constraints,
} = region_constraints;

assert!(verifys.is_empty());
Expand Down Expand Up @@ -684,5 +694,5 @@ pub fn make_query_outlives<'tcx>(
)
.collect();

outlives
QueryRegionConstraints { outlives, member_constraints: member_constraints.clone() }
}
22 changes: 21 additions & 1 deletion src/librustc/infer/error_reporting/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@ use crate::infer::{self, SuppressRegionErrors};
use crate::hir;
use crate::hir::def_id::DefId;
use crate::hir::Node;
use crate::infer::opaque_types;
use crate::middle::region;
use crate::traits::{ObligationCause, ObligationCauseCode};
use crate::ty::error::TypeError;
Expand Down Expand Up @@ -375,6 +376,23 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> {
);
}
}

RegionResolutionError::MemberConstraintFailure {
opaque_type_def_id,
hidden_ty,
member_region,
span: _,
choice_regions: _,
} => {
let hidden_ty = self.resolve_vars_if_possible(&hidden_ty);
opaque_types::unexpected_hidden_region_diagnostic(
self.tcx,
Some(region_scope_tree),
opaque_type_def_id,
hidden_ty,
member_region,
).emit();
}
}
}
}
Expand Down Expand Up @@ -411,7 +429,8 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> {
let is_bound_failure = |e: &RegionResolutionError<'tcx>| match *e {
RegionResolutionError::GenericBoundFailure(..) => true,
RegionResolutionError::ConcreteFailure(..)
| RegionResolutionError::SubSupConflict(..) => false,
| RegionResolutionError::SubSupConflict(..)
| RegionResolutionError::MemberConstraintFailure { .. } => false,
};

let mut errors = if errors.iter().all(|e| is_bound_failure(e)) {
Expand All @@ -429,6 +448,7 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> {
RegionResolutionError::ConcreteFailure(ref sro, _, _) => sro.span(),
RegionResolutionError::GenericBoundFailure(ref sro, _, _) => sro.span(),
RegionResolutionError::SubSupConflict(_, ref rvo, _, _, _, _) => rvo.span(),
RegionResolutionError::MemberConstraintFailure { span, .. } => span,
});
errors
}
Expand Down
Loading

0 comments on commit 8301de1

Please sign in to comment.