From 930381060f4ef2fb8d1cf38509a0900f2c08ba7e Mon Sep 17 00:00:00 2001 From: Aaron Hill Date: Thu, 17 Oct 2019 19:58:25 -0400 Subject: [PATCH 01/15] 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 `: 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 `` 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`. 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 #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. --- src/librustc/hir/mod.rs | 4 + src/librustc/traits/chalk_fulfill.rs | 43 ++++- src/librustc/traits/engine.rs | 40 +++++ src/librustc/traits/error_reporting.rs | 2 +- src/librustc/traits/fulfill.rs | 60 ++++++- src/librustc/traits/mod.rs | 100 ++++++++++- src/librustc/traits/project.rs | 4 +- src/librustc/traits/select.rs | 89 +++++++++- src/librustc/ty/codec.rs | 20 +++ src/librustc/ty/context.rs | 16 +- src/librustc/ty/error.rs | 5 +- src/librustc/ty/fast_reject.rs | 2 +- src/librustc/ty/flags.rs | 2 +- src/librustc/ty/mod.rs | 7 +- src/librustc/ty/print/obsolete.rs | 2 +- src/librustc/ty/print/pretty.rs | 4 +- src/librustc/ty/relate.rs | 5 +- src/librustc/ty/structural_impls.rs | 4 +- src/librustc/ty/sty.rs | 8 +- src/librustc/ty/walk.rs | 2 +- src/librustc_codegen_utils/symbol_names/v0.rs | 2 +- .../borrow_check/nll/type_check/mod.rs | 79 ++++++++- .../interpret/intrinsics/type_name.rs | 2 +- src/librustc_mir/transform/generator.rs | 4 +- .../check/generator_interior.rs | 3 +- src/librustc_typeck/check/mod.rs | 165 ++++++++++++++++-- src/librustc_typeck/check/writeback.rs | 8 +- src/test/ui/async-await/async-fn-nonsend.rs | 6 +- src/test/ui/generator/auto-trait-regions.rs | 6 +- .../ui/generator/auto-trait-regions.stderr | 2 +- src/test/ui/generator/not-send-sync.rs | 1 - src/test/ui/generator/not-send-sync.stderr | 19 +- 32 files changed, 628 insertions(+), 88 deletions(-) diff --git a/src/librustc/hir/mod.rs b/src/librustc/hir/mod.rs index 66bb3a8d883a4..207b09af144b5 100644 --- a/src/librustc/hir/mod.rs +++ b/src/librustc/hir/mod.rs @@ -89,6 +89,10 @@ impl HirId { } } +CloneTypeFoldableImpls! { + HirId, +} + impl rustc_serialize::UseSpecializedEncodable for HirId { fn default_encode(&self, s: &mut S) -> Result<(), S::Error> { let HirId { diff --git a/src/librustc/traits/chalk_fulfill.rs b/src/librustc/traits/chalk_fulfill.rs index d9e83df7ddda6..bd956ec38f7de 100644 --- a/src/librustc/traits/chalk_fulfill.rs +++ b/src/librustc/traits/chalk_fulfill.rs @@ -9,23 +9,39 @@ use crate::traits::{ SelectionError, }; use crate::traits::query::NoSolution; +use crate::traits::DelayedGenerators; use crate::infer::InferCtxt; use crate::infer::canonical::{Canonical, OriginalQueryValues}; -use crate::ty::{self, Ty}; +use crate::ty::{self, Ty, Predicate}; use rustc_data_structures::fx::FxHashSet; pub type CanonicalGoal<'tcx> = Canonical<'tcx, InEnvironment<'tcx, ty::Predicate<'tcx>>>; pub struct FulfillmentContext<'tcx> { obligations: FxHashSet>>, + // See FulfillmentContext for more details about delayed generator + // witness obligations + delayed_generator_witnesses: DelayedGenerators<'tcx>, + has_delayed_generator_witnesses: bool } impl FulfillmentContext<'tcx> { crate fn new() -> Self { FulfillmentContext { obligations: FxHashSet::default(), + delayed_generator_witnesses: None, + has_delayed_generator_witnesses: false } } + + crate fn with_delayed_generator_witness() -> Self { + FulfillmentContext { + obligations: FxHashSet::default(), + delayed_generator_witnesses: Some(Vec::new()), + has_delayed_generator_witnesses: true + } + + } } fn in_environment( @@ -108,6 +124,24 @@ impl TraitEngine<'tcx> for FulfillmentContext<'tcx> { goal: obligation.goal.predicate, }, &mut orig_values); + // See FulfillmentContext for more details about this + if self.has_delayed_generator_witnesses { + + if let Predicate::Trait(p) = obligation.goal.predicate { + if let ty::GeneratorWitness(..) = p.skip_binder().self_ty().kind { + if infcx.tcx.trait_is_auto(p.def_id()) { + debug!("delaying generator witness predicate {:?}", + obligation.goal.predicate); + + self.delayed_generator_witnesses.as_mut() + .expect("Already consumed generator witnesses!") + .push(obligation.goal); + continue; + } + } + } + } + match infcx.tcx.evaluate_goal(canonical_goal) { Ok(response) => { if response.is_proven() { @@ -165,4 +199,11 @@ impl TraitEngine<'tcx> for FulfillmentContext<'tcx> { fn pending_obligations(&self) -> Vec> { self.obligations.iter().map(|obligation| obligation.goal.clone()).collect() } + + fn delayed_generator_obligations(&mut self) -> Option>> { + if !self.has_delayed_generator_witnesses { + panic!("Tried to retrieve delayed generators in wrong mode!") + } + self.delayed_generator_witnesses.take() + } } diff --git a/src/librustc/traits/engine.rs b/src/librustc/traits/engine.rs index b96126c7b6735..75b23a722b9dc 100644 --- a/src/librustc/traits/engine.rs +++ b/src/librustc/traits/engine.rs @@ -55,6 +55,22 @@ pub trait TraitEngine<'tcx>: 'tcx { ) -> Result<(), Vec>>; fn pending_obligations(&self) -> Vec>; + + /// Retrieves the list of delayed generator witness predicates + /// stored by this `TraitEngine`. This `TraitEngine` must have been + /// created by `TraitEngine::with_delayed_generator_witness` - otherwise, + /// this method will panic. + /// + /// Calling this method consumes the underling `Vec` - subsequent calls + /// will return `None`. + /// + /// This method *MUST* be called on a `TraitEngine` created by + /// `with_delayed_generator_witness` - if the `TraitEngine` is dropped + /// without this method being called, a panic will occur. This ensures + /// that the caller explicitly acknowledges these stored predicates - + /// failure to do so will result in unsound code being accepted by + /// the compiler. + fn delayed_generator_obligations(&mut self) -> Option>>; } pub trait TraitEngineExt<'tcx> { @@ -85,4 +101,28 @@ impl dyn TraitEngine<'tcx> { Box::new(FulfillmentContext::new()) } } + + /// Creates a `TraitEngine` in a special 'delay generator witness' mode. + /// This imposes additional requirements for the caller in order to avoid + /// accepting unsound code, and should only be used by `FnCtxt`. All other + /// users of `TraitEngine` should use `TraitEngine::new` + /// + /// A `TraitEngine` returned by this constructor will not attempt + /// to resolve any `GeneratorWitness` predicates involving auto traits, + /// Specifically, predicates of the form: + /// + /// `: MyTrait` where `MyTrait` is an auto-trait + /// will be stored for later retrieval by `delayed_generator_obligations`. + /// The caller of this code *MUST* register these predicates with a + /// regular `TraitEngine` (created with `TraitEngine::new`) at some point. + /// Otherwise, these predicates will never be evaluated, resulting in + /// unsound programs being accepted by the compiler. + pub fn with_delayed_generator_witness(tcx: TyCtxt<'tcx>) -> Box { + if tcx.sess.opts.debugging_opts.chalk { + Box::new(ChalkFulfillmentContext::with_delayed_generator_witness()) + } else { + Box::new(FulfillmentContext::with_delayed_generator_witness()) + } + + } } diff --git a/src/librustc/traits/error_reporting.rs b/src/librustc/traits/error_reporting.rs index 35017d6330da3..384686ae2ad08 100644 --- a/src/librustc/traits/error_reporting.rs +++ b/src/librustc/traits/error_reporting.rs @@ -2181,7 +2181,7 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> { ty::Adt(ty::AdtDef { did, .. }, ..) if self.tcx.is_diagnostic_item(sym::gen_future, *did) => {}, ty::Generator(did, ..) => generator = generator.or(Some(did)), - ty::GeneratorWitness(_) | ty::Opaque(..) => {}, + ty::GeneratorWitness(..) | ty::Opaque(..) => {}, _ => return false, } diff --git a/src/librustc/traits/fulfill.rs b/src/librustc/traits/fulfill.rs index a981162fdc326..e6b37b8e192e6 100644 --- a/src/librustc/traits/fulfill.rs +++ b/src/librustc/traits/fulfill.rs @@ -15,7 +15,7 @@ use super::{FulfillmentError, FulfillmentErrorCode}; use super::{ObligationCause, PredicateObligation}; use super::project; use super::select::SelectionContext; -use super::{Unimplemented, ConstEvalFailure}; +use super::{Unimplemented, ConstEvalFailure, DelayedGenerators}; impl<'tcx> ForestObligation for PendingPredicateObligation<'tcx> { type Predicate = ty::Predicate<'tcx>; @@ -23,6 +23,7 @@ impl<'tcx> ForestObligation for PendingPredicateObligation<'tcx> { fn as_predicate(&self) -> &Self::Predicate { &self.obligation.predicate } } + /// The fulfillment context is used to drive trait resolution. It /// consists of a list of obligations that must be (eventually) /// satisfied. The job is to track which are satisfied, which yielded @@ -59,7 +60,17 @@ pub struct FulfillmentContext<'tcx> { // other fulfillment contexts sometimes do live inside of // a snapshot (they don't *straddle* a snapshot, so there // is no trouble there). - usable_in_snapshot: bool + usable_in_snapshot: bool, + + // Whether or not we show delay the selection of predicates + // involving `ty::GeneratorWitness`. + // This is used by `FnCtxt` to delay the checking of + // `GeneratorWitness` predicates, since generator MIR is + // not available when `FnCxtxt` is run. + has_delayed_generator_witness: bool, + // The delayed generator witness predicates. This is only + // used when `has_delayed_generator_witness:` is `true` + delayed_generator_witness: DelayedGenerators<'tcx>, } #[derive(Clone, Debug)] @@ -79,6 +90,18 @@ impl<'a, 'tcx> FulfillmentContext<'tcx> { predicates: ObligationForest::new(), register_region_obligations: true, usable_in_snapshot: false, + delayed_generator_witness: None, + has_delayed_generator_witness: false + } + } + + pub fn with_delayed_generator_witness() -> FulfillmentContext<'tcx> { + FulfillmentContext { + predicates: ObligationForest::new(), + register_region_obligations: true, + usable_in_snapshot: false, + delayed_generator_witness: Some(Vec::new()), + has_delayed_generator_witness: true } } @@ -87,6 +110,8 @@ impl<'a, 'tcx> FulfillmentContext<'tcx> { predicates: ObligationForest::new(), register_region_obligations: true, usable_in_snapshot: true, + delayed_generator_witness: None, + has_delayed_generator_witness: false } } @@ -94,7 +119,9 @@ impl<'a, 'tcx> FulfillmentContext<'tcx> { FulfillmentContext { predicates: ObligationForest::new(), register_region_obligations: false, - usable_in_snapshot: false + usable_in_snapshot: false, + delayed_generator_witness: None, + has_delayed_generator_witness: false } } @@ -112,6 +139,8 @@ impl<'a, 'tcx> FulfillmentContext<'tcx> { // Process pending obligations. let outcome = self.predicates.process_obligations(&mut FulfillProcessor { + has_delayed_generator_witness: self.has_delayed_generator_witness, + delayed_generator_witness: &mut self.delayed_generator_witness, selcx, register_region_obligations: self.register_region_obligations }, DoCompleted::No); @@ -226,10 +255,19 @@ impl<'tcx> TraitEngine<'tcx> for FulfillmentContext<'tcx> { fn pending_obligations(&self) -> Vec> { self.predicates.map_pending_obligations(|o| o.obligation.clone()) } + + fn delayed_generator_obligations(&mut self) -> Option>> { + if !self.has_delayed_generator_witness { + panic!("Tried to retrieve delayed generators in wrong mode!") + } + self.delayed_generator_witness.take() + } } struct FulfillProcessor<'a, 'b, 'tcx> { selcx: &'a mut SelectionContext<'b, 'tcx>, + has_delayed_generator_witness: bool, + delayed_generator_witness: &'a mut DelayedGenerators<'tcx>, register_region_obligations: bool, } @@ -309,6 +347,21 @@ impl<'a, 'b, 'tcx> ObligationProcessor for FulfillProcessor<'a, 'b, 'tcx> { ty::Predicate::Trait(ref data) => { let trait_obligation = obligation.with(data.clone()); + + if self.has_delayed_generator_witness && + self.selcx.tcx().trait_is_auto(data.def_id()) { + // Ok to skip binder - bound regions do not affect whether self + // type is a `GeneratorWitness + if let ty::GeneratorWitness(..) = data.skip_binder().self_ty().kind { + debug!("delaying generator witness predicate `{:?}` at depth {}", + data, obligation.recursion_depth); + self.delayed_generator_witness.as_mut() + .expect("Delayed generator witnesses already consumed!") + .push(obligation.clone()); + return ProcessResult::Changed(vec![]) + } + } + if data.is_global() { // no type variables present, can use evaluation for better caching. // FIXME: consider caching errors too. @@ -319,6 +372,7 @@ impl<'a, 'b, 'tcx> ObligationProcessor for FulfillProcessor<'a, 'b, 'tcx> { } } + match self.selcx.select(&trait_obligation) { Ok(Some(vtable)) => { debug!("selecting trait `{:?}` at depth {} yielded Ok(Some)", diff --git a/src/librustc/traits/mod.rs b/src/librustc/traits/mod.rs index d94e004db2978..5717b518b0a8e 100644 --- a/src/librustc/traits/mod.rs +++ b/src/librustc/traits/mod.rs @@ -75,6 +75,9 @@ pub use self::FulfillmentErrorCode::*; pub use self::SelectionError::*; pub use self::Vtable::*; +pub type DelayedGenerators<'tcx> = Option>>; + + /// Whether to enable bug compatibility with issue #43355. #[derive(Copy, Clone, PartialEq, Eq, Debug)] pub enum IntercrateMode { @@ -101,9 +104,13 @@ pub enum TraitQueryMode { /// either identifying an `impl` (e.g., `impl Eq for int`) that /// provides the required vtable, or else finding a bound that is in /// scope. The eventual result is usually a `Selection` (defined below). -#[derive(Clone, PartialEq, Eq, Hash)] +#[derive(Clone, PartialEq, Eq, Hash, RustcEncodable, RustcDecodable, HashStable)] pub struct Obligation<'tcx, T> { /// The reason we have to prove this thing. + /// Note that this is *not* folded by the `TypeFoldable` + /// impl for `Obligation`. This is almost always the correct + /// behavior - an `ObligationCause` is only for generating error messages, + /// and should have no effect on type-checking or trait resolution. pub cause: ObligationCause<'tcx>, /// The environment in which we should prove this thing. @@ -128,7 +135,7 @@ pub type TraitObligation<'tcx> = Obligation<'tcx, ty::PolyTraitPredicate<'tcx>>; static_assert_size!(PredicateObligation<'_>, 112); /// The reason why we incurred this obligation; used for error reporting. -#[derive(Clone, Debug, PartialEq, Eq, Hash)] +#[derive(Clone, Debug, PartialEq, Eq, Hash, HashStable, RustcEncodable, RustcDecodable)] pub struct ObligationCause<'tcx> { pub span: Span, @@ -158,7 +165,13 @@ impl<'tcx> ObligationCause<'tcx> { } } -#[derive(Clone, Debug, PartialEq, Eq, Hash)] +BraceStructTypeFoldableImpl! { + impl<'tcx> TypeFoldable<'tcx> for ObligationCause<'tcx> { + span, body_id, code + } +} + +#[derive(Clone, Debug, PartialEq, Eq, Hash, HashStable, RustcEncodable, RustcDecodable)] pub enum ObligationCauseCode<'tcx> { /// Not well classified or should be obvious from the span. MiscObligation, @@ -290,7 +303,57 @@ pub struct AssocTypeBoundData { #[cfg(target_arch = "x86_64")] static_assert_size!(ObligationCauseCode<'_>, 32); -#[derive(Clone, Debug, PartialEq, Eq, Hash)] +EnumTypeFoldableImpl! { + impl<'tcx> TypeFoldable<'tcx> for ObligationCauseCode<'tcx> { + (ObligationCauseCode::MiscObligation), + (ObligationCauseCode::SliceOrArrayElem), + (ObligationCauseCode::TupleElem), + (ObligationCauseCode::ProjectionWf)(a), + (ObligationCauseCode::ItemObligation)(a), + (ObligationCauseCode::BindingObligation)(a, b), + (ObligationCauseCode::ReferenceOutlivesReferent)(a), + (ObligationCauseCode::ObjectTypeBound)(a, b), + (ObligationCauseCode::ObjectCastObligation)(a), + (ObligationCauseCode::Coercion){source, target}, + (ObligationCauseCode::AssignmentLhsSized), + (ObligationCauseCode::TupleInitializerSized), + (ObligationCauseCode::StructInitializerSized), + (ObligationCauseCode::VariableType)(a), + (ObligationCauseCode::SizedArgumentType), + (ObligationCauseCode::SizedReturnType), + (ObligationCauseCode::SizedYieldType), + (ObligationCauseCode::RepeatVec), + (ObligationCauseCode::FieldSized){ adt_kind, last }, + (ObligationCauseCode::ConstSized), + (ObligationCauseCode::SharedStatic), + (ObligationCauseCode::BuiltinDerivedObligation)(a), + (ObligationCauseCode::ImplDerivedObligation)(a), + (ObligationCauseCode::CompareImplMethodObligation){ + item_name, + impl_item_def_id, + trait_item_def_id + }, + (ObligationCauseCode::ExprAssignable), + (ObligationCauseCode::MatchExpressionArm)(a), + (ObligationCauseCode::MatchExpressionArmPattern){ + span, + ty + }, + (ObligationCauseCode::IfExpression)(a), + (ObligationCauseCode::IfExpressionWithNoElse), + (ObligationCauseCode::MainFunctionType), + (ObligationCauseCode::StartFunctionType), + (ObligationCauseCode::IntrinsicType), + (ObligationCauseCode::MethodReceiver), + (ObligationCauseCode::ReturnNoExpression), + (ObligationCauseCode::ReturnValue)(a), + (ObligationCauseCode::ReturnType), + (ObligationCauseCode::BlockTailExpression)(a), + (ObligationCauseCode::TrivialBound), + } +} + +#[derive(Clone, Debug, PartialEq, Eq, Hash, HashStable, RustcEncodable, RustcDecodable)] pub struct MatchExpressionArmCause<'tcx> { pub arm_span: Span, pub source: hir::MatchSource, @@ -299,14 +362,26 @@ pub struct MatchExpressionArmCause<'tcx> { pub discrim_hir_id: hir::HirId, } -#[derive(Clone, Debug, PartialEq, Eq, Hash)] +BraceStructTypeFoldableImpl! { + impl<'tcx> TypeFoldable<'tcx> for MatchExpressionArmCause<'tcx> { + arm_span, source, prior_arms, last_ty, discrim_hir_id + } +} + +#[derive(Clone, Debug, PartialEq, Eq, Hash, HashStable, RustcEncodable, RustcDecodable)] pub struct IfExpressionCause { pub then: Span, pub outer: Option, pub semicolon: Option, } -#[derive(Clone, Debug, PartialEq, Eq, Hash)] +BraceStructTypeFoldableImpl! { + impl<'tcx> TypeFoldable<'tcx> for IfExpressionCause { + then, outer, semicolon + } +} + +#[derive(Clone, Debug, PartialEq, Eq, Hash, HashStable, RustcEncodable, RustcDecodable)] pub struct DerivedObligationCause<'tcx> { /// The trait reference of the parent obligation that led to the /// current obligation. Note that only trait obligations lead to @@ -318,6 +393,13 @@ pub struct DerivedObligationCause<'tcx> { parent_code: Rc> } +BraceStructTypeFoldableImpl! { + impl<'tcx> TypeFoldable<'tcx> for DerivedObligationCause<'tcx> { + parent_trait_ref, parent_code + } +} + + pub type Obligations<'tcx, O> = Vec>; pub type PredicateObligations<'tcx> = Vec>; pub type TraitObligations<'tcx> = Vec>; @@ -483,7 +565,7 @@ pub struct InEnvironment<'tcx, G> { pub type Selection<'tcx> = Vtable<'tcx, PredicateObligation<'tcx>>; -#[derive(Clone,Debug,TypeFoldable)] +#[derive(Clone,Debug, RustcEncodable, RustcDecodable, HashStable, TypeFoldable)] pub enum SelectionError<'tcx> { Unimplemented, OutputTypeParameterMismatch(ty::PolyTraitRef<'tcx>, @@ -494,6 +576,8 @@ pub enum SelectionError<'tcx> { Overflow, } + +#[derive(RustcEncodable, RustcDecodable, HashStable)] pub struct FulfillmentError<'tcx> { pub obligation: PredicateObligation<'tcx>, pub code: FulfillmentErrorCode<'tcx>, @@ -503,7 +587,7 @@ pub struct FulfillmentError<'tcx> { pub points_at_arg_span: bool, } -#[derive(Clone)] +#[derive(Clone, RustcEncodable, RustcDecodable, HashStable)] pub enum FulfillmentErrorCode<'tcx> { CodeSelectionError(SelectionError<'tcx>), CodeProjectionError(MismatchedProjectionTypes<'tcx>), diff --git a/src/librustc/traits/project.rs b/src/librustc/traits/project.rs index c345b9a969c90..5ceefdeaa6096 100644 --- a/src/librustc/traits/project.rs +++ b/src/librustc/traits/project.rs @@ -26,7 +26,7 @@ use crate::util::common::FN_OUTPUT_NAME; /// Depending on the stage of compilation, we want projection to be /// more or less conservative. -#[derive(Debug, Copy, Clone, PartialEq, Eq, Hash, HashStable)] +#[derive(Debug, Copy, Clone, PartialEq, Eq, Hash, HashStable, RustcEncodable, RustcDecodable)] pub enum Reveal { /// At type-checking time, we refuse to project any associated /// type that is marked `default`. Non-`default` ("final") types @@ -85,7 +85,7 @@ pub enum ProjectionTyError<'tcx> { TraitSelectionError(SelectionError<'tcx>), } -#[derive(Clone)] +#[derive(Clone, RustcEncodable, RustcDecodable, HashStable)] pub struct MismatchedProjectionTypes<'tcx> { pub err: ty::error::TypeError<'tcx> } diff --git a/src/librustc/traits/select.rs b/src/librustc/traits/select.rs index 5f324527a2725..3d7743183390d 100644 --- a/src/librustc/traits/select.rs +++ b/src/librustc/traits/select.rs @@ -2177,6 +2177,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())), } @@ -2688,7 +2697,10 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> { /// Bar where struct Bar { x: T, y: u32 } -> [i32, u32] /// Zed where enum Zed { A(T), B(u32) } -> [i32, u32] /// ``` - fn constituent_types_for_ty(&self, t: Ty<'tcx>) -> Vec> { + fn constituent_types_for_ty( + &self, + t: Ty<'tcx>, + ) -> Vec> { match t.kind { ty::Uint(_) | ty::Int(_) @@ -2744,11 +2756,69 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> { .collect() } - ty::GeneratorWitness(types) => { - // This is sound because no regions in the witness can refer to - // the binder outside the witness. So we'll effectivly reuse - // the implicit binder around the witness. - types.skip_binder().to_vec() + ty::GeneratorWitness(did, types) => { + // Note that we need to use optimized_mir here, + // in order to have the `StateTransform` pass run + let gen_mir = self.tcx().optimized_mir(did); + let gen_layout = gen_mir.generator_layout.as_ref() + .expect("Missing generator layout!"); + + // We need to compare the types from the GeneratoWitness + // to the types from the MIR. Since the generator MIR (specifically + // the StateTransform pass) runs after lifetimes are erased, we must + // erase lifetime from our witness types in order to perform the comparsion + // + // First, we erase all of late-bound regions bound by + // the witness type. + let unbound_tys = self.tcx().erase_late_bound_regions(&types); + + // However, we may still have other regions within the inner + // witness type (eg.. 'static'). Thus, we need to call + // 'erase_regions' on each of the inner witness types. + // These erased types can now be directly compared with + // types from the generator MIR + // + // The only case we need to worry about is where the witness + // contains two types that differ only in regions. Currently, + // the construction of a witness type causes all of its sub-types + // to have their regions replace with unique late-bound regions. + // Effectively, we are throwing away region information when + // we construct the generator witness. Any types that differ only + // in their late-bound regions are effectily 'the same', since + // we don't know anything about the relationships between those + // regions. + // + // In the future, we might keep track of more region information + // in the generator witness (e.g. recording `MyType<'static>`, + // or that `'a: 'b` in `OtherType<'a, 'b>`. Should this change be made, + // this code will still be sound - however, it could overapproximate + // which types are actually contained in the generator MIR, since + // types that differ only in regions become equivalent when regions are + // erased. This would have the effect of adding unecessary auto-trait + // bounds for generator types - e.g. we require `MyType: Send` for some + // type `MyType` when it would be safe to ignore `MyType`. This is + // perfectly safe, but could be relaxed to allow more valid code + // to type-check. + // + // Note that we need map erased types back to the orignal type. + // This allows to return the original type from the GeneratorWitness + // in our Vec of consiitutent types - preservering the internal regions + let erased_types: FxHashMap<_, _> = self.tcx().erase_regions(&unbound_tys) + .into_iter() + .zip(types.skip_binder().into_iter()) + .collect(); + + let mut used_types = Vec::new(); + + for ty in &gen_layout.field_tys { + if let Some(witness_ty) = erased_types.get(&self.tcx().erase_regions(ty)) { + used_types.push(**witness_ty); + } + } + + debug!("Used witness types for witness {:?} are: '{:?}'", t, used_types); + + used_types.to_vec() } // For `PhantomData`, we pass `T`. @@ -2856,7 +2926,7 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> { ))), AutoImplCandidate(trait_def_id) => { - let data = self.confirm_auto_impl_candidate(obligation, trait_def_id); + let data = self.confirm_auto_impl_candidate(obligation, trait_def_id)?; Ok(VtableAutoImpl(data)) } @@ -2996,7 +3066,7 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> { &mut self, obligation: &TraitObligation<'tcx>, trait_def_id: DefId, - ) -> VtableAutoImplData> { + ) -> Result>, SelectionError<'tcx>> { debug!( "confirm_auto_impl_candidate({:?}, {:?})", obligation, trait_def_id @@ -3006,7 +3076,8 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> { let self_ty = self.infcx.shallow_resolve(inner.self_ty()); self.constituent_types_for_ty(self_ty) }); - self.vtable_auto_impl(obligation, trait_def_id, types) + + Ok(self.vtable_auto_impl(obligation, trait_def_id, types)) } /// See `confirm_auto_impl_candidate`. diff --git a/src/librustc/ty/codec.rs b/src/librustc/ty/codec.rs index b3ef3217ec6a0..e02c6f9b2c933 100644 --- a/src/librustc/ty/codec.rs +++ b/src/librustc/ty/codec.rs @@ -271,6 +271,18 @@ where .mk_existential_predicates((0..len).map(|_| Decodable::decode(decoder)))?) } +#[inline] +pub fn decode_predicate_slice( + decoder: &mut D, +) -> Result<&'tcx ty::List>, D::Error> +where + D: TyDecoder<'tcx>, +{ + let len = decoder.read_usize()?; + Ok(decoder.tcx() + .mk_predicates((0..len).map(|_| Decodable::decode(decoder)))?) +} + #[inline] pub fn decode_canonical_var_infos(decoder: &mut D) -> Result, D::Error> where @@ -467,6 +479,14 @@ macro_rules! implement_ty_decoder { } } + impl<$($typaram),*> SpecializedDecoder<&'tcx ty::List>> + for $DecoderName<$($typaram),*> { + fn specialized_decode(&mut self) + -> Result<&'tcx ty::List>, Self::Error> { + decode_predicate_slice(self) + } + } + impl<$($typaram),*> SpecializedDecoder> for $DecoderName<$($typaram),*> { fn specialized_decode(&mut self) diff --git a/src/librustc/ty/context.rs b/src/librustc/ty/context.rs index 6a0002cd80fd0..640dd6274be3b 100644 --- a/src/librustc/ty/context.rs +++ b/src/librustc/ty/context.rs @@ -26,7 +26,7 @@ use crate::mir::{BodyCache, Field, interpret, Local, Place, PlaceElem, Projectio use crate::mir::interpret::{ConstValue, Allocation, Scalar}; use crate::ty::subst::{GenericArg, InternalSubsts, SubstsRef, Subst}; use crate::ty::ReprOptions; -use crate::traits; +use crate::traits::{self, PredicateObligation}; use crate::traits::{Clause, Clauses, GoalKind, Goal, Goals}; use crate::ty::{self, DefIdTree, Ty, TypeAndMut}; use crate::ty::{TyS, TyKind, List}; @@ -430,6 +430,8 @@ pub struct TypeckTables<'tcx> { /// Stores the type, span and optional scope span of all types /// that are live across the yield of this generator (if a generator). pub generator_interior_types: Vec>, + + pub generator_obligations: Vec> } impl<'tcx> TypeckTables<'tcx> { @@ -456,6 +458,7 @@ impl<'tcx> TypeckTables<'tcx> { concrete_opaque_types: Default::default(), upvar_list: Default::default(), generator_interior_types: Default::default(), + generator_obligations: Default::default() } } @@ -764,7 +767,7 @@ impl<'a, 'tcx> HashStable> for TypeckTables<'tcx> { ref concrete_opaque_types, ref upvar_list, ref generator_interior_types, - + ref generator_obligations } = *self; hcx.with_node_id_hashing_mode(NodeIdHashingMode::HashDefPath, |hcx| { @@ -809,6 +812,7 @@ impl<'a, 'tcx> HashStable> for TypeckTables<'tcx> { concrete_opaque_types.hash_stable(hcx, hasher); upvar_list.hash_stable(hcx, hasher); generator_interior_types.hash_stable(hcx, hasher); + generator_obligations.hash_stable(hcx, hasher); }) } } @@ -2496,8 +2500,12 @@ impl<'tcx> TyCtxt<'tcx> { } #[inline] - pub fn mk_generator_witness(self, types: ty::Binder<&'tcx List>>) -> Ty<'tcx> { - self.mk_ty(GeneratorWitness(types)) + pub fn mk_generator_witness( + self, + id: DefId, + types: ty::Binder<&'tcx List>> + ) -> Ty<'tcx> { + self.mk_ty(GeneratorWitness(id, types)) } #[inline] diff --git a/src/librustc/ty/error.rs b/src/librustc/ty/error.rs index be6d21564a0f1..b6274d2e68951 100644 --- a/src/librustc/ty/error.rs +++ b/src/librustc/ty/error.rs @@ -11,14 +11,15 @@ use syntax_pos::Span; use std::borrow::Cow; use std::fmt; -#[derive(Clone, Copy, Debug, PartialEq, Eq, TypeFoldable)] +#[derive(Clone, Copy, Debug, PartialEq, Eq, RustcEncodable, RustcDecodable, HashStable, + TypeFoldable)] pub struct ExpectedFound { pub expected: T, pub found: T, } // Data structures used in type unification -#[derive(Clone, Debug, TypeFoldable)] +#[derive(Clone, Debug, RustcEncodable, RustcDecodable, HashStable, TypeFoldable)] pub enum TypeError<'tcx> { Mismatch, UnsafetyMismatch(ExpectedFound), diff --git a/src/librustc/ty/fast_reject.rs b/src/librustc/ty/fast_reject.rs index 27a09b394b8fa..bccf3b6d88ffa 100644 --- a/src/librustc/ty/fast_reject.rs +++ b/src/librustc/ty/fast_reject.rs @@ -90,7 +90,7 @@ pub fn simplify_type( ty::Generator(def_id, _, _) => { Some(GeneratorSimplifiedType(def_id)) } - ty::GeneratorWitness(ref tys) => { + ty::GeneratorWitness(_, ref tys) => { Some(GeneratorWitnessSimplifiedType(tys.skip_binder().len())) } ty::Never => Some(NeverSimplifiedType), diff --git a/src/librustc/ty/flags.rs b/src/librustc/ty/flags.rs index aee0ec7806b76..8acefb47b8fb2 100644 --- a/src/librustc/ty/flags.rs +++ b/src/librustc/ty/flags.rs @@ -96,7 +96,7 @@ impl FlagComputation { self.add_substs(substs); } - &ty::GeneratorWitness(ref ts) => { + &ty::GeneratorWitness(_, ref ts) => { let mut computation = FlagComputation::new(); computation.add_tys(&ts.skip_binder()[..]); self.add_bound_computation(&computation); diff --git a/src/librustc/ty/mod.rs b/src/librustc/ty/mod.rs index 05a2704cc5dfc..6b01d0627fd2b 100644 --- a/src/librustc/ty/mod.rs +++ b/src/librustc/ty/mod.rs @@ -810,7 +810,7 @@ pub struct ClosureUpvar<'tcx> { pub ty: Ty<'tcx>, } -#[derive(Clone, Copy, PartialEq, Eq)] +#[derive(Clone, Copy, PartialEq, Eq, RustcEncodable, RustcDecodable, HashStable)] pub enum IntVarValue { IntType(ast::IntTy), UintType(ast::UintTy), @@ -1632,7 +1632,8 @@ pub type PlaceholderConst = Placeholder; /// When type checking, we use the `ParamEnv` to track /// details about the set of where-clauses that are in scope at this /// particular point. -#[derive(Copy, Clone, Debug, PartialEq, Eq, Hash, HashStable, TypeFoldable)] +#[derive(Copy, Clone, Debug, PartialEq, Eq, Hash, HashStable, RustcEncodable, RustcDecodable, + TypeFoldable)] pub struct ParamEnv<'tcx> { /// `Obligation`s that the caller must satisfy. This is basically /// the set of bounds on the in-scope type parameters, translated @@ -2006,7 +2007,7 @@ impl<'a> HashStable> for AdtDef { } } -#[derive(Copy, Clone, Debug, Eq, PartialEq, Hash)] +#[derive(Copy, Clone, Debug, Eq, PartialEq, Hash, HashStable, RustcEncodable, RustcDecodable)] pub enum AdtKind { Struct, Union, Enum } impl Into for AdtKind { diff --git a/src/librustc/ty/print/obsolete.rs b/src/librustc/ty/print/obsolete.rs index 7eb774849b178..2cde94272fa7a 100644 --- a/src/librustc/ty/print/obsolete.rs +++ b/src/librustc/ty/print/obsolete.rs @@ -151,7 +151,7 @@ impl DefPathBasedNames<'tcx> { | ty::UnnormalizedProjection(..) | ty::Projection(..) | ty::Param(_) - | ty::GeneratorWitness(_) + | ty::GeneratorWitness(..) | ty::Opaque(..) => { if debug { output.push_str(&format!("`{:?}`", t)); diff --git a/src/librustc/ty/print/pretty.rs b/src/librustc/ty/print/pretty.rs index 745f7d0276d80..f33aacdace8b4 100644 --- a/src/librustc/ty/print/pretty.rs +++ b/src/librustc/ty/print/pretty.rs @@ -672,8 +672,10 @@ pub trait PrettyPrinter<'tcx>: p!(write(" "), print(witness), write("]")) }, - ty::GeneratorWitness(types) => { + ty::GeneratorWitness(did, types) => { + p!(write("[witness@{:?}", did)); p!(in_binder(&types)); + p!(write("]")); } ty::Closure(did, substs) => { let upvar_tys = substs.as_closure().upvar_tys(did, self.tcx()); diff --git a/src/librustc/ty/relate.rs b/src/librustc/ty/relate.rs index efdf40fd6b8a4..9c3d854374d7d 100644 --- a/src/librustc/ty/relate.rs +++ b/src/librustc/ty/relate.rs @@ -417,7 +417,8 @@ pub fn super_relate_tys>( Ok(tcx.mk_generator(a_id, substs, movability)) } - (&ty::GeneratorWitness(a_types), &ty::GeneratorWitness(b_types)) => + (&ty::GeneratorWitness(a_id, a_types), &ty::GeneratorWitness(b_id, b_types)) + if a_id == b_id => { // Wrap our types with a temporary GeneratorWitness struct // inside the binder so we can related them @@ -425,7 +426,7 @@ pub fn super_relate_tys>( let b_types = b_types.map_bound(GeneratorWitness); // Then remove the GeneratorWitness for the result let types = relation.relate(&a_types, &b_types)?.map_bound(|witness| witness.0); - Ok(tcx.mk_generator_witness(types)) + Ok(tcx.mk_generator_witness(a_id, types)) } (&ty::Closure(a_id, a_substs), diff --git a/src/librustc/ty/structural_impls.rs b/src/librustc/ty/structural_impls.rs index ce76a4c831b58..873524ff8f6bf 100644 --- a/src/librustc/ty/structural_impls.rs +++ b/src/librustc/ty/structural_impls.rs @@ -1015,7 +1015,7 @@ impl<'tcx> TypeFoldable<'tcx> for Ty<'tcx> { substs.fold_with(folder), movability) } - ty::GeneratorWitness(types) => ty::GeneratorWitness(types.fold_with(folder)), + ty::GeneratorWitness(did, types) => ty::GeneratorWitness(did, types.fold_with(folder)), ty::Closure(did, substs) => ty::Closure(did, substs.fold_with(folder)), ty::Projection(ref data) => ty::Projection(data.fold_with(folder)), ty::UnnormalizedProjection(ref data) => { @@ -1064,7 +1064,7 @@ impl<'tcx> TypeFoldable<'tcx> for Ty<'tcx> { ty::Generator(_did, ref substs, _) => { substs.visit_with(visitor) } - ty::GeneratorWitness(ref types) => types.visit_with(visitor), + ty::GeneratorWitness(_, ref types) => types.visit_with(visitor), ty::Closure(_did, ref substs) => substs.visit_with(visitor), ty::Projection(ref data) | ty::UnnormalizedProjection(ref data) => { data.visit_with(visitor) diff --git a/src/librustc/ty/sty.rs b/src/librustc/ty/sty.rs index 6cb0d1e9946b5..aa5c7fcc22dfb 100644 --- a/src/librustc/ty/sty.rs +++ b/src/librustc/ty/sty.rs @@ -170,7 +170,9 @@ pub enum TyKind<'tcx> { /// A type representin the types stored inside a generator. /// This should only appear in GeneratorInteriors. - GeneratorWitness(Binder<&'tcx List>>), + /// The `DefId` refers to the generator corresponding + /// to this `GeneratorWitness + GeneratorWitness(DefId, Binder<&'tcx List>>), /// The never type `!` Never, @@ -643,6 +645,7 @@ impl<'tcx> Binder> { } } +impl<'tcx> rustc_serialize::UseSpecializedDecodable for &'tcx List> {} impl<'tcx> rustc_serialize::UseSpecializedDecodable for &'tcx List> {} impl<'tcx> List> { @@ -897,7 +900,8 @@ impl Binder { pub fn dummy<'tcx>(value: T) -> Binder where T: TypeFoldable<'tcx> { - debug_assert!(!value.has_escaping_bound_vars()); + debug_assert!(!value.has_escaping_bound_vars(), + "Value has escaping vars: {:?}", value); Binder(value) } diff --git a/src/librustc/ty/walk.rs b/src/librustc/ty/walk.rs index 8d0f9a4716247..de89e03dda508 100644 --- a/src/librustc/ty/walk.rs +++ b/src/librustc/ty/walk.rs @@ -113,7 +113,7 @@ fn push_subtypes<'tcx>(stack: &mut TypeWalkerStack<'tcx>, parent_ty: Ty<'tcx>) { | ty::Generator(_, ref substs, _) => { stack.extend(substs.types().rev()); } - ty::GeneratorWitness(ts) => { + ty::GeneratorWitness(_, ts) => { stack.extend(ts.skip_binder().iter().cloned().rev()); } ty::Tuple(..) => { diff --git a/src/librustc_codegen_utils/symbol_names/v0.rs b/src/librustc_codegen_utils/symbol_names/v0.rs index 1dfcc21f3903d..5ad79096658bd 100644 --- a/src/librustc_codegen_utils/symbol_names/v0.rs +++ b/src/librustc_codegen_utils/symbol_names/v0.rs @@ -460,7 +460,7 @@ impl Printer<'tcx> for SymbolMangler<'tcx> { self = r.print(self)?; } - ty::GeneratorWitness(_) => { + ty::GeneratorWitness(..) => { bug!("symbol_names: unexpected `GeneratorWitness`") } } diff --git a/src/librustc_mir/borrow_check/nll/type_check/mod.rs b/src/librustc_mir/borrow_check/nll/type_check/mod.rs index de304202a0840..c9b4a2687c50d 100644 --- a/src/librustc_mir/borrow_check/nll/type_check/mod.rs +++ b/src/librustc_mir/borrow_check/nll/type_check/mod.rs @@ -17,12 +17,13 @@ use rustc::mir::tcx::PlaceTy; use rustc::mir::visit::{NonMutatingUseContext, PlaceContext, Visitor}; use rustc::traits::{self, ObligationCause, PredicateObligations}; use rustc::traits::query::{Fallible, NoSolution}; +use rustc::traits::{TraitEngine, TraitEngineExt}; use rustc::traits::query::type_op; use rustc::traits::query::type_op::custom::CustomTypeOp; use rustc::ty::{ - self, CanonicalUserTypeAnnotation, CanonicalUserTypeAnnotations, RegionVid, ToPolyTraitRef, Ty, - TyCtxt, UserType, - UserTypeAnnotationIndex, + self, RegionVid, ToPolyTraitRef, Ty, TyCtxt, UserType, + CanonicalUserTypeAnnotation, CanonicalUserTypeAnnotations, + UserTypeAnnotationIndex }; use rustc::ty::adjustment::PointerCast; use rustc::ty::cast::CastTy; @@ -92,8 +93,16 @@ mod relate_tys; /// are live at all points where that local variable may later be /// used. /// -/// This phase of type-check ought to be infallible -- this is because -/// the original, HIR-based type-check succeeded. So if any errors +/// This phase of type-check is usually infallible -- this is because +/// the original, HIR-based type-check succeeded. +/// +/// The only exception to this is generators. We rely on analyis of the generated +/// MIR to determine which types are live across a `yield` point. This means that +/// we need to partially delay type-checking of generators until MIR is generated - +/// that is, after the HIR-based type-check has run. As a result, we may need to report +/// type-check errors from this phase. +/// +/// So if any errors unrelated to generators /// occur here, we will get a `bug!` reported. /// /// # Parameters @@ -159,6 +168,66 @@ pub(crate) fn type_check<'tcx>( constraints: &mut constraints, }; + // When processing the 'base' function for a given + // TypeckTable, resolve any delayed obligations for + // it. + let closure_base = infcx.tcx.closure_base_def_id(mir_def_id); + debug!("considering delayed generator for {:?}: {:?}", mir_def_id, closure_base); + + let has_errors = infcx.tcx.sess.has_errors(); + + if closure_base == mir_def_id { + debug!("evaluating delayed generator obligations for {:?}", + mir_def_id); + + // Enter a fresh InferCtxt, so that we can resolve + // the region obligations generated + // by selecting these predicates + infcx.tcx.infer_ctxt().enter(|infcx| { + + let tables = infcx.tcx.typeck_tables_of(mir_def_id); + + // We use a normal TraitEngine here - this is where + // we actually try to resolve generator obligations + let mut fulfill = TraitEngine::new(infcx.tcx); + + debug!("registering delayed bounds for {:?}: {:?}", + mir_def_id, tables.generator_obligations); + fulfill.register_predicate_obligations( + &infcx, tables.generator_obligations.clone() + ); + if let Err(errs) = fulfill.select_all_or_error(&infcx) { + debug!("reporting MIR typeck errors: {:?}", errs); + infcx.report_fulfillment_errors(&errs, None, false); + infcx.tcx.sess.abort_if_errors() + } else { + infcx.resolve_regions_and_report_errors( + mir_def_id, + &Default::default(), + &rustc::infer::outlives::env::OutlivesEnvironment::new(ty::ParamEnv::empty()), + Default::default() + ); + } + }); + } + + // Attempting to fulfill generator predicates can cause + // us to run `tcx.optimized_mir` for other generators, + // which may result in errors being registered. + // To avoid weird ICEs from being reported, we abort + // early if any errors were triggered during the + // processing of delayed generator obligations. + // Note that errors can be triggered even if we + // successfully select the generator predicates. + // + // We don't want to undcondtionally abort, as + // this would prevent us from displaying additional + // errors in the case where errors were triggered + // before we started doing type checking. + if !has_errors && infcx.tcx.sess.has_errors() { + infcx.tcx.sess.abort_if_errors() + } + type_check_internal( infcx, mir_def_id, diff --git a/src/librustc_mir/interpret/intrinsics/type_name.rs b/src/librustc_mir/interpret/intrinsics/type_name.rs index f1f9fac08ca3a..d717b082246b0 100644 --- a/src/librustc_mir/interpret/intrinsics/type_name.rs +++ b/src/librustc_mir/interpret/intrinsics/type_name.rs @@ -71,7 +71,7 @@ impl<'tcx> Printer<'tcx> for AbsolutePathPrinter<'tcx> { | ty::Generator(def_id, substs, _) => self.print_def_path(def_id, substs), ty::Foreign(def_id) => self.print_def_path(def_id, &[]), - ty::GeneratorWitness(_) => { + ty::GeneratorWitness(..) => { bug!("type_name: unexpected `GeneratorWitness`") } } diff --git a/src/librustc_mir/transform/generator.rs b/src/librustc_mir/transform/generator.rs index e55737e8859ef..eca4864d63052 100644 --- a/src/librustc_mir/transform/generator.rs +++ b/src/librustc_mir/transform/generator.rs @@ -766,7 +766,7 @@ fn compute_layout<'tcx>( // MIR types let allowed_upvars = tcx.erase_regions(upvars); let allowed = match interior.kind { - ty::GeneratorWitness(s) => tcx.erase_late_bound_regions(&s), + ty::GeneratorWitness(_, s) => tcx.erase_late_bound_regions(&s), _ => bug!(), }; @@ -1198,6 +1198,8 @@ impl<'tcx> MirPass<'tcx> for StateTransform { _ => bug!(), }; + debug!("MIR generator type: {:?}", gen_ty); + // Compute GeneratorState let state_did = tcx.lang_items().gen_state().unwrap(); let state_adt_ref = tcx.adt_def(state_did); diff --git a/src/librustc_typeck/check/generator_interior.rs b/src/librustc_typeck/check/generator_interior.rs index fcf6b22f74f3c..b34a87a3331ad 100644 --- a/src/librustc_typeck/check/generator_interior.rs +++ b/src/librustc_typeck/check/generator_interior.rs @@ -101,6 +101,7 @@ impl<'a, 'tcx> InteriorVisitor<'a, 'tcx> { pub fn resolve_interior<'a, 'tcx>( fcx: &'a FnCtxt<'a, 'tcx>, def_id: DefId, + gen_def_id: DefId, body_id: hir::BodyId, interior: Ty<'tcx>, kind: hir::GeneratorKind, @@ -152,7 +153,7 @@ pub fn resolve_interior<'a, 'tcx>( // Extract type components let type_list = fcx.tcx.mk_type_list(types.into_iter().map(|t| (t.0).ty)); - let witness = fcx.tcx.mk_generator_witness(ty::Binder::bind(type_list)); + let witness = fcx.tcx.mk_generator_witness(gen_def_id, ty::Binder::bind(type_list)); debug!("types in generator after region replacement {:?}, span = {:?}", witness, body.value.span); diff --git a/src/librustc_typeck/check/mod.rs b/src/librustc_typeck/check/mod.rs index a956aba4f62b9..143fe22f54d84 100644 --- a/src/librustc_typeck/check/mod.rs +++ b/src/librustc_typeck/check/mod.rs @@ -109,8 +109,9 @@ use rustc::mir::interpret::{ConstValue, GlobalId}; use rustc::traits::{self, ObligationCause, ObligationCauseCode, TraitEngine}; use rustc::ty::{ self, AdtKind, CanonicalUserType, Ty, TyCtxt, Const, GenericParamDefKind, - ToPolyTraitRef, ToPredicate, RegionKind, UserType + ToPolyTraitRef, ToPredicate, RegionKind, UserType, Predicate }; +use rustc::traits::PredicateObligation; use rustc::ty::adjustment::{ Adjust, Adjustment, AllowTwoPhase, AutoBorrow, AutoBorrowMutability, PointerCast }; @@ -157,6 +158,7 @@ pub use self::compare_method::{compare_impl_method, compare_const_impl}; use self::method::{MethodCallee, SelfSource}; use self::TupleArgumentsFlag::*; + /// The type of a local binding, including the revealed type for anon types. #[derive(Copy, Clone, Debug)] pub struct LocalTy<'tcx> { @@ -223,7 +225,7 @@ pub struct Inherited<'a, 'tcx> { deferred_cast_checks: RefCell>>, - deferred_generator_interiors: RefCell, hir::GeneratorKind)>>, + deferred_generator_interiors: RefCell, hir::GeneratorKind)>>, // Opaque types found in explicit return types and their // associated fresh inference variable. Writeback resolves these @@ -247,6 +249,20 @@ pub struct Inherited<'a, 'tcx> { implicit_region_bound: Option>, body_id: Option, + delayed_generators: bool +} + +impl<'a, 'tcx> Drop for Inherited<'a, 'tcx> { + fn drop(&mut self) { + if self.delayed_generators && !self.infcx.is_tainted_by_errors() { + // By the time Inherited is dropped, we should have already called + // .delayed_generator_obligations() + if let Some(generators) = self.fulfillment_cx.borrow_mut() + .delayed_generator_obligations() { + panic!("Failed to consume generator obligations: {:?}", generators); + } + } + } } impl<'a, 'tcx> Deref for Inherited<'a, 'tcx> { @@ -671,15 +687,30 @@ impl Inherited<'_, 'tcx> { impl<'tcx> InheritedBuilder<'tcx> { fn enter(&mut self, f: F) -> R where - F: for<'a> FnOnce(Inherited<'a, 'tcx>) -> R, + F: for<'a> FnOnce(&mut Inherited<'a, 'tcx>) -> R, { let def_id = self.def_id; - self.infcx.enter(|infcx| f(Inherited::new(infcx, def_id))) + self.infcx.enter(|infcx| f(&mut Inherited::new(infcx, def_id))) + } + + fn enter_delayed_generator(&mut self, f: F) -> R + where + F: for<'a> FnOnce(&mut Inherited<'a, 'tcx>) -> R, + { + let def_id = self.def_id; + self.infcx.enter(|infcx| { + let mut inh = Inherited::new_delayed_generators(infcx, def_id); + f(&mut inh) + }) } } impl Inherited<'a, 'tcx> { - fn new(infcx: InferCtxt<'a, 'tcx>, def_id: DefId) -> Self { + fn new_internal( + infcx: InferCtxt<'a, 'tcx>, + def_id: DefId, + delayed_generators: bool + ) -> Self { let tcx = infcx.tcx; let item_id = tcx.hir().as_local_hir_id(def_id); let body_id = item_id.and_then(|id| tcx.hir().maybe_body_owned_by(id)); @@ -691,12 +722,18 @@ impl Inherited<'a, 'tcx> { })) }); + let engine = if delayed_generators { + TraitEngine::with_delayed_generator_witness(tcx) + } else { + TraitEngine::new(tcx) + }; + Inherited { tables: MaybeInProgressTables { maybe_tables: infcx.in_progress_tables, }, infcx, - fulfillment_cx: RefCell::new(TraitEngine::new(tcx)), + fulfillment_cx: RefCell::new(engine), locals: RefCell::new(Default::default()), deferred_sized_obligations: RefCell::new(Vec::new()), deferred_call_resolutions: RefCell::new(Default::default()), @@ -706,9 +743,19 @@ impl Inherited<'a, 'tcx> { opaque_types_vars: RefCell::new(Default::default()), implicit_region_bound, body_id, + delayed_generators } } + fn new(infcx: InferCtxt<'a, 'tcx>, def_id: DefId) -> Self { + Self::new_internal(infcx, def_id, false) + } + + fn new_delayed_generators(infcx: InferCtxt<'a, 'tcx>, def_id: DefId) -> Self { + Self::new_internal(infcx, def_id, true) + } + + fn register_predicate(&self, obligation: traits::PredicateObligation<'tcx>) { debug!("register_predicate({:?})", obligation); if obligation.has_escaping_bound_vars() { @@ -995,7 +1042,7 @@ fn typeck_tables_of(tcx: TyCtxt<'_>, def_id: DefId) -> &ty::TypeckTables<'_> { }); let body = tcx.hir().body(body_id); - let tables = Inherited::build(tcx, def_id).enter(|inh| { + let tables = Inherited::build(tcx, def_id).enter_delayed_generator(|inh| { let param_env = tcx.param_env(def_id); let fcx = if let (Some(header), Some(decl)) = (fn_header, fn_decl) { let fn_sig = if crate::collect::get_infer_ret_ty(&decl.output).is_some() { @@ -1119,6 +1166,8 @@ fn typeck_tables_of(tcx: TyCtxt<'_>, def_id: DefId) -> &ty::TypeckTables<'_> { fcx.resolve_type_vars_in_body(body) }); + debug!("table generator obligations: {:?}", tables.generator_obligations); + // Consistency check our TypeckTables instance can hold all ItemLocalIds // it will need to hold. assert_eq!(tables.local_id_root, Some(DefId::local(id.owner))); @@ -1350,11 +1399,14 @@ fn check_fn<'a, 'tcx>( // This ensures that all nested generators appear before the entry of this generator. // resolve_generator_interiors relies on this property. let gen_ty = if let (Some(_), Some(gen_kind)) = (can_be_generator, body.generator_kind) { + let gen_def_id = fcx.tcx.hir().local_def_id(fn_id); let interior = fcx.next_ty_var(TypeVariableOrigin { kind: TypeVariableOriginKind::MiscVariable, span, }); - fcx.deferred_generator_interiors.borrow_mut().push((body.id(), interior, gen_kind)); + fcx.deferred_generator_interiors.borrow_mut().push( + (gen_def_id, body.id(), interior, gen_kind) + ); Some(GeneratorTypes { yield_ty: fcx.yield_ty.unwrap(), interior, @@ -3233,9 +3285,9 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { fn resolve_generator_interiors(&self, def_id: DefId) { let mut generators = self.deferred_generator_interiors.borrow_mut(); - for (body_id, interior, kind) in generators.drain(..) { + for (gen_def_id, body_id, interior, kind) in generators.drain(..) { self.select_obligations_where_possible(false, |_| {}); - generator_interior::resolve_interior(self, def_id, body_id, interior, kind); + generator_interior::resolve_interior(self, def_id, gen_def_id, body_id, interior, kind); } } @@ -5428,6 +5480,99 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { contained_in_place } + + /// Processes the delayed generator witness obligations, + /// sanitizing them for inclusion into a `TypeckTables`. + /// + /// When we try to resolve a predicate of the form + /// `: AutoTrait`, we inspect the generator + /// MIR to determine which inner types are actually stored in the generator + /// (e.g. which types must implement `AutoTrait`). This allows us to + /// avoid creating spurious auto-trait bounds, making using + /// generators much more ergonomic for the user. + /// + /// However, when we actually generate these auto-trait predicates + /// (during `typeck_tables_of`), we are not in a position to actually + /// resolve them, since the MIR will not be generated until much + /// later in compilation. As a result, we delay the resolution + /// until we run MIR type-checking for the containing function + /// (i.e. the function in which the generator is defined). + /// + /// This involves 'transplating' a `PredicateObligation` from + /// one `InferCtxt` (the one used by `FnCtxt`) to a new `InferCtxt` + /// (the fresh one we create when we are ready to resolve the obligation). + /// Fortunately, the actual `TraitPredicate` can be freely + /// moved between different `InferCtxt`, as it contains no inference + /// variables. This is due to the fact that a `GeneratorWitness`. + /// is constructed such that it contains only late-bound regions + /// (bound by the `Binder` in the `GeneratorWitness). Auto traits + /// cannot have any generic parameters whatsoever, so this guarnatees + /// that the `TraitPredicate` will have no inference variables. As a result, + /// it is independent of the `InferCtxt` in which is was constructed, + /// and can be re-processed in a different `InferCtxt` without causing any issues. + /// + /// However, we still need to worry about the `ObligationCause` stored by the + /// `PredicateObligation`. An `ObligationCause` can (and usually does) contain + /// region inference variables. Fortunately, `ObligationCause`s are only used + /// when generating error messages. This means that we can safely erase all + /// region variables from the `PredicateObligation`, without affecting + /// anything other than the printing of error messages. + fn process_generators(&self) -> Vec> { + let mut obligations = self + .fulfillment_cx + .borrow_mut() + .delayed_generator_obligations() + .unwrap_or_default(); + + obligations = self.resolve_vars_if_possible(&obligations); + + let mut delayed_obligations = Vec::new(); + + debug!("process_generators: {:?}", obligations); + + for obligation in obligations { + let p = match obligation.predicate { + Predicate::Trait(p) => p, + _ => panic!("Unexpected obligation {:?}", obligation) + }; + + let self_ty = p.skip_binder().self_ty(); + match self_ty.kind { + ty::GeneratorWitness(..) => {}, + _ => panic!("Unexpected self type in obligation {:?}", obligation) + }; + + let trait_did = p.def_id(); + if !self.tcx.trait_is_auto(trait_did) { + panic!("Predicate does not involve an auto trait: {:?}", obligation); + } + + let mut erased_obligation = self.resolve_vars_if_possible(&obligation); + + // Erase regions from cause - it is only used to display error messages + // if we end up failing to fulfill this obligation, so it's fine to + // lose region information + erased_obligation.cause = self.tcx.erase_regions( + &self.resolve_vars_if_possible(&&erased_obligation.cause) + ); + + if erased_obligation.needs_infer() { + panic!("Failed to resolve inference variables in obligation: {:?}", + erased_obligation); + } + + if erased_obligation.cause.needs_infer() { + panic!("Failed to resolve inference variables in cause: {:?}", + erased_obligation.cause); + } + + debug!("process_generators_inner: delaying auto-trait obligation {:?} \ + (original = {:?})", erased_obligation, obligation); + delayed_obligations.push(erased_obligation); + + } + delayed_obligations + } } pub fn check_bounds_are_used<'tcx>(tcx: TyCtxt<'tcx>, generics: &ty::Generics, ty: Ty<'tcx>) { diff --git a/src/librustc_typeck/check/writeback.rs b/src/librustc_typeck/check/writeback.rs index 3113e9b241daa..47dc8b6505a26 100644 --- a/src/librustc_typeck/check/writeback.rs +++ b/src/librustc_typeck/check/writeback.rs @@ -30,8 +30,13 @@ use std::mem; // so instead all of the replacement happens at the end in // resolve_type_vars_in_body, which creates a new TypeTables which // doesn't contain any inference types. + + impl<'a, 'tcx> FnCtxt<'a, 'tcx> { - pub fn resolve_type_vars_in_body(&self, body: &'tcx hir::Body) -> &'tcx ty::TypeckTables<'tcx> { + pub fn resolve_type_vars_in_body( + &self, + body: &'tcx hir::Body, + ) -> &'tcx ty::TypeckTables<'tcx> { let item_id = self.tcx.hir().body_owner(body.id()); let item_def_id = self.tcx.hir().local_def_id(item_id); @@ -78,6 +83,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { ); wbcx.tables.tainted_by_errors = self.is_tainted_by_errors(); + wbcx.tables.generator_obligations = self.process_generators(); debug!( "writeback: tables for {:?} are {:#?}", diff --git a/src/test/ui/async-await/async-fn-nonsend.rs b/src/test/ui/async-await/async-fn-nonsend.rs index 1f1bf4250eadf..ee13543f4492a 100644 --- a/src/test/ui/async-await/async-fn-nonsend.rs +++ b/src/test/ui/async-await/async-fn-nonsend.rs @@ -37,7 +37,6 @@ async fn non_send_temporary_in_match() { } async fn non_sync_with_method_call() { - // FIXME: it'd be nice for this to work. let f: &mut std::fmt::Formatter = panic!(); if non_sync().fmt(f).unwrap() == () { fut().await; @@ -48,10 +47,9 @@ fn assert_send(_: impl Send) {} pub fn pass_assert() { assert_send(local_dropped_before_await()); - //~^ ERROR `std::rc::Rc<()>` cannot be sent between threads safely assert_send(non_send_temporary_in_match()); //~^ ERROR `std::rc::Rc<()>` cannot be sent between threads safely assert_send(non_sync_with_method_call()); - //~^ ERROR `dyn std::fmt::Write` cannot be sent between threads safely - //~^^ ERROR `*mut (dyn std::ops::Fn() + 'static)` cannot be shared between threads safely } + +fn main() {} diff --git a/src/test/ui/generator/auto-trait-regions.rs b/src/test/ui/generator/auto-trait-regions.rs index 46d7289943857..877798659023b 100644 --- a/src/test/ui/generator/auto-trait-regions.rs +++ b/src/test/ui/generator/auto-trait-regions.rs @@ -40,8 +40,10 @@ fn main() { assert_foo(gen); // ok // Disallow impls which relates lifetimes in the generator interior - let gen = || { - let a = A(&mut true, &mut true, No); + let gen = static || { + let first = &mut true; + let second = &mut true; + let a = A(first, second, No); yield; assert_foo(a); }; diff --git a/src/test/ui/generator/auto-trait-regions.stderr b/src/test/ui/generator/auto-trait-regions.stderr index dab4d348ceb60..4e3f5da63faa8 100644 --- a/src/test/ui/generator/auto-trait-regions.stderr +++ b/src/test/ui/generator/auto-trait-regions.stderr @@ -11,7 +11,7 @@ LL | assert_foo(gen); = note: ...but `Foo` is actually implemented for the type `&'1 OnlyFooIfStaticRef`, for some specific lifetime `'1` error: implementation of `Foo` is not general enough - --> $DIR/auto-trait-regions.rs:48:5 + --> $DIR/auto-trait-regions.rs:50:5 | LL | auto trait Foo {} | ----------------- trait `Foo` defined here diff --git a/src/test/ui/generator/not-send-sync.rs b/src/test/ui/generator/not-send-sync.rs index ae0a288bbb496..d647f62c6bd28 100644 --- a/src/test/ui/generator/not-send-sync.rs +++ b/src/test/ui/generator/not-send-sync.rs @@ -7,7 +7,6 @@ fn main() { fn assert_send(_: T) {} assert_sync(|| { - //~^ ERROR: E0277 let a = Cell::new(2); yield; }); diff --git a/src/test/ui/generator/not-send-sync.stderr b/src/test/ui/generator/not-send-sync.stderr index 620db245d3e57..ced07f6766180 100644 --- a/src/test/ui/generator/not-send-sync.stderr +++ b/src/test/ui/generator/not-send-sync.stderr @@ -1,5 +1,5 @@ error[E0277]: `std::cell::Cell` cannot be shared between threads safely - --> $DIR/not-send-sync.rs:16:5 + --> $DIR/not-send-sync.rs:15:5 | LL | fn assert_send(_: T) {} | ----------- ---- required by this bound in `main::assert_send` @@ -9,21 +9,8 @@ LL | assert_send(|| { | = help: the trait `std::marker::Sync` is not implemented for `std::cell::Cell` = note: required because of the requirements on the impl of `std::marker::Send` for `&std::cell::Cell` - = note: required because it appears within the type `[generator@$DIR/not-send-sync.rs:16:17: 20:6 a:&std::cell::Cell _]` + = note: required because it appears within the type `[generator@$DIR/not-send-sync.rs:15:17: 19:6 a:&std::cell::Cell _]` -error[E0277]: `std::cell::Cell` cannot be shared between threads safely - --> $DIR/not-send-sync.rs:9:5 - | -LL | fn assert_sync(_: T) {} - | ----------- ---- required by this bound in `main::assert_sync` -... -LL | assert_sync(|| { - | ^^^^^^^^^^^ `std::cell::Cell` cannot be shared between threads safely - | - = help: within `[generator@$DIR/not-send-sync.rs:9:17: 13:6 {std::cell::Cell, ()}]`, the trait `std::marker::Sync` is not implemented for `std::cell::Cell` - = note: required because it appears within the type `{std::cell::Cell, ()}` - = note: required because it appears within the type `[generator@$DIR/not-send-sync.rs:9:17: 13:6 {std::cell::Cell, ()}]` - -error: aborting due to 2 previous errors +error: aborting due to previous error For more information about this error, try `rustc --explain E0277`. From 78159e14200755dc12ed4b9222b23d72a311990a Mon Sep 17 00:00:00 2001 From: Aaron Hill Date: Thu, 24 Oct 2019 20:18:10 -0400 Subject: [PATCH 02/15] Use an Arc in DerivedObligationCauseCode --- src/librustc/traits/mod.rs | 4 ++-- src/librustc/traits/select.rs | 4 ++-- src/librustc/traits/structural_impls.rs | 4 ++-- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/librustc/traits/mod.rs b/src/librustc/traits/mod.rs index 5717b518b0a8e..137a432769bd0 100644 --- a/src/librustc/traits/mod.rs +++ b/src/librustc/traits/mod.rs @@ -36,7 +36,7 @@ use crate::ty::fold::{TypeFolder, TypeFoldable, TypeVisitor}; use crate::util::common::ErrorReported; use std::fmt::Debug; -use std::rc::Rc; +use std::sync::Arc; pub use self::SelectionError::*; pub use self::FulfillmentErrorCode::*; @@ -390,7 +390,7 @@ pub struct DerivedObligationCause<'tcx> { parent_trait_ref: ty::PolyTraitRef<'tcx>, /// The parent trait had this cause. - parent_code: Rc> + parent_code: Arc> } BraceStructTypeFoldableImpl! { diff --git a/src/librustc/traits/select.rs b/src/librustc/traits/select.rs index 3d7743183390d..226ca20a7f631 100644 --- a/src/librustc/traits/select.rs +++ b/src/librustc/traits/select.rs @@ -49,7 +49,7 @@ use std::cell::{Cell, RefCell}; use std::cmp; use std::fmt::{self, Display}; use std::iter; -use std::rc::Rc; +use std::sync::Arc; use crate::util::nodemap::{FxHashMap, FxHashSet}; pub struct SelectionContext<'cx, 'tcx> { @@ -4078,7 +4078,7 @@ impl<'tcx> TraitObligation<'tcx> { if obligation.recursion_depth >= 0 { let derived_cause = DerivedObligationCause { parent_trait_ref: obligation.predicate.to_poly_trait_ref(), - parent_code: Rc::new(obligation.cause.code.clone()), + parent_code: Arc::new(obligation.cause.code.clone()), }; let derived_code = variant(derived_cause); ObligationCause::new( diff --git a/src/librustc/traits/structural_impls.rs b/src/librustc/traits/structural_impls.rs index 8c300da11fcbf..34bbe775e024b 100644 --- a/src/librustc/traits/structural_impls.rs +++ b/src/librustc/traits/structural_impls.rs @@ -7,7 +7,7 @@ use crate::ty::{self, Lift, Ty, TyCtxt}; use syntax::symbol::Symbol; use std::fmt; -use std::rc::Rc; +use std::sync::Arc; use std::collections::{BTreeSet, BTreeMap}; // Structural impls for the structs in `traits`. @@ -561,7 +561,7 @@ impl<'a, 'tcx> Lift<'tcx> for traits::DerivedObligationCause<'a> { tcx.lift(&*self.parent_code) .map(|code| traits::DerivedObligationCause { parent_trait_ref: trait_ref, - parent_code: Rc::new(code), + parent_code: Arc::new(code), }) ) } From 46e2d2c4b8e0f0465b5354f58b29a99e017d4c0d Mon Sep 17 00:00:00 2001 From: Aaron Hill Date: Fri, 25 Oct 2019 18:04:29 -0400 Subject: [PATCH 03/15] Make generators !Freeze --- src/librustc/traits/select.rs | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/src/librustc/traits/select.rs b/src/librustc/traits/select.rs index 226ca20a7f631..94f62f2cec682 100644 --- a/src/librustc/traits/select.rs +++ b/src/librustc/traits/select.rs @@ -2180,11 +2180,7 @@ 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 }); + // For now, always consider generators to be !Freeze } _ => candidates.vec.push(AutoImplCandidate(def_id.clone())), From 5ce43067f5886e4c63ca355799332a9df45fa3c1 Mon Sep 17 00:00:00 2001 From: Aaron Hill Date: Mon, 18 Nov 2019 16:21:44 -0500 Subject: [PATCH 04/15] Fix rebase fallout --- src/librustc/traits/mod.rs | 93 +++++--------------------------------- 1 file changed, 12 insertions(+), 81 deletions(-) diff --git a/src/librustc/traits/mod.rs b/src/librustc/traits/mod.rs index 137a432769bd0..7e3ac63e5b414 100644 --- a/src/librustc/traits/mod.rs +++ b/src/librustc/traits/mod.rs @@ -135,7 +135,8 @@ pub type TraitObligation<'tcx> = Obligation<'tcx, ty::PolyTraitPredicate<'tcx>>; static_assert_size!(PredicateObligation<'_>, 112); /// The reason why we incurred this obligation; used for error reporting. -#[derive(Clone, Debug, PartialEq, Eq, Hash, HashStable, RustcEncodable, RustcDecodable)] +#[derive(Clone, Debug, PartialEq, Eq, Hash, HashStable, RustcEncodable, RustcDecodable, + TypeFoldable)] pub struct ObligationCause<'tcx> { pub span: Span, @@ -165,13 +166,8 @@ impl<'tcx> ObligationCause<'tcx> { } } -BraceStructTypeFoldableImpl! { - impl<'tcx> TypeFoldable<'tcx> for ObligationCause<'tcx> { - span, body_id, code - } -} - -#[derive(Clone, Debug, PartialEq, Eq, Hash, HashStable, RustcEncodable, RustcDecodable)] +#[derive(Clone, Debug, PartialEq, Eq, Hash, HashStable, RustcEncodable, RustcDecodable, + TypeFoldable)] pub enum ObligationCauseCode<'tcx> { /// Not well classified or should be obvious from the span. MiscObligation, @@ -292,7 +288,8 @@ pub enum ObligationCauseCode<'tcx> { AssocTypeBound(Box), } -#[derive(Clone, Debug, PartialEq, Eq, Hash)] +#[derive(Clone, Debug, PartialEq, Eq, Hash, HashStable, RustcEncodable, RustcDecodable, + TypeFoldable)] pub struct AssocTypeBoundData { pub impl_span: Option, pub original: Span, @@ -303,57 +300,8 @@ pub struct AssocTypeBoundData { #[cfg(target_arch = "x86_64")] static_assert_size!(ObligationCauseCode<'_>, 32); -EnumTypeFoldableImpl! { - impl<'tcx> TypeFoldable<'tcx> for ObligationCauseCode<'tcx> { - (ObligationCauseCode::MiscObligation), - (ObligationCauseCode::SliceOrArrayElem), - (ObligationCauseCode::TupleElem), - (ObligationCauseCode::ProjectionWf)(a), - (ObligationCauseCode::ItemObligation)(a), - (ObligationCauseCode::BindingObligation)(a, b), - (ObligationCauseCode::ReferenceOutlivesReferent)(a), - (ObligationCauseCode::ObjectTypeBound)(a, b), - (ObligationCauseCode::ObjectCastObligation)(a), - (ObligationCauseCode::Coercion){source, target}, - (ObligationCauseCode::AssignmentLhsSized), - (ObligationCauseCode::TupleInitializerSized), - (ObligationCauseCode::StructInitializerSized), - (ObligationCauseCode::VariableType)(a), - (ObligationCauseCode::SizedArgumentType), - (ObligationCauseCode::SizedReturnType), - (ObligationCauseCode::SizedYieldType), - (ObligationCauseCode::RepeatVec), - (ObligationCauseCode::FieldSized){ adt_kind, last }, - (ObligationCauseCode::ConstSized), - (ObligationCauseCode::SharedStatic), - (ObligationCauseCode::BuiltinDerivedObligation)(a), - (ObligationCauseCode::ImplDerivedObligation)(a), - (ObligationCauseCode::CompareImplMethodObligation){ - item_name, - impl_item_def_id, - trait_item_def_id - }, - (ObligationCauseCode::ExprAssignable), - (ObligationCauseCode::MatchExpressionArm)(a), - (ObligationCauseCode::MatchExpressionArmPattern){ - span, - ty - }, - (ObligationCauseCode::IfExpression)(a), - (ObligationCauseCode::IfExpressionWithNoElse), - (ObligationCauseCode::MainFunctionType), - (ObligationCauseCode::StartFunctionType), - (ObligationCauseCode::IntrinsicType), - (ObligationCauseCode::MethodReceiver), - (ObligationCauseCode::ReturnNoExpression), - (ObligationCauseCode::ReturnValue)(a), - (ObligationCauseCode::ReturnType), - (ObligationCauseCode::BlockTailExpression)(a), - (ObligationCauseCode::TrivialBound), - } -} - -#[derive(Clone, Debug, PartialEq, Eq, Hash, HashStable, RustcEncodable, RustcDecodable)] +#[derive(Clone, Debug, PartialEq, Eq, Hash, HashStable, RustcEncodable, RustcDecodable, + TypeFoldable)] pub struct MatchExpressionArmCause<'tcx> { pub arm_span: Span, pub source: hir::MatchSource, @@ -362,26 +310,16 @@ pub struct MatchExpressionArmCause<'tcx> { pub discrim_hir_id: hir::HirId, } -BraceStructTypeFoldableImpl! { - impl<'tcx> TypeFoldable<'tcx> for MatchExpressionArmCause<'tcx> { - arm_span, source, prior_arms, last_ty, discrim_hir_id - } -} - -#[derive(Clone, Debug, PartialEq, Eq, Hash, HashStable, RustcEncodable, RustcDecodable)] +#[derive(Clone, Debug, PartialEq, Eq, Hash, HashStable, RustcEncodable, RustcDecodable, + TypeFoldable)] pub struct IfExpressionCause { pub then: Span, pub outer: Option, pub semicolon: Option, } -BraceStructTypeFoldableImpl! { - impl<'tcx> TypeFoldable<'tcx> for IfExpressionCause { - then, outer, semicolon - } -} - -#[derive(Clone, Debug, PartialEq, Eq, Hash, HashStable, RustcEncodable, RustcDecodable)] +#[derive(Clone, Debug, PartialEq, Eq, Hash, HashStable, RustcEncodable, RustcDecodable, + TypeFoldable)] pub struct DerivedObligationCause<'tcx> { /// The trait reference of the parent obligation that led to the /// current obligation. Note that only trait obligations lead to @@ -393,13 +331,6 @@ pub struct DerivedObligationCause<'tcx> { parent_code: Arc> } -BraceStructTypeFoldableImpl! { - impl<'tcx> TypeFoldable<'tcx> for DerivedObligationCause<'tcx> { - parent_trait_ref, parent_code - } -} - - pub type Obligations<'tcx, O> = Vec>; pub type PredicateObligations<'tcx> = Vec>; pub type TraitObligations<'tcx> = Vec>; From 6840bc4bc928cf94d600222fc3405a26bbbe8f57 Mon Sep 17 00:00:00 2001 From: Aaron Hill Date: Mon, 18 Nov 2019 16:22:05 -0500 Subject: [PATCH 05/15] Update stderr files --- .../ui/async-await/async-fn-nonsend.stderr | 68 ++----------------- .../recursive-impl-trait-type-indirect.stderr | 4 +- 2 files changed, 7 insertions(+), 65 deletions(-) diff --git a/src/test/ui/async-await/async-fn-nonsend.stderr b/src/test/ui/async-await/async-fn-nonsend.stderr index 6e89deb407e92..1d8735237c26a 100644 --- a/src/test/ui/async-await/async-fn-nonsend.stderr +++ b/src/test/ui/async-await/async-fn-nonsend.stderr @@ -4,76 +4,18 @@ error[E0277]: `std::rc::Rc<()>` cannot be sent between threads safely LL | fn assert_send(_: impl Send) {} | ----------- ---- required by this bound in `assert_send` ... -LL | assert_send(local_dropped_before_await()); - | ^^^^^^^^^^^ `std::rc::Rc<()>` cannot be sent between threads safely - | - = help: within `impl std::future::Future`, the trait `std::marker::Send` is not implemented for `std::rc::Rc<()>` - = note: required because it appears within the type `impl std::fmt::Debug` - = note: required because it appears within the type `{impl std::fmt::Debug, impl std::future::Future, impl std::future::Future, ()}` - = note: required because it appears within the type `[static generator@$DIR/async-fn-nonsend.rs:21:39: 26:2 {impl std::fmt::Debug, impl std::future::Future, impl std::future::Future, ()}]` - = note: required because it appears within the type `std::future::GenFuture<[static generator@$DIR/async-fn-nonsend.rs:21:39: 26:2 {impl std::fmt::Debug, impl std::future::Future, impl std::future::Future, ()}]>` - = note: required because it appears within the type `impl std::future::Future` - = note: required because it appears within the type `impl std::future::Future` - -error[E0277]: `std::rc::Rc<()>` cannot be sent between threads safely - --> $DIR/async-fn-nonsend.rs:52:5 - | -LL | fn assert_send(_: impl Send) {} - | ----------- ---- required by this bound in `assert_send` -... LL | assert_send(non_send_temporary_in_match()); | ^^^^^^^^^^^ `std::rc::Rc<()>` cannot be sent between threads safely | = help: within `impl std::future::Future`, the trait `std::marker::Send` is not implemented for `std::rc::Rc<()>` = note: required because it appears within the type `impl std::fmt::Debug` - = note: required because it appears within the type `{impl std::fmt::Debug, std::option::Option, impl std::future::Future, impl std::future::Future, ()}` - = note: required because it appears within the type `[static generator@$DIR/async-fn-nonsend.rs:28:40: 37:2 {impl std::fmt::Debug, std::option::Option, impl std::future::Future, impl std::future::Future, ()}]` - = note: required because it appears within the type `std::future::GenFuture<[static generator@$DIR/async-fn-nonsend.rs:28:40: 37:2 {impl std::fmt::Debug, std::option::Option, impl std::future::Future, impl std::future::Future, ()}]>` - = note: required because it appears within the type `impl std::future::Future` - = note: required because it appears within the type `impl std::future::Future` - -error[E0277]: `dyn std::fmt::Write` cannot be sent between threads safely - --> $DIR/async-fn-nonsend.rs:54:5 - | -LL | fn assert_send(_: impl Send) {} - | ----------- ---- required by this bound in `assert_send` -... -LL | assert_send(non_sync_with_method_call()); - | ^^^^^^^^^^^ `dyn std::fmt::Write` cannot be sent between threads safely - | - = help: the trait `std::marker::Send` is not implemented for `dyn std::fmt::Write` - = note: required because of the requirements on the impl of `std::marker::Send` for `&mut dyn std::fmt::Write` - = note: required because it appears within the type `std::fmt::Formatter<'_>` - = note: required because of the requirements on the impl of `std::marker::Send` for `&mut std::fmt::Formatter<'_>` - = note: required because it appears within the type `for<'r, 's> {&'r mut std::fmt::Formatter<'s>, bool, bool, impl std::future::Future, impl std::future::Future, ()}` - = note: required because it appears within the type `[static generator@$DIR/async-fn-nonsend.rs:39:38: 45:2 for<'r, 's> {&'r mut std::fmt::Formatter<'s>, bool, bool, impl std::future::Future, impl std::future::Future, ()}]` - = note: required because it appears within the type `std::future::GenFuture<[static generator@$DIR/async-fn-nonsend.rs:39:38: 45:2 for<'r, 's> {&'r mut std::fmt::Formatter<'s>, bool, bool, impl std::future::Future, impl std::future::Future, ()}]>` - = note: required because it appears within the type `impl std::future::Future` - = note: required because it appears within the type `impl std::future::Future` - -error[E0277]: `*mut (dyn std::ops::Fn() + 'static)` cannot be shared between threads safely - --> $DIR/async-fn-nonsend.rs:54:5 - | -LL | fn assert_send(_: impl Send) {} - | ----------- ---- required by this bound in `assert_send` -... -LL | assert_send(non_sync_with_method_call()); - | ^^^^^^^^^^^ `*mut (dyn std::ops::Fn() + 'static)` cannot be shared between threads safely - | - = help: within `std::fmt::ArgumentV1<'_>`, the trait `std::marker::Sync` is not implemented for `*mut (dyn std::ops::Fn() + 'static)` - = note: required because it appears within the type `std::marker::PhantomData<*mut (dyn std::ops::Fn() + 'static)>` - = note: required because it appears within the type `core::fmt::Void` - = note: required because it appears within the type `&core::fmt::Void` - = note: required because it appears within the type `std::fmt::ArgumentV1<'_>` - = note: required because of the requirements on the impl of `std::marker::Send` for `std::slice::Iter<'_, std::fmt::ArgumentV1<'_>>` - = note: required because it appears within the type `std::fmt::Formatter<'_>` - = note: required because of the requirements on the impl of `std::marker::Send` for `&mut std::fmt::Formatter<'_>` - = note: required because it appears within the type `for<'r, 's> {&'r mut std::fmt::Formatter<'s>, bool, bool, impl std::future::Future, impl std::future::Future, ()}` - = note: required because it appears within the type `[static generator@$DIR/async-fn-nonsend.rs:39:38: 45:2 for<'r, 's> {&'r mut std::fmt::Formatter<'s>, bool, bool, impl std::future::Future, impl std::future::Future, ()}]` - = note: required because it appears within the type `std::future::GenFuture<[static generator@$DIR/async-fn-nonsend.rs:39:38: 45:2 for<'r, 's> {&'r mut std::fmt::Formatter<'s>, bool, bool, impl std::future::Future, impl std::future::Future, ()}]>` + = note: required because it appears within the type `std::option::Option` + = note: required because it appears within the type `[witness@DefId(0:25 ~ async_fn_nonsend[8787]::non_send_temporary_in_match[0]::{{closure}}[0]){impl std::fmt::Debug, std::option::Option, impl std::future::Future, impl std::future::Future, ()}]` + = note: required because it appears within the type `[static generator@$DIR/async-fn-nonsend.rs:28:40: 37:2 [witness@DefId(0:25 ~ async_fn_nonsend[8787]::non_send_temporary_in_match[0]::{{closure}}[0]){impl std::fmt::Debug, std::option::Option, impl std::future::Future, impl std::future::Future, ()}]]` + = note: required because it appears within the type `std::future::GenFuture<[static generator@$DIR/async-fn-nonsend.rs:28:40: 37:2 [witness@DefId(0:25 ~ async_fn_nonsend[8787]::non_send_temporary_in_match[0]::{{closure}}[0]){impl std::fmt::Debug, std::option::Option, impl std::future::Future, impl std::future::Future, ()}]]>` = note: required because it appears within the type `impl std::future::Future` = note: required because it appears within the type `impl std::future::Future` -error: aborting due to 4 previous errors +error: aborting due to previous error For more information about this error, try `rustc --explain E0277`. diff --git a/src/test/ui/impl-trait/recursive-impl-trait-type-indirect.stderr b/src/test/ui/impl-trait/recursive-impl-trait-type-indirect.stderr index b7ba0d6ab177c..c93a45f5b22f2 100644 --- a/src/test/ui/impl-trait/recursive-impl-trait-type-indirect.stderr +++ b/src/test/ui/impl-trait/recursive-impl-trait-type-indirect.stderr @@ -76,7 +76,7 @@ error[E0720]: opaque type expands to a recursive type LL | fn generator_capture() -> impl Sized { | ^^^^^^^^^^ expands to a recursive type | - = note: expanded type is `[generator@$DIR/recursive-impl-trait-type-indirect.rs:50:5: 50:26 x:impl Sized {()}]` + = note: expanded type is `[generator@$DIR/recursive-impl-trait-type-indirect.rs:50:5: 50:26 x:impl Sized [witness@DefId(0:27 ~ recursive_impl_trait_type_indirect[317d]::generator_capture[0]::{{closure}}[0]){()}]]` error[E0720]: opaque type expands to a recursive type --> $DIR/recursive-impl-trait-type-indirect.rs:53:26 @@ -92,7 +92,7 @@ error[E0720]: opaque type expands to a recursive type LL | fn generator_hold() -> impl Sized { | ^^^^^^^^^^ expands to a recursive type | - = note: expanded type is `[generator@$DIR/recursive-impl-trait-type-indirect.rs:58:5: 62:6 {impl Sized, ()}]` + = note: expanded type is `[generator@$DIR/recursive-impl-trait-type-indirect.rs:58:5: 62:6 [witness@DefId(0:33 ~ recursive_impl_trait_type_indirect[317d]::generator_hold[0]::{{closure}}[0]){impl Sized, ()}]]` error[E0720]: opaque type expands to a recursive type --> $DIR/recursive-impl-trait-type-indirect.rs:69:26 From bea0a4253b33783a620bd114d852cf695155c216 Mon Sep 17 00:00:00 2001 From: Aaron Hill Date: Mon, 2 Dec 2019 16:47:28 -0500 Subject: [PATCH 06/15] Perform generator field computation before optimizations --- src/librustc/mir/mod.rs | 5 ++- src/librustc/traits/select.rs | 13 ++++++-- src/librustc_mir/transform/generator.rs | 41 ++++++++++++++++++++++--- src/librustc_mir/transform/mod.rs | 9 ++++-- 4 files changed, 58 insertions(+), 10 deletions(-) diff --git a/src/librustc/mir/mod.rs b/src/librustc/mir/mod.rs index 814c244ecbc2b..5d7f678e3603d 100644 --- a/src/librustc/mir/mod.rs +++ b/src/librustc/mir/mod.rs @@ -38,7 +38,7 @@ use syntax::symbol::Symbol; use syntax_pos::{Span, DUMMY_SP}; pub use crate::mir::interpret::AssertMessage; -pub use crate::mir::cache::{BodyCache, ReadOnlyBodyCache}; +pub use crate::mir::cache::{Cache, BodyCache, ReadOnlyBodyCache}; pub use crate::read_only; mod cache; @@ -117,6 +117,8 @@ pub struct Body<'tcx> { /// to be created. pub generator_kind: Option, + pub generator_interior_tys: Option>>, + /// Declarations of locals. /// /// The first local is the return value pointer, followed by `arg_count` @@ -184,6 +186,7 @@ impl<'tcx> Body<'tcx> { generator_drop: None, generator_layout: None, generator_kind, + generator_interior_tys: None, local_decls, user_type_annotations, arg_count, diff --git a/src/librustc/traits/select.rs b/src/librustc/traits/select.rs index 94f62f2cec682..31862eb0448a6 100644 --- a/src/librustc/traits/select.rs +++ b/src/librustc/traits/select.rs @@ -2755,9 +2755,9 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> { ty::GeneratorWitness(did, types) => { // Note that we need to use optimized_mir here, // in order to have the `StateTransform` pass run - let gen_mir = self.tcx().optimized_mir(did); + /*let gen_mir = self.tcx().optimized_mir(did); let gen_layout = gen_mir.generator_layout.as_ref() - .expect("Missing generator layout!"); + .expect("Missing generator layout!");*/ // We need to compare the types from the GeneratoWitness // to the types from the MIR. Since the generator MIR (specifically @@ -2806,7 +2806,14 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> { let mut used_types = Vec::new(); - for ty in &gen_layout.field_tys { + // Note - while we use `optimized_mir`, the .generator_interior_tys + // field is only set during construction of the original MIR, + // and is unaffected by optimizations + let interior_tys = self.tcx().optimized_mir(did).generator_interior_tys + .as_ref().expect("Missing generator interior types!"); + + //for ty in &gen_layout.field_tys { + for ty in interior_tys { if let Some(witness_ty) = erased_types.get(&self.tcx().erase_regions(ty)) { used_types.push(**witness_ty); } diff --git a/src/librustc_mir/transform/generator.rs b/src/librustc_mir/transform/generator.rs index eca4864d63052..90a826a4595e6 100644 --- a/src/librustc_mir/transform/generator.rs +++ b/src/librustc_mir/transform/generator.rs @@ -757,10 +757,11 @@ fn compute_layout<'tcx>( GeneratorLayout<'tcx>, FxHashMap, ) { + // Use a liveness analysis to compute locals which are live across a suspension point - let LivenessInfo { + let (LivenessInfo { live_locals, live_locals_at_suspension_points, storage_conflicts, storage_liveness - } = locals_live_across_suspend_points(tcx, read_only!(body), source, movable); + }, tys) = compute_field_tys(tcx, source, movable, read_only!(body)); // Erase regions from the types passed in from typeck so we can compare them with // MIR types @@ -789,10 +790,8 @@ fn compute_layout<'tcx>( // Gather live local types and their indices. let mut locals = IndexVec::::new(); - let mut tys = IndexVec::::new(); for (idx, local) in live_locals.iter().enumerate() { locals.push(local); - tys.push(body.local_decls[local].ty); debug!("generator saved local {:?} => {:?}", GeneratorSavedLocal::from(idx), local); } @@ -1168,6 +1167,40 @@ where }).collect() } +pub(super) fn generator_interior_tys<'tcx>( + tcx: TyCtxt<'tcx>, + def_id: DefId, + body: &BodyCache<'tcx> +) -> Vec> { + // The first argument is the generator type passed by value + //let gen_ty = body.local_decls.raw[1].tyi; + let gen_ty = tcx.type_of(def_id); + let movable = match gen_ty.kind { + ty::Generator(_, _, movability) => { + movability == hir::Movability::Movable + }, + _ => bug!("Unexpected type {:?}", gen_ty) + }; + + compute_field_tys(tcx, MirSource::item(def_id), movable, body.unwrap_read_only()).1.raw +} + +fn compute_field_tys( + tcx: TyCtxt<'tcx>, + source: MirSource<'tcx>, + movable: bool, + body: ReadOnlyBodyCache<'_, 'tcx> +) -> (LivenessInfo, IndexVec>) { + let liveness_info = locals_live_across_suspend_points( + tcx, body, source, movable + ); + let mut tys = IndexVec::::new(); + for local in liveness_info.live_locals.iter() { + tys.push(body.local_decls[local].ty); + } + (liveness_info, tys) +} + impl<'tcx> MirPass<'tcx> for StateTransform { fn run_pass( &self, tcx: TyCtxt<'tcx>, source: MirSource<'tcx>, body: &mut BodyCache<'tcx> diff --git a/src/librustc_mir/transform/mod.rs b/src/librustc_mir/transform/mod.rs index df4cb7615336a..ee9555ae8a63b 100644 --- a/src/librustc_mir/transform/mod.rs +++ b/src/librustc_mir/transform/mod.rs @@ -2,7 +2,7 @@ use crate::{build, shim}; use rustc_index::vec::IndexVec; use rustc::hir::def_id::{CrateNum, DefId, LOCAL_CRATE}; use rustc::mir::{BodyCache, MirPhase, Promoted, ConstQualifs}; -use rustc::ty::{TyCtxt, InstanceDef, TypeFoldable}; +use rustc::ty::{self, TyCtxt, InstanceDef, TypeFoldable}; use rustc::ty::query::Providers; use rustc::ty::steal::Steal; use rustc::hir; @@ -98,7 +98,12 @@ fn mir_keys(tcx: TyCtxt<'_>, krate: CrateNum) -> &DefIdSet { } fn mir_built(tcx: TyCtxt<'_>, def_id: DefId) -> &Steal> { - let mir = build::mir_build(tcx, def_id); + let mut mir = build::mir_build(tcx, def_id); + if let ty::Generator(..) = tcx.type_of(def_id).kind { + let interior_types = generator::generator_interior_tys(tcx, def_id, &mir); + mir.generator_interior_tys = Some(interior_types); + } + tcx.alloc_steal_mir(mir) } From 2b2986d1c296e726dd1edfd97f5d511a953883e8 Mon Sep 17 00:00:00 2001 From: Aaron Hill Date: Mon, 2 Dec 2019 17:21:48 -0500 Subject: [PATCH 07/15] Gate more precise generator auto traits behind feature --- src/librustc/query/mod.rs | 4 ++++ src/librustc/traits/select.rs | 12 ++++++++++++ src/librustc_feature/active.rs | 2 ++ src/librustc_metadata/rmeta/decoder/cstore_impl.rs | 4 ++++ src/librustc_metadata/rmeta/encoder.rs | 1 + src/librustc_metadata/rmeta/mod.rs | 1 + src/libsyntax_pos/symbol.rs | 1 + 7 files changed, 25 insertions(+) diff --git a/src/librustc/query/mod.rs b/src/librustc/query/mod.rs index a6d7e5c9291dc..371bab60d301a 100644 --- a/src/librustc/query/mod.rs +++ b/src/librustc/query/mod.rs @@ -931,6 +931,10 @@ rustc_queries! { query all_traits(_: CrateNum) -> &'tcx [DefId] { desc { "fetching all foreign and local traits" } } + + query uses_generator_mir_traits(_: CrateNum) -> bool { + desc { "determining whether crate uses generator MIR traits" } + } } Linking { diff --git a/src/librustc/traits/select.rs b/src/librustc/traits/select.rs index 31862eb0448a6..bd4dfd6c60b81 100644 --- a/src/librustc/traits/select.rs +++ b/src/librustc/traits/select.rs @@ -2753,6 +2753,18 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> { } ty::GeneratorWitness(did, types) => { + let use_mir = if did.is_local() { + self.tcx().features().generator_mir_traits + } else { + self.tcx().uses_generator_mir_traits(did.krate) + }; + + if !use_mir { + // This is sound because no regions in the witness can refer to + // the binder outside the witness. So we'll effectivly reuse + // the implicit binder around the witness. + return types.skip_binder().to_vec() + } // Note that we need to use optimized_mir here, // in order to have the `StateTransform` pass run /*let gen_mir = self.tcx().optimized_mir(did); diff --git a/src/librustc_feature/active.rs b/src/librustc_feature/active.rs index b1ae7c6ca33d8..fd93fcfa246d9 100644 --- a/src/librustc_feature/active.rs +++ b/src/librustc_feature/active.rs @@ -526,6 +526,8 @@ declare_features! ( /// Allows using `&mut` in constant functions. (active, const_mut_refs, "1.41.0", Some(57349), None), + (active, generator_mir_traits, "1.41.0", None, None), + // ------------------------------------------------------------------------- // feature-group-end: actual feature gates // ------------------------------------------------------------------------- diff --git a/src/librustc_metadata/rmeta/decoder/cstore_impl.rs b/src/librustc_metadata/rmeta/decoder/cstore_impl.rs index 13db9a6fef9ca..ae8740aeb493c 100644 --- a/src/librustc_metadata/rmeta/decoder/cstore_impl.rs +++ b/src/librustc_metadata/rmeta/decoder/cstore_impl.rs @@ -240,6 +240,10 @@ provide! { <'tcx> tcx, def_id, other, cdata, Arc::new(syms) } + + uses_generator_mir_traits => { + cdata.root.generator_mir_traits + } } pub fn provide(providers: &mut Providers<'_>) { diff --git a/src/librustc_metadata/rmeta/encoder.rs b/src/librustc_metadata/rmeta/encoder.rs index fb70e10c84f3d..fd42f177212a7 100644 --- a/src/librustc_metadata/rmeta/encoder.rs +++ b/src/librustc_metadata/rmeta/encoder.rs @@ -544,6 +544,7 @@ impl<'tcx> EncodeContext<'tcx> { exported_symbols, interpret_alloc_index, per_def, + generator_mir_traits: tcx.features().generator_mir_traits }); let total_bytes = self.position(); diff --git a/src/librustc_metadata/rmeta/mod.rs b/src/librustc_metadata/rmeta/mod.rs index fdf43f06eb10f..4359057587bc8 100644 --- a/src/librustc_metadata/rmeta/mod.rs +++ b/src/librustc_metadata/rmeta/mod.rs @@ -214,6 +214,7 @@ crate struct CrateRoot<'tcx> { profiler_runtime: bool, sanitizer_runtime: bool, symbol_mangling_version: SymbolManglingVersion, + generator_mir_traits: bool } #[derive(RustcEncodable, RustcDecodable)] diff --git a/src/libsyntax_pos/symbol.rs b/src/libsyntax_pos/symbol.rs index e8f7a125739ac..7d98df81f74c7 100644 --- a/src/libsyntax_pos/symbol.rs +++ b/src/libsyntax_pos/symbol.rs @@ -334,6 +334,7 @@ symbols! { FxHashSet, FxHashMap, gen_future, + generator_mir_traits, generators, generic_associated_types, generic_param_attrs, From e4684c74421761adfd8bd078064977f9b1005f94 Mon Sep 17 00:00:00 2001 From: Aaron Hill Date: Mon, 2 Dec 2019 17:23:44 -0500 Subject: [PATCH 08/15] Remove Freeze special-case for generators --- src/librustc/traits/select.rs | 5 ----- 1 file changed, 5 deletions(-) diff --git a/src/librustc/traits/select.rs b/src/librustc/traits/select.rs index bd4dfd6c60b81..c1766b7abcce2 100644 --- a/src/librustc/traits/select.rs +++ b/src/librustc/traits/select.rs @@ -2177,11 +2177,6 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> { } } } - ty::Generator(..) - if self.tcx().lang_items().freeze_trait() == Some(def_id) => - { - // For now, always consider generators to be !Freeze - } _ => candidates.vec.push(AutoImplCandidate(def_id.clone())), } From 97116eb10ddbe2ecae962d168727bf7b611c2355 Mon Sep 17 00:00:00 2001 From: Aaron Hill Date: Mon, 2 Dec 2019 17:57:47 -0500 Subject: [PATCH 09/15] Some cleanup --- src/librustc/traits/select.rs | 10 +++++----- src/librustc_feature/active.rs | 2 +- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/librustc/traits/select.rs b/src/librustc/traits/select.rs index c1766b7abcce2..661e19e84e8cf 100644 --- a/src/librustc/traits/select.rs +++ b/src/librustc/traits/select.rs @@ -2748,17 +2748,17 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> { } ty::GeneratorWitness(did, types) => { - let use_mir = if did.is_local() { + let use_mir = if did.is_local() { self.tcx().features().generator_mir_traits } else { self.tcx().uses_generator_mir_traits(did.krate) }; if !use_mir { - // This is sound because no regions in the witness can refer to - // the binder outside the witness. So we'll effectivly reuse - // the implicit binder around the witness. - return types.skip_binder().to_vec() + // This is sound because no regions in the witness can refer to + // the binder outside the witness. So we'll effectivly reuse + // the implicit binder around the witness. + return types.skip_binder().to_vec() } // Note that we need to use optimized_mir here, // in order to have the `StateTransform` pass run diff --git a/src/librustc_feature/active.rs b/src/librustc_feature/active.rs index fd93fcfa246d9..503934ab33e71 100644 --- a/src/librustc_feature/active.rs +++ b/src/librustc_feature/active.rs @@ -526,7 +526,7 @@ declare_features! ( /// Allows using `&mut` in constant functions. (active, const_mut_refs, "1.41.0", Some(57349), None), - (active, generator_mir_traits, "1.41.0", None, None), + (active, generator_mir_traits, "1.41.0", Some(0), None), // ------------------------------------------------------------------------- // feature-group-end: actual feature gates From b3e6f8ddd1ea843cb088eea6cd31be491ea29fb1 Mon Sep 17 00:00:00 2001 From: Aaron Hill Date: Mon, 2 Dec 2019 18:00:09 -0500 Subject: [PATCH 10/15] Update tests --- src/test/ui/async-await/async-fn-nonsend.rs | 2 ++ .../ui/async-await/async-fn-nonsend.stderr | 6 ++--- .../feature-gate-generator_mir_traits.rs | 16 +++++++++++++ .../feature-gate-generator_mir_traits.stderr | 23 +++++++++++++++++++ 4 files changed, 44 insertions(+), 3 deletions(-) create mode 100644 src/test/ui/feature-gate/feature-gate-generator_mir_traits.rs create mode 100644 src/test/ui/feature-gate/feature-gate-generator_mir_traits.stderr diff --git a/src/test/ui/async-await/async-fn-nonsend.rs b/src/test/ui/async-await/async-fn-nonsend.rs index ee13543f4492a..051579fc9fe1a 100644 --- a/src/test/ui/async-await/async-fn-nonsend.rs +++ b/src/test/ui/async-await/async-fn-nonsend.rs @@ -2,6 +2,8 @@ // edition:2018 // compile-flags: --crate-type lib +#![feature(generator_mir_traits)] + use std::{ cell::RefCell, fmt::Debug, diff --git a/src/test/ui/async-await/async-fn-nonsend.stderr b/src/test/ui/async-await/async-fn-nonsend.stderr index 1d8735237c26a..ad32000574a19 100644 --- a/src/test/ui/async-await/async-fn-nonsend.stderr +++ b/src/test/ui/async-await/async-fn-nonsend.stderr @@ -1,5 +1,5 @@ error[E0277]: `std::rc::Rc<()>` cannot be sent between threads safely - --> $DIR/async-fn-nonsend.rs:50:5 + --> $DIR/async-fn-nonsend.rs:52:5 | LL | fn assert_send(_: impl Send) {} | ----------- ---- required by this bound in `assert_send` @@ -11,8 +11,8 @@ LL | assert_send(non_send_temporary_in_match()); = note: required because it appears within the type `impl std::fmt::Debug` = note: required because it appears within the type `std::option::Option` = note: required because it appears within the type `[witness@DefId(0:25 ~ async_fn_nonsend[8787]::non_send_temporary_in_match[0]::{{closure}}[0]){impl std::fmt::Debug, std::option::Option, impl std::future::Future, impl std::future::Future, ()}]` - = note: required because it appears within the type `[static generator@$DIR/async-fn-nonsend.rs:28:40: 37:2 [witness@DefId(0:25 ~ async_fn_nonsend[8787]::non_send_temporary_in_match[0]::{{closure}}[0]){impl std::fmt::Debug, std::option::Option, impl std::future::Future, impl std::future::Future, ()}]]` - = note: required because it appears within the type `std::future::GenFuture<[static generator@$DIR/async-fn-nonsend.rs:28:40: 37:2 [witness@DefId(0:25 ~ async_fn_nonsend[8787]::non_send_temporary_in_match[0]::{{closure}}[0]){impl std::fmt::Debug, std::option::Option, impl std::future::Future, impl std::future::Future, ()}]]>` + = note: required because it appears within the type `[static generator@$DIR/async-fn-nonsend.rs:30:40: 39:2 [witness@DefId(0:25 ~ async_fn_nonsend[8787]::non_send_temporary_in_match[0]::{{closure}}[0]){impl std::fmt::Debug, std::option::Option, impl std::future::Future, impl std::future::Future, ()}]]` + = note: required because it appears within the type `std::future::GenFuture<[static generator@$DIR/async-fn-nonsend.rs:30:40: 39:2 [witness@DefId(0:25 ~ async_fn_nonsend[8787]::non_send_temporary_in_match[0]::{{closure}}[0]){impl std::fmt::Debug, std::option::Option, impl std::future::Future, impl std::future::Future, ()}]]>` = note: required because it appears within the type `impl std::future::Future` = note: required because it appears within the type `impl std::future::Future` diff --git a/src/test/ui/feature-gate/feature-gate-generator_mir_traits.rs b/src/test/ui/feature-gate/feature-gate-generator_mir_traits.rs new file mode 100644 index 0000000000000..e319d5e671df8 --- /dev/null +++ b/src/test/ui/feature-gate/feature-gate-generator_mir_traits.rs @@ -0,0 +1,16 @@ +// compile-fail +// edition:2018 + +fn is_send(val: T) {} + +async fn dummy() {} + +async fn not_send() { + let val: *const (); + dummy().await; +} + +fn main() { + is_send(not_send()); + //~^ ERROR `*const ()` cannot be sent between threads safely +} diff --git a/src/test/ui/feature-gate/feature-gate-generator_mir_traits.stderr b/src/test/ui/feature-gate/feature-gate-generator_mir_traits.stderr new file mode 100644 index 0000000000000..8a80803fca84b --- /dev/null +++ b/src/test/ui/feature-gate/feature-gate-generator_mir_traits.stderr @@ -0,0 +1,23 @@ +error[E0277]: `*const ()` cannot be sent between threads safely + --> $DIR/feature-gate-generator_mir_traits.rs:14:5 + | +LL | fn is_send(val: T) {} + | ------- ---- required by this bound in `is_send` +... +LL | is_send(not_send()); + | ^^^^^^^ `*const ()` cannot be sent between threads safely + | + = help: within `impl std::future::Future`, the trait `std::marker::Send` is not implemented for `*const ()` +note: future does not implement `std::marker::Send` as this value is used across an await + --> $DIR/feature-gate-generator_mir_traits.rs:10:5 + | +LL | let val: *const (); + | --- has type `*const ()` +LL | dummy().await; + | ^^^^^^^^^^^^^ await occurs here, with `val` maybe used later +LL | } + | - `val` is later dropped here + +error: aborting due to previous error + +For more information about this error, try `rustc --explain E0277`. From 0c92b837de11ea5e75a79b8eb60ff9fd28bc5267 Mon Sep 17 00:00:00 2001 From: Aaron Hill Date: Mon, 2 Dec 2019 18:03:20 -0500 Subject: [PATCH 11/15] Add check-pass test --- src/test/ui/async-await/generator-mir-traits.rs | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) create mode 100644 src/test/ui/async-await/generator-mir-traits.rs diff --git a/src/test/ui/async-await/generator-mir-traits.rs b/src/test/ui/async-await/generator-mir-traits.rs new file mode 100644 index 0000000000000..89f927a79c469 --- /dev/null +++ b/src/test/ui/async-await/generator-mir-traits.rs @@ -0,0 +1,17 @@ +// edition:2018 +// check-pass + +#![feature(generator_mir_traits)] + +fn is_send(val: T) {} + +async fn dummy() {} + +async fn not_send() { + let val: *const (); + dummy().await; +} + +fn main() { + is_send(not_send()); +} From b891e49f97fb5c87fd0d722a5aee730b7b5cbe2d Mon Sep 17 00:00:00 2001 From: Aaron Hill Date: Tue, 3 Dec 2019 15:54:11 -0500 Subject: [PATCH 12/15] Apply nits Co-Authored-By: Ralf Jung --- src/librustc/traits/fulfill.rs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/librustc/traits/fulfill.rs b/src/librustc/traits/fulfill.rs index e6b37b8e192e6..7dd9b76289e26 100644 --- a/src/librustc/traits/fulfill.rs +++ b/src/librustc/traits/fulfill.rs @@ -69,7 +69,7 @@ pub struct FulfillmentContext<'tcx> { // not available when `FnCxtxt` is run. has_delayed_generator_witness: bool, // The delayed generator witness predicates. This is only - // used when `has_delayed_generator_witness:` is `true` + // used when `has_delayed_generator_witness:` is `true`. delayed_generator_witness: DelayedGenerators<'tcx>, } @@ -91,7 +91,7 @@ impl<'a, 'tcx> FulfillmentContext<'tcx> { register_region_obligations: true, usable_in_snapshot: false, delayed_generator_witness: None, - has_delayed_generator_witness: false + has_delayed_generator_witness: false, } } @@ -101,7 +101,7 @@ impl<'a, 'tcx> FulfillmentContext<'tcx> { register_region_obligations: true, usable_in_snapshot: false, delayed_generator_witness: Some(Vec::new()), - has_delayed_generator_witness: true + has_delayed_generator_witness: true, } } @@ -111,7 +111,7 @@ impl<'a, 'tcx> FulfillmentContext<'tcx> { register_region_obligations: true, usable_in_snapshot: true, delayed_generator_witness: None, - has_delayed_generator_witness: false + has_delayed_generator_witness: false, } } @@ -121,7 +121,7 @@ impl<'a, 'tcx> FulfillmentContext<'tcx> { register_region_obligations: false, usable_in_snapshot: false, delayed_generator_witness: None, - has_delayed_generator_witness: false + has_delayed_generator_witness: false, } } From 87fc30d668e36ea06a3a3b7f507a034351f29f65 Mon Sep 17 00:00:00 2001 From: Aaron Hill Date: Tue, 3 Dec 2019 16:10:44 -0500 Subject: [PATCH 13/15] Cleanup trait selection logic --- src/librustc/traits/select.rs | 35 +++++++++++++++-------------------- 1 file changed, 15 insertions(+), 20 deletions(-) diff --git a/src/librustc/traits/select.rs b/src/librustc/traits/select.rs index 661e19e84e8cf..1a03acd63f7ef 100644 --- a/src/librustc/traits/select.rs +++ b/src/librustc/traits/select.rs @@ -2760,21 +2760,12 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> { // the implicit binder around the witness. return types.skip_binder().to_vec() } - // Note that we need to use optimized_mir here, - // in order to have the `StateTransform` pass run - /*let gen_mir = self.tcx().optimized_mir(did); - let gen_layout = gen_mir.generator_layout.as_ref() - .expect("Missing generator layout!");*/ - - // We need to compare the types from the GeneratoWitness - // to the types from the MIR. Since the generator MIR (specifically - // the StateTransform pass) runs after lifetimes are erased, we must - // erase lifetime from our witness types in order to perform the comparsion + // We ignore all regions when comparing the computed interior + // MIR types to the witness types. + // + // We use `erase_late_bound_regions` to remove the outermost + // layer of late-bound regions from the witness types. // - // First, we erase all of late-bound regions bound by - // the witness type. - let unbound_tys = self.tcx().erase_late_bound_regions(&types); - // However, we may still have other regions within the inner // witness type (eg.. 'static'). Thus, we need to call // 'erase_regions' on each of the inner witness types. @@ -2806,8 +2797,11 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> { // Note that we need map erased types back to the orignal type. // This allows to return the original type from the GeneratorWitness // in our Vec of consiitutent types - preservering the internal regions - let erased_types: FxHashMap<_, _> = self.tcx().erase_regions(&unbound_tys) - .into_iter() + let erased_witness_types = self.tcx().erase_regions( + &self.tcx().erase_late_bound_regions(&types) + ); + + let erased_types: FxHashMap<_, _> = erased_witness_types.into_iter() .zip(types.skip_binder().into_iter()) .collect(); @@ -2819,11 +2813,12 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> { let interior_tys = self.tcx().optimized_mir(did).generator_interior_tys .as_ref().expect("Missing generator interior types!"); - //for ty in &gen_layout.field_tys { for ty in interior_tys { - if let Some(witness_ty) = erased_types.get(&self.tcx().erase_regions(ty)) { - used_types.push(**witness_ty); - } + let witness_ty = erased_types.get(&self.tcx().erase_regions(ty)) + .unwrap_or_else(|| panic!("Interior type {:?} not in {:?}", + ty, erased_types)); + + used_types.push(**witness_ty); } debug!("Used witness types for witness {:?} are: '{:?}'", t, used_types); From 8f58a71a88e1201341a026992da4707c8a4e62cf Mon Sep 17 00:00:00 2001 From: Aaron Hill Date: Tue, 3 Dec 2019 16:28:24 -0500 Subject: [PATCH 14/15] Fix missed nit Co-Authored-By: Ralf Jung --- src/librustc/traits/engine.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/librustc/traits/engine.rs b/src/librustc/traits/engine.rs index 75b23a722b9dc..b4e3e2f64e0da 100644 --- a/src/librustc/traits/engine.rs +++ b/src/librustc/traits/engine.rs @@ -105,7 +105,7 @@ impl dyn TraitEngine<'tcx> { /// Creates a `TraitEngine` in a special 'delay generator witness' mode. /// This imposes additional requirements for the caller in order to avoid /// accepting unsound code, and should only be used by `FnCtxt`. All other - /// users of `TraitEngine` should use `TraitEngine::new` + /// users of `TraitEngine` should use `TraitEngine::new`. /// /// A `TraitEngine` returned by this constructor will not attempt /// to resolve any `GeneratorWitness` predicates involving auto traits, From 71c9dd5ea3abaf2e505dc95eaa5968bfb366b8de Mon Sep 17 00:00:00 2001 From: Aaron Hill Date: Tue, 3 Dec 2019 17:28:46 -0500 Subject: [PATCH 15/15] Refactor generator interior types calculation --- src/librustc/query/mod.rs | 5 +++++ src/librustc/traits/select.rs | 10 ++++++---- src/librustc_mir/transform/mod.rs | 10 ++++++++-- 3 files changed, 19 insertions(+), 6 deletions(-) diff --git a/src/librustc/query/mod.rs b/src/librustc/query/mod.rs index 371bab60d301a..1e7ce171ab2e6 100644 --- a/src/librustc/query/mod.rs +++ b/src/librustc/query/mod.rs @@ -108,6 +108,11 @@ rustc_queries! { /// unreachable code. query mir_built(_: DefId) -> &'tcx Steal> {} + /// Compute the generator interior types for a given `DefId` + /// (if it corresponds to a generator), for use in determining + /// generator auto trait implementation + query mir_generator_interior(_: DefId) -> &'tcx Steal> {} + /// Fetch the MIR for a given `DefId` up till the point where it is /// ready for const evaluation. /// diff --git a/src/librustc/traits/select.rs b/src/librustc/traits/select.rs index 1a03acd63f7ef..432d579e14721 100644 --- a/src/librustc/traits/select.rs +++ b/src/librustc/traits/select.rs @@ -2807,10 +2807,12 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> { let mut used_types = Vec::new(); - // Note - while we use `optimized_mir`, the .generator_interior_tys - // field is only set during construction of the original MIR, - // and is unaffected by optimizations - let interior_tys = self.tcx().optimized_mir(did).generator_interior_tys + // We use `mir_validated` since earlier queries (e.g. `mir_const`) + // may be been stolen by the time this code runs. However, `generator_interior_tys` + // is computed early on and never modified, so it's fine to use + // a later query. + let mir = self.tcx().mir_validated(did).0.borrow(); + let interior_tys = mir.generator_interior_tys .as_ref().expect("Missing generator interior types!"); for ty in interior_tys { diff --git a/src/librustc_mir/transform/mod.rs b/src/librustc_mir/transform/mod.rs index ee9555ae8a63b..23fee0717864a 100644 --- a/src/librustc_mir/transform/mod.rs +++ b/src/librustc_mir/transform/mod.rs @@ -46,6 +46,7 @@ pub(crate) fn provide(providers: &mut Providers<'_>) { mir_const, mir_const_qualif, mir_validated, + mir_generator_interior, optimized_mir, is_mir_available, promoted_mir, @@ -98,7 +99,12 @@ fn mir_keys(tcx: TyCtxt<'_>, krate: CrateNum) -> &DefIdSet { } fn mir_built(tcx: TyCtxt<'_>, def_id: DefId) -> &Steal> { - let mut mir = build::mir_build(tcx, def_id); + let mir = build::mir_build(tcx, def_id); + tcx.alloc_steal_mir(mir) +} + +fn mir_generator_interior(tcx: TyCtxt<'_>, def_id: DefId) -> &Steal> { + let mut mir = tcx.mir_const(def_id).steal(); if let ty::Generator(..) = tcx.type_of(def_id).kind { let interior_types = generator::generator_interior_tys(tcx, def_id, &mir); mir.generator_interior_tys = Some(interior_types); @@ -248,7 +254,7 @@ fn mir_validated( // this point, before we steal the mir-const result. let _ = tcx.mir_const_qualif(def_id); - let mut body = tcx.mir_const(def_id).steal(); + let mut body = tcx.mir_generator_interior(def_id).steal(); let promote_pass = promote_consts::PromoteTemps::default(); run_passes(tcx, &mut body, InstanceDef::Item(def_id), None, MirPhase::Validated, &[ // What we need to run borrowck etc.