Skip to content

Commit 9303810

Browse files
committed
Use the recorded types in MIR to determine generator auto-trait implementations
When we construct a generator type, we build a `ty::GeneratorWitness` from a list of types that may live across a suspend point. These types are used to determine the 'constitutent types' for a generator when selecting an auto-trait predicate. Any types appearing in the GeneratorWitness are required to implement the auto trait (e.g. `Send` or `Sync`). This analysis is based on the HIR - as a result, it is unable to take liveness of variables into account. This often results in unnecessary bounds being computing (e.g requiring that a `Rc` be `Sync` even if it is dropped before a suspend point), making generators and `async fn`s much less ergonomic to write. This commit uses the generator MIR to determine the actual 'constituent types' of a generator. Specifically, a type in the generator witness is considered to be a 'constituent type' of the witness if it appears in the `field_tys` of the computed `GeneratorLayout`. Any type which is stored across an suspend point must be a constituent type (since it could be used again after a suspend), while any type that is not stored across a suspend point cannot possible be a constituent type (since it is impossible for it to be used again). By re-using the existing generator layout computation logic, we get some nice properties for free: * Types which are dead before the suspend point are not considered constituent types * Types without Drop impls not considered constituent types if their scope extends across an await point (assuming that they are never used after an await point). Note that this only affects `ty::GeneratorWitness`, *not* `ty::Generator` itself. Upvars (captured types from the parent scope) are considered to be constituent types of the base `ty::Generator`, not the inner `ty::GeneratorWitness`. This means that upvars are always considered constituent types - this is because by defintion, they always live across the first implicit suspend point. ------- Implementation: The most significant part of this PR is the introduction of a new 'delayed generator witness mode' to `TraitEngine`. As @nikomatsakis pointed out, attmepting to compute generator MIR during type-checking results in the following cycle: 1. We attempt to type-check a generator's parent function 2. During type checking of the parent function, we record a predicate of the form `<generator>: AutoTrait` 3. We add this predicate to a `TraitEngine`, and attempt to fulfill it. 4. When we atempt to select the predicate, we attempt to compute the MIR for `<generator>` 5. The MIR query attempts to compute `type_of(generator_def_id)`, which results in us attempting to type-check the generator's parent function. To break this cycle, we defer processing of all auto-trait predicates involving `ty::GeneratorWitness`. These predicates are recorded in the `TypeckTables` for the parent function. During MIR type-checking of the parent function, we actually attempt to fulfill these predicates, reporting any errors that occur. The rest of the PR is mostly fallout from this change: * `ty::GeneratorWitness` now stores the `DefId` of its generator. This allows us to retrieve the MIR for the generator when `SelectionContext` processes a predicate involving a `ty::GeneratorWitness` * Since we now store `PredicateObligations` in `TypeckTables`, several different types have now become `RustcEncodable`/`RustcDecodable`. These are purely mechanical changes (adding new `#[derives]`), with one exception - a new `SpecializedDecoder` impl for `List<Predicate>`. This was essentially identical to other `SpecializedDecoder` imps, but it would be good to have someone check it over. * When we delay processing of a `Predicate`, we move it from one `InferCtxt` to another. This requires us to prevent any inference variables from leaking out from the first `InferCtxt` - if used in another `InferCtxt`, they will either be non-existent or refer to the the wrong variable. Fortunately, the predicate itself has no region variables - the `ty::GeneratorWitness` has only late-bound regions, while auto-traits have no generic parameters whatsoever. However, we still need to deal with the `ObligationCause` stored by the `PredicateObligation`. An `ObligationCause` (or a nested cause) may have any number of region variables stored inside it (e.g. from stored types). Luckily, `ObligationCause` is only used for error reporting, so we can safely erase all regions variables from it, without affecting the actual processing of the obligation. To accomplish this, I took the somewhat unusual approach of implementing `TypeFoldable` for `ObligationCause`, but did *not* change the `TypeFoldable` implementation of `Obligation` to fold its contained `ObligationCause. Other than this one odd case, all other callers of `TypeFoldable` have no interest in folding an `ObligationCause`. As a result, we explicitly fold the `ObligationCause` when computing our deferred generator witness predicates. Since `ObligationCause` is only used for displaying error messages, the worst that can happen is that a slightly odd error message is displayed to a user. With this change, several tests now have fewer errors than they did previously, due to the improved generator analysis. Unfortunately, this does not resolve issue rust-lang#64960. The MIR generator transformation stores format temporaries in the generator, due to the fact that the `format!` macro takes a reference to them. As a result, they are still considered constituent types of the `GeneratorWitness`, and are still required to implement `Send` and `Sync. * I've changed the pretty-printing of `ty::GeneratorWitness` to print out its generator DefId, as well as the word `witness`. This makes debugging issues related to the computation of constituent types much simpler. As a final note, this PR contains one unrelated change - all generators now implement `Freeze` unconditionally. I've opened a separate PR containing this change - however, it's necessary to allow this branch to compile without cycle errors. I've left it in this PR to make it easy to test out this branch on actual code. Assuming that it is accepted, this PR will be rebased against `master` when it is merged. Otherwise, I'll need to figure out a different approach to generator const-qualification.
1 parent 41601a8 commit 9303810

