From 0d02577ff219a4c1202cf2654d6145d968a04279 Mon Sep 17 00:00:00 2001 From: Aaron Hill Date: Sun, 20 Oct 2019 20:55:55 -0400 Subject: [PATCH] Make generator types always Freeze. 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`) 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. --- src/librustc/traits/select.rs | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/src/librustc/traits/select.rs b/src/librustc/traits/select.rs index 44d611ace77d0..0f36e17931a91 100644 --- a/src/librustc/traits/select.rs +++ b/src/librustc/traits/select.rs @@ -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())), }