Skip to content
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

[RFC] Clarify (and improve) rules for projections and well-formedness #1214

Merged
merged 10 commits into from
Aug 7, 2015
Prev Previous commit
Next Next commit
rewrite the inference section to be less reliant on the compiler
implementation
  • Loading branch information
nikomatsakis committed Aug 7, 2015
commit abcd4be3b000864fcc89875d68e093040f1c74ca
83 changes: 64 additions & 19 deletions text/0000-projections-lifetimes-and-wf.md
Original file line number Diff line number Diff line change
Expand Up @@ -446,7 +446,7 @@ or projections are involved:
--------------------------------------------------
R ⊢ for<r..> fn(T1..Tn) -> T0
Copy link
Member

Choose a reason for hiding this comment

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

Is this missing a : 'a?


OutlivesTraitRef:
OutlivesFragment:
∀i. R,r.. ⊢ Pi: 'a
--------------------------------------------------
R ⊢ for<r..> TraitId<P0..Pn>: 'a
Copy link
Member

Choose a reason for hiding this comment

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

The grammar above does not contain anything called a "trait reference" - but this looks suspiciously similar to an "object type fragment". Furthermore, in the subsection "WF checking a trait reference" of the WF section, trait references and object type fragments seem to be equated. If these two are the same thing, maybe they should get the same name.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@RalfJung

The grammar above does not contain anything called a "trait reference" - but this looks suspiciously similar to an "object type fragment". Furthermore, in the subsection "WF checking a trait reference" of the WF section, trait references and object type fragments seem to be equated. If these two are the same thing, maybe they should get the same name.

They are not the same thing. An object fragment is a trait reference with an erased self type. You can think of it as \exists T0. <T0 as TraitRef<P1...Pn>.

Copy link
Member

Choose a reason for hiding this comment

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

A trait reference does not yet give a self type, does it? If I understood this correctly, a trait reference is what I write in places like "where T: Trait<...>".
Grammatically, the two look exactly equal: What you have in this "OutlivesTraitRef" rule is grammatically also a valid object type fragment, according to the grammar above. If the two are different, object references should be added to the grammar somewhere - and there should be some way to tell the two apart when they are mentioned in this document.

Also, the subsection "WF checking a trait reference" gives only one rule, called "WfObjectFragment". How does that make sense?

And finally, I cannot find the outlives rule for object fragments anywhere, or the WF rule for trait references.

EDIT: Actually, further down in the aforementioned "WF checking a trait reference" subsection, a trait reference looks grammatically different from an object fragment. It is written "for<r..> P: TraitId<P0..Pn>: 'a " (notice the "P:" after the quantifier.) So now I assume the rule down there has simply been mis-named, and the rule here is either mis-named or it lacks the "P:" in the conclusion.

Expand All @@ -471,6 +471,12 @@ implied bounds.
--------------------------------------------------
R ⊢ 'a: 'a

OutlivesRegionTransitive:
R ⊢ 'a: 'c
R ⊢ 'c: 'b
--------------------------------------------------
R ⊢ 'a: 'b

For higher-ranked lifetimes, we simply ignore the relation, since the
lifetime is not yet known. This means for example that `for<'a> fn(&'a
i32): 'x` holds, even though we do not yet know what region `'a` is
Expand Down Expand Up @@ -508,13 +514,15 @@ higher-ranked lifetimes. (This is somewhat stricter than necessary,
but reflects the behavior of my prototype implementation.)

OutlivesProjectionEnv:
<P0 as Trait<P1..Pn>>::Id: 'a in Env
<P0 as Trait<P1..Pn>>::Id: 'b in Env
<> ⊢ 'b: 'a
--------------------------------------------------
<> ⊢ <P0 as Trait<P1..Pn>>::Id: 'a

OutlivesProjectionTraitDef:
WC = [Xi => Pi] WhereClauses(Trait)
<P0 as Trait<P1..Pn>>::Id: 'a in WC
<P0 as Trait<P1..Pn>>::Id: 'b in WC
<> ⊢ 'b: 'a
--------------------------------------------------
<> ⊢ <P0 as Trait<P1..Pn>>::Id: 'a
Copy link
Member

Choose a reason for hiding this comment

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

Is it possible for there to be a case where the information from the environment and the trait definition needs to be combined? Like, say, the trait definition of T<'a, 'b> has "type Item: 'a", and then in a context where "'a: 'c" is in the environment, we need to know whether "T<'a, 'b>::Item: 'c". Neither of these two inference rules alone is good enough to capture this case, I think.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@RalfJung