32 files changed

+628
-88
lines changed

src/librustc/hir/mod.rs

+4
Original file line numberDiff line numberDiff line change
@@ -89,6 +89,10 @@ impl HirId {
8989
}
9090
}
9191

92+
CloneTypeFoldableImpls! {
93+
HirId,
94+
}
95+
9296
impl rustc_serialize::UseSpecializedEncodable for HirId {
9397
fn default_encode<S: Encoder>(&self, s: &mut S) -> Result<(), S::Error> {
9498
let HirId {

src/librustc/traits/chalk_fulfill.rs

+42-1
Original file line numberDiff line numberDiff line change
@@ -9,23 +9,39 @@ use crate::traits::{
99
SelectionError,
1010
};
1111
use crate::traits::query::NoSolution;
12+
use crate::traits::DelayedGenerators;
1213
use crate::infer::InferCtxt;
1314
use crate::infer::canonical::{Canonical, OriginalQueryValues};
14-
use crate::ty::{self, Ty};
15+
use crate::ty::{self, Ty, Predicate};
1516
use rustc_data_structures::fx::FxHashSet;
1617

1718
pub type CanonicalGoal<'tcx> = Canonical<'tcx, InEnvironment<'tcx, ty::Predicate<'tcx>>>;
1819

1920
pub struct FulfillmentContext<'tcx> {
2021
obligations: FxHashSet<InEnvironment<'tcx, PredicateObligation<'tcx>>>,
22+
// See FulfillmentContext for more details about delayed generator
23+
// witness obligations
24+
delayed_generator_witnesses: DelayedGenerators<'tcx>,
25+
has_delayed_generator_witnesses: bool
2126
}
2227

2328
impl FulfillmentContext<'tcx> {
2429
crate fn new() -> Self {
2530
FulfillmentContext {
2631
obligations: FxHashSet::default(),
32+
delayed_generator_witnesses: None,
33+
has_delayed_generator_witnesses: false
2734
}
2835
}
36+
37+
crate fn with_delayed_generator_witness() -> Self {
38+
FulfillmentContext {
39+
obligations: FxHashSet::default(),
40+
delayed_generator_witnesses: Some(Vec::new()),
41+
has_delayed_generator_witnesses: true
42+
}
43+
44+
}
2945
}
3046

3147
fn in_environment(
@@ -108,6 +124,24 @@ impl TraitEngine<'tcx> for FulfillmentContext<'tcx> {
108124
goal: obligation.goal.predicate,
109125
}, &mut orig_values);
110126

127+
// See FulfillmentContext for more details about this
128+
if self.has_delayed_generator_witnesses {
129+
130+
if let Predicate::Trait(p) = obligation.goal.predicate {
131+
if let ty::GeneratorWitness(..) = p.skip_binder().self_ty().kind {
132+
if infcx.tcx.trait_is_auto(p.def_id()) {
133+
debug!("delaying generator witness predicate {:?}",
134+
obligation.goal.predicate);
135+
136+
self.delayed_generator_witnesses.as_mut()
137+
.expect("Already consumed generator witnesses!")
138+
.push(obligation.goal);
139+
continue;
140+
}
141+
}
142+
}
143+
}
144+
111145
match infcx.tcx.evaluate_goal(canonical_goal) {
112146
Ok(response) => {
113147
if response.is_proven() {
@@ -165,4 +199,11 @@ impl TraitEngine<'tcx> for FulfillmentContext<'tcx> {
165199
fn pending_obligations(&self) -> Vec<PredicateObligation<'tcx>> {
166200
self.obligations.iter().map(|obligation| obligation.goal.clone()).collect()
167201
}
202+
203+
fn delayed_generator_obligations(&mut self) -> Option<Vec<PredicateObligation<'tcx>>> {
204+
if !self.has_delayed_generator_witnesses {
205+
panic!("Tried to retrieve delayed generators in wrong mode!")
206+
}
207+
self.delayed_generator_witnesses.take()
208+
}
168209
}

src/librustc/traits/engine.rs

+40
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,22 @@ pub trait TraitEngine<'tcx>: 'tcx {
5555
) -> Result<(), Vec<FulfillmentError<'tcx>>>;
5656

5757
fn pending_obligations(&self) -> Vec<PredicateObligation<'tcx>>;
58+
59+
/// Retrieves the list of delayed generator witness predicates
60+
/// stored by this `TraitEngine`. This `TraitEngine` must have been
61+
/// created by `TraitEngine::with_delayed_generator_witness` - otherwise,
62+
/// this method will panic.
63+
///
64+
/// Calling this method consumes the underling `Vec` - subsequent calls
65+
/// will return `None`.
66+
///
67+
/// This method *MUST* be called on a `TraitEngine` created by
68+
/// `with_delayed_generator_witness` - if the `TraitEngine` is dropped
69+
/// without this method being called, a panic will occur. This ensures
70+
/// that the caller explicitly acknowledges these stored predicates -
71+
/// failure to do so will result in unsound code being accepted by
72+
/// the compiler.
73+
fn delayed_generator_obligations(&mut self) -> Option<Vec<PredicateObligation<'tcx>>>;
5874
}
5975

6076
pub trait TraitEngineExt<'tcx> {
@@ -85,4 +101,28 @@ impl dyn TraitEngine<'tcx> {
85101
Box::new(FulfillmentContext::new())
86102
}
87103
}
104+
105+
/// Creates a `TraitEngine` in a special 'delay generator witness' mode.
106+
/// This imposes additional requirements for the caller in order to avoid
107+
/// accepting unsound code, and should only be used by `FnCtxt`. All other
108+
/// users of `TraitEngine` should use `TraitEngine::new`
109+
///
110+
/// A `TraitEngine` returned by this constructor will not attempt
111+
/// to resolve any `GeneratorWitness` predicates involving auto traits,
112+
/// Specifically, predicates of the form:
113+
///
114+
/// `<GeneratorWitness>: MyTrait` where `MyTrait` is an auto-trait
115+
/// will be stored for later retrieval by `delayed_generator_obligations`.
116+
/// The caller of this code *MUST* register these predicates with a
117+
/// regular `TraitEngine` (created with `TraitEngine::new`) at some point.
118+
/// Otherwise, these predicates will never be evaluated, resulting in
119+
/// unsound programs being accepted by the compiler.
120+
pub fn with_delayed_generator_witness(tcx: TyCtxt<'tcx>) -> Box<Self> {
121+
if tcx.sess.opts.debugging_opts.chalk {
122+
Box::new(ChalkFulfillmentContext::with_delayed_generator_witness())
123+
} else {
124+
Box::new(FulfillmentContext::with_delayed_generator_witness())
125+
}
126+
127+
}
88128
}

