Skip to content

Commit

Permalink
Make generator types always Freeze.
Browse files Browse the repository at this point in the history
When working on #57017, I discovered that generators are currently
considered `Freeze` based on the witness types they contain (just like
any other auto trait). That is, a generator which contains a `!Freeze`
type (e.g. `Cell<T>`) will also be `!Freeze`.

However, I believe that it is sound to always treat generator types as
`Freeze`, regardless of what types they may store internally. The only
possible of interacting with a generator is through the `resume` method
of the generator trait. `resume` takes a `Pin<&mut Self>`, which
requires a `&mut Self` to be created. In other words, given only a
shared reference to a generator (`&{generator}`), it is impossible to
mutate the generator. Note that is true regardless of whether or not
types *inside* the generator have interior mutability - they can only
change when `resume` is called, which is impossible to call using a
shared reference.

The motivation for this PR is to support further work on #57017. My
approach is to delay resolution of auto-trait predicates (e.g.
`{generator}: Send)` until after we've constructed the generator MIR
(specifically, after we've run the `StateTransform` MIR transformation
pass). However, the const qualification pass (which runs before
`StateTransform`) explictly checks whether types are `Freeze`. There are
several ways of resolving this:

1. Refactor const-qualification to avoid the need to check whether
generators are `Freeze`. This might involve running (part of ) const
qualification after the `StateTransform` pass runs.
2. Use the current, more conservative approach for generator when
checking `Freeze` - that is, use the witness types computed from the
HIR.
3. Make generators always `Freeze`.

Option 1 and 2 introduce a large amount of additional complexity for the
sole purpose of supporting generators. Option 3 (this PR), requires only
a minor change to `SelectionContext`. Theoretically, it could also
improve performance, since we now perform less work when checking for
`Freeze` types.

This should probably have a test, but I'm not really sure how to write
one, considering that `Freeze` is private.
  • Loading branch information
Aaron1011 committed Oct 24, 2019
1 parent b7a9c28 commit 0d02577
Showing 1 changed file with 9 additions and 0 deletions.
9 changes: 9 additions & 0 deletions src/librustc/traits/select.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2208,6 +2208,15 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
}
}
}
ty::Generator(..)
if self.tcx().lang_items().freeze_trait() == Some(def_id) =>
{
// Generators are always Freeze - it's impossible to do anything
// with them unless you have a mutable reference, so any interior
// mutability of types 'inside' them is not observable from
// outside the generator
candidates.vec.push(BuiltinCandidate { has_nested: false });
}

_ => candidates.vec.push(AutoImplCandidate(def_id.clone())),
}
Expand Down

0 comments on commit 0d02577

Please sign in to comment.