Like, say, the trait definition of T<'a, 'b> has "type Item: 'a", and then in a context where "'a: 'c" is in the environment, we need to know whether "T<'a, 'b>::Item: 'c". Neither of these two inference rules alone is good enough to capture this case, I think.

Yes, that can happen. There is a transitive rule missing, I guess. It's also true that I've tweaked the impl somewhat in the meantime to make it more expressive. I should probably just write out the rules here to be more flexible, even if the current impl doesn't always find a correct solution.


Expand Down Expand Up @@ -624,16 +632,46 @@ any lifetime or type parameters.

#### Implementation complications

One complication for the implementation is that there are so many
potential outlives rules for projections. In particular, the rule that
says `<P0 as Trait<P1..Pn>>>: 'a` holds if `Pi: 'a` is not an "if and
only if" rule. So, for example, if we know that `T: 'a` and `'b: 'a`,
then we know that `<T as Trait<'b>>:: Item: 'a` (for any trait and
item), but not vice versa. This complicates inference significantly,
since if variables are involved, we do not know whether to create
edges between the variables or not (put another way, the simple
dataflow model we are currently using doesn't truly suffice for these
rules).
The current region inference code only permits constraints of the
form:

```
C = r0: r1
| C AND C
```

This is convenient because a simple fixed-point iteration suffices to
find the minimal regions which satisfy the constraints.

Unfortunately, this constraint model does not scale to the outlives
rules for projections. Consider a trait reference like `<T as
Trait<'X>>::Item: 'Y`, where `'X` and `'Y` are both region variables
whose value is being inferred. At this point, there are several
inference rules which could potentially apply. Let us assume that
there is a where-clause in the environment like `<T as
Trait<'a>>::Item: 'b`. In that case, *if* `'X == 'a` and `'b: 'Y`,
then we could employ the `OutlivesProjectionEnv` rule. This would
correspond to a constraint set like:

```
C = 'X:'a AND 'a:'X AND 'b:'Y
```

Otherwise, if `T: 'a` and `'X: 'Y`, then we could use the
`OutlivesProjectionComponents` rule, which would require a constraint
set like:

```
C = C1 AND 'X:'Y
```

where `C1` is the constraint set for `T:'a`.

As you can see, these two rules yielded distinct constraint sets.
Ideally, we would combine them with an `OR` constraint, but no such
constraint is available. Adding such a constraint complicates how
inference works, since a fixed-point iteration is no longer
sufficient.

This complication is unfortunate, but to a large extent already exists
with where-clauses and trait matching (see e.g. [#21974]). (Moreover,
Expand All @@ -642,11 +680,17 @@ take several inputs (the parameters to the trait) which may or may not
be related to the actual type definition in question.)

For the time being, the current implementation takes a pragmatic
approach based on heuristics. It tries to avoid adding edges to the
region graph in various common scenarios, and in the end falls back to
enforcing conditions that may be stricter than necessary, but which
certainly suffice. We have not yet encountered an example in practice
where the current implementation rules do not suffice.
approach based on heuristics. It first examines whether any region
bounds are declared in the trait and, if so, prefers to use
those. Otherwise, if there are region variables in the projection,
then it falls back to the `OutlivesProjectionComponents` rule. This is
always sufficient but may be stricter than necessary. If there are no
region variables in the projection, then it can simply run inference
to completion and check each of the other two rules in turn. (It is
still necessary to run inference because the bound may be a region
variable.) So far this approach has sufficed for all situations
encountered in practice. Eventually, we should extend the region
inferencer to a richer model that includes "OR" constraints.

### The WF relation

Expand Down Expand Up @@ -718,7 +762,8 @@ remainder are checked. For example, if we have a type `HashSet<K>`
which requires that `K: Hash`, then `fn(HashSet<NoHash>)` would be
illegal since `NoHash: Hash` does not hold, but `for<'a>
fn(HashSet<&'a NoHash>)` *would* be legal, since `&'a NoHash: Hash`
involves a bound region `'a`. See the next section for details.
involves a bound region `'a`. See the "Checking Conditions" section
for details.

Note that `fn` types do not require that `T0..Tn` be `Sized`. This is
intentional. The limitation that only sized values can be passed as
Expand Down