src/librustc/traits/error_reporting.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -2181,7 +2181,7 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> {
21812181
ty::Adt(ty::AdtDef { did, .. }, ..) if
21822182
self.tcx.is_diagnostic_item(sym::gen_future, *did) => {},
21832183
ty::Generator(did, ..) => generator = generator.or(Some(did)),
2184-
ty::GeneratorWitness(_) | ty::Opaque(..) => {},
2184+
ty::GeneratorWitness(..) | ty::Opaque(..) => {},
21852185
_ => return false,
21862186
}
21872187

src/librustc/traits/fulfill.rs

+57-3
Original file line numberDiff line numberDiff line change
@@ -15,14 +15,15 @@ use super::{FulfillmentError, FulfillmentErrorCode};
1515
use super::{ObligationCause, PredicateObligation};
1616
use super::project;
1717
use super::select::SelectionContext;
18-
use super::{Unimplemented, ConstEvalFailure};
18+
use super::{Unimplemented, ConstEvalFailure, DelayedGenerators};
1919

2020
impl<'tcx> ForestObligation for PendingPredicateObligation<'tcx> {
2121
type Predicate = ty::Predicate<'tcx>;
2222

2323
fn as_predicate(&self) -> &Self::Predicate { &self.obligation.predicate }
2424
}
2525

26+
2627
/// The fulfillment context is used to drive trait resolution. It
2728
/// consists of a list of obligations that must be (eventually)
2829
/// satisfied. The job is to track which are satisfied, which yielded
@@ -59,7 +60,17 @@ pub struct FulfillmentContext<'tcx> {
5960
// other fulfillment contexts sometimes do live inside of
6061
// a snapshot (they don't *straddle* a snapshot, so there
6162
// is no trouble there).
62-
usable_in_snapshot: bool
63+
usable_in_snapshot: bool,
64+
65+
// Whether or not we show delay the selection of predicates
66+
// involving `ty::GeneratorWitness`.
67+
// This is used by `FnCtxt` to delay the checking of
68+
// `GeneratorWitness` predicates, since generator MIR is
69+
// not available when `FnCxtxt` is run.
70+
has_delayed_generator_witness: bool,
71+
// The delayed generator witness predicates. This is only
72+
// used when `has_delayed_generator_witness:` is `true`
73+
delayed_generator_witness: DelayedGenerators<'tcx>,
6374
}
6475

6576
#[derive(Clone, Debug)]
@@ -79,6 +90,18 @@ impl<'a, 'tcx> FulfillmentContext<'tcx> {
7990
predicates: ObligationForest::new(),
8091
register_region_obligations: true,
8192
usable_in_snapshot: false,
93+
delayed_generator_witness: None,
94+
has_delayed_generator_witness: false
95+
}
96+
}
97+
98+
pub fn with_delayed_generator_witness() -> FulfillmentContext<'tcx> {
99+
FulfillmentContext {
100+
predicates: ObligationForest::new(),
101+
register_region_obligations: true,
102+
usable_in_snapshot: false,
103+
delayed_generator_witness: Some(Vec::new()),
104+
has_delayed_generator_witness: true
82105
}
83106
}
84107

@@ -87,14 +110,18 @@ impl<'a, 'tcx> FulfillmentContext<'tcx> {
87110
predicates: ObligationForest::new(),
88111
register_region_obligations: true,
89112
usable_in_snapshot: true,
113+
delayed_generator_witness: None,
114+
has_delayed_generator_witness: false
90115
}
91116
}
92117

93118
pub fn new_ignoring_regions() -> FulfillmentContext<'tcx> {
94119
FulfillmentContext {
95120
predicates: ObligationForest::new(),
96121
register_region_obligations: false,
97-
usable_in_snapshot: false
122+
usable_in_snapshot: false,
123+
delayed_generator_witness: None,
124+
has_delayed_generator_witness: false
98125
}
99126
}
100127

@@ -112,6 +139,8 @@ impl<'a, 'tcx> FulfillmentContext<'tcx> {
112139

113140
// Process pending obligations.
114141
let outcome = self.predicates.process_obligations(&mut FulfillProcessor {
142+
has_delayed_generator_witness: self.has_delayed_generator_witness,
143+
delayed_generator_witness: &mut self.delayed_generator_witness,
115144
selcx,
116145
register_region_obligations: self.register_region_obligations
117146
}, DoCompleted::No);
@@ -226,10 +255,19 @@ impl<'tcx> TraitEngine<'tcx> for FulfillmentContext<'tcx> {
226255
fn pending_obligations(&self) -> Vec<PredicateObligation<'tcx>> {
227256
self.predicates.map_pending_obligations(|o| o.obligation.clone())
228257
}
258+
259+
fn delayed_generator_obligations(&mut self) -> Option<Vec<PredicateObligation<'tcx>>> {
260+
if !self.has_delayed_generator_witness {
261+
panic!("Tried to retrieve delayed generators in wrong mode!")
262+
}
263+
self.delayed_generator_witness.take()
264+
}
229265
}
230266

231267
struct FulfillProcessor<'a, 'b, 'tcx> {
232268
selcx: &'a mut SelectionContext<'b, 'tcx>,
269+
has_delayed_generator_witness: bool,
270+
delayed_generator_witness: &'a mut DelayedGenerators<'tcx>,
233271
register_region_obligations: bool,
234272
}
235273

@@ -309,6 +347,21 @@ impl<'a, 'b, 'tcx> ObligationProcessor for FulfillProcessor<'a, 'b, 'tcx> {
309347
ty::Predicate::Trait(ref data) => {
310348
let trait_obligation = obligation.with(data.clone());
311349

350+
351+
if self.has_delayed_generator_witness &&
352+
self.selcx.tcx().trait_is_auto(data.def_id()) {
353+
// Ok to skip binder - bound regions do not affect whether self
354+
// type is a `GeneratorWitness
355+
if let ty::GeneratorWitness(..) = data.skip_binder().self_ty().kind {
356+
debug!("delaying generator witness predicate `{:?}` at depth {}",
357+
data, obligation.recursion_depth);
358+
self.delayed_generator_witness.as_mut()
359+
.expect("Delayed generator witnesses already consumed!")
360+
.push(obligation.clone());
361+
return ProcessResult::Changed(vec![])
362+
}
363+
}
364+
312365
if data.is_global() {
313366
// no type variables present, can use evaluation for better caching.
314367
// FIXME: consider caching errors too.
@@ -319,6 +372,7 @@ impl<'a, 'b, 'tcx> ObligationProcessor for FulfillProcessor<'a, 'b, 'tcx> {
319372
}
320373
}
321374

375+
322376
match self.selcx.select(&trait_obligation) {
323377
Ok(Some(vtable)) => {
324378
debug!("selecting trait `{:?}` at depth {} yielded Ok(Some)",

0 commit comments

Comments
 (0)