From 6e5b9c1472175c090700eaae71a7033bc33cd253 Mon Sep 17 00:00:00 2001 From: Oliver Schneider Date: Fri, 29 Jun 2018 10:58:17 +0200 Subject: [PATCH] Get rid of `TyImplTraitExistential` --- src/librustc/hir/intravisit.rs | 7 - src/librustc/hir/lowering.rs | 29 ++-- src/librustc/hir/mod.rs | 12 -- src/librustc/hir/print.rs | 9 -- src/librustc/ich/impls_hir.rs | 1 - src/librustc/middle/resolve_lifetime.rs | 189 +++++++++++++----------- src/librustc_privacy/lib.rs | 44 ++---- src/librustc_typeck/astconv.rs | 16 +- src/librustc_typeck/collect.rs | 4 - 9 files changed, 143 insertions(+), 168 deletions(-) diff --git a/src/librustc/hir/intravisit.rs b/src/librustc/hir/intravisit.rs index 60e944e5aff..a7ed854d016 100644 --- a/src/librustc/hir/intravisit.rs +++ b/src/librustc/hir/intravisit.rs @@ -607,13 +607,6 @@ pub fn walk_ty<'v, V: Visitor<'v>>(visitor: &mut V, typ: &'v Ty) { } visitor.visit_lifetime(lifetime); } - TyImplTraitExistential(_, def_id, ref lifetimes) => { - // we are not recursing into the `existential` item, because it is already being visited - // as part of the surrounding module. The `NodeId` just exists so we don't have to look - // it up everywhere else in the compiler - visitor.visit_def_mention(Def::Existential(def_id)); - walk_list!(visitor, visit_lifetime, lifetimes); - } TyTypeof(ref expression) => { visitor.visit_anon_const(expression) } diff --git a/src/librustc/hir/lowering.rs b/src/librustc/hir/lowering.rs index e59e50ae9e6..5990340ae29 100644 --- a/src/librustc/hir/lowering.rs +++ b/src/librustc/hir/lowering.rs @@ -1306,13 +1306,20 @@ impl<'a> LoweringContext<'a> { lctx.items.insert(exist_ty_id.node_id, exist_ty_item); // `impl Trait` now just becomes `Foo<'a, 'b, ..>` - hir::TyImplTraitExistential( - hir::ItemId { - id: exist_ty_id.node_id - }, - DefId::local(exist_ty_def_index), - lifetimes, - ) + let path = P(hir::Path { + span: exist_ty_span, + def: Def::Existential(DefId::local(exist_ty_def_index)), + segments: hir_vec![hir::PathSegment { + infer_types: false, + ident: Ident::new(keywords::Invalid.name(), exist_ty_span), + args: Some(P(hir::GenericArgs { + parenthesized: false, + bindings: HirVec::new(), + args: lifetimes, + })) + }], + }); + hir::TyPath(hir::QPath::Resolved(None, path)) }) } @@ -1321,7 +1328,7 @@ impl<'a> LoweringContext<'a> { exist_ty_id: NodeId, parent_index: DefIndex, bounds: &hir::GenericBounds, - ) -> (HirVec, HirVec) { + ) -> (HirVec, HirVec) { // This visitor walks over impl trait bounds and creates defs for all lifetimes which // appear in the bounds, excluding lifetimes that are created within the bounds. // e.g. 'a, 'b, but not 'c in `impl for<'c> SomeTrait<'a, 'b, 'c>` @@ -1332,7 +1339,7 @@ impl<'a> LoweringContext<'a> { collect_elided_lifetimes: bool, currently_bound_lifetimes: Vec, already_defined_lifetimes: HashSet, - output_lifetimes: Vec, + output_lifetimes: Vec, output_lifetime_params: Vec, } @@ -1416,11 +1423,11 @@ impl<'a> LoweringContext<'a> { && !self.already_defined_lifetimes.contains(&name) { self.already_defined_lifetimes.insert(name); - self.output_lifetimes.push(hir::Lifetime { + self.output_lifetimes.push(hir::GenericArg::Lifetime(hir::Lifetime { id: self.context.next_id().node_id, span: lifetime.span, name, - }); + })); // We need to manually create the ids here, because the // definitions will go into the explicit `existential type` diff --git a/src/librustc/hir/mod.rs b/src/librustc/hir/mod.rs index b0b81316148..8d83dd3279c 100644 --- a/src/librustc/hir/mod.rs +++ b/src/librustc/hir/mod.rs @@ -1692,18 +1692,6 @@ pub enum Ty_ { /// A trait object type `Bound1 + Bound2 + Bound3` /// where `Bound` is a trait or a lifetime. TyTraitObject(HirVec, Lifetime), - /// An existentially quantified (there exists a type satisfying) `impl - /// Bound1 + Bound2 + Bound3` type where `Bound` is a trait or a lifetime. - /// - /// The `Item` is the generated - /// `existential type Foo<'a, 'b>: MyTrait<'a, 'b>;`. - /// - /// The `HirVec` is the list of lifetimes applied as parameters - /// to the `abstract type`, e.g. the `'c` and `'d` in `-> Foo<'c, 'd>`. - /// This list is only a list of lifetimes and not type parameters - /// because all in-scope type parameters are captured by `impl Trait`, - /// so they are resolved directly through the parent `Generics`. - TyImplTraitExistential(ItemId, DefId, HirVec), /// Unused for now TyTypeof(AnonConst), /// TyInfer means the type should be inferred instead of it having been diff --git a/src/librustc/hir/print.rs b/src/librustc/hir/print.rs index 255009c94c6..4d0969d898e 100644 --- a/src/librustc/hir/print.rs +++ b/src/librustc/hir/print.rs @@ -420,15 +420,6 @@ impl<'a> State<'a> { self.print_lifetime(lifetime)?; } } - hir::TyImplTraitExistential(hir_id, _def_id, ref _lifetimes) => { - match self.ann.try_fetch_item(hir_id.id).map(|it| &it.node) { - None => self.word_space("impl {{Trait}}")?, - Some(&hir::ItemExistential(ref exist_ty)) => { - self.print_bounds("impl", &exist_ty.bounds)?; - }, - other => bug!("impl Trait pointed to {:#?}", other), - } - } hir::TyArray(ref ty, ref length) => { self.s.word("[")?; self.print_type(&ty)?; diff --git a/src/librustc/ich/impls_hir.rs b/src/librustc/ich/impls_hir.rs index 0c7baea85ad..dd19d59578f 100644 --- a/src/librustc/ich/impls_hir.rs +++ b/src/librustc/ich/impls_hir.rs @@ -340,7 +340,6 @@ impl_stable_hash_for!(enum hir::Ty_ { TyTup(ts), TyPath(qpath), TyTraitObject(trait_refs, lifetime), - TyImplTraitExistential(existty, def_id, lifetimes), TyTypeof(body_id), TyErr, TyInfer diff --git a/src/librustc/middle/resolve_lifetime.rs b/src/librustc/middle/resolve_lifetime.rs index ed2b9c50689..369f65c214a 100644 --- a/src/librustc/middle/resolve_lifetime.rs +++ b/src/librustc/middle/resolve_lifetime.rs @@ -625,122 +625,131 @@ impl<'a, 'tcx> Visitor<'tcx> for LifetimeContext<'a, 'tcx> { }; self.with(scope, |_, this| this.visit_ty(&mt.ty)); } - hir::TyImplTraitExistential(item_id, _, ref lifetimes) => { - // Resolve the lifetimes that are applied to the existential type. - // These are resolved in the current scope. - // `fn foo<'a>() -> impl MyTrait<'a> { ... }` desugars to - // `fn foo<'a>() -> MyAnonTy<'a> { ... }` - // ^ ^this gets resolved in the current scope - for lifetime in lifetimes { - self.visit_lifetime(lifetime); + hir::TyPath(hir::QPath::Resolved(None, ref path)) => { + if let Def::Existential(exist_ty_did) = path.def { + assert!(exist_ty_did.is_local()); + // Resolve the lifetimes that are applied to the existential type. + // These are resolved in the current scope. + // `fn foo<'a>() -> impl MyTrait<'a> { ... }` desugars to + // `fn foo<'a>() -> MyAnonTy<'a> { ... }` + // ^ ^this gets resolved in the current scope + for lifetime in &path.segments[0].args.as_ref().unwrap().args { + if let hir::GenericArg::Lifetime(lifetime) = lifetime { + self.visit_lifetime(lifetime); - // Check for predicates like `impl for<'a> SomeTrait>` - // and ban them. Type variables instantiated inside binders aren't - // well-supported at the moment, so this doesn't work. - // In the future, this should be fixed and this error should be removed. - let def = self.map.defs.get(&lifetime.id).cloned(); - if let Some(Region::LateBound(_, def_id, _)) = def { - if let Some(node_id) = self.tcx.hir.as_local_node_id(def_id) { - // Ensure that the parent of the def is an item, not HRTB - let parent_id = self.tcx.hir.get_parent_node(node_id); - let parent_impl_id = hir::ImplItemId { node_id: parent_id }; - let parent_trait_id = hir::TraitItemId { node_id: parent_id }; - let krate = self.tcx.hir.forest.krate(); - if !(krate.items.contains_key(&parent_id) - || krate.impl_items.contains_key(&parent_impl_id) - || krate.trait_items.contains_key(&parent_trait_id)) - { - span_err!( - self.tcx.sess, - lifetime.span, - E0657, - "`impl Trait` can only capture lifetimes \ - bound at the fn or impl level" - ); - self.uninsert_lifetime_on_error(lifetime, def.unwrap()); + // Check for predicates like `impl for<'a> Trait>` + // and ban them. Type variables instantiated inside binders aren't + // well-supported at the moment, so this doesn't work. + // In the future, this should be fixed and this error should be removed. + let def = self.map.defs.get(&lifetime.id).cloned(); + if let Some(Region::LateBound(_, def_id, _)) = def { + if let Some(node_id) = self.tcx.hir.as_local_node_id(def_id) { + // Ensure that the parent of the def is an item, not HRTB + let parent_id = self.tcx.hir.get_parent_node(node_id); + let parent_impl_id = hir::ImplItemId { node_id: parent_id }; + let parent_trait_id = hir::TraitItemId { node_id: parent_id }; + let krate = self.tcx.hir.forest.krate(); + if !(krate.items.contains_key(&parent_id) + || krate.impl_items.contains_key(&parent_impl_id) + || krate.trait_items.contains_key(&parent_trait_id)) + { + span_err!( + self.tcx.sess, + lifetime.span, + E0657, + "`impl Trait` can only capture lifetimes \ + bound at the fn or impl level" + ); + self.uninsert_lifetime_on_error(lifetime, def.unwrap()); + } + } } } } - } - // Resolve the lifetimes in the bounds to the lifetime defs in the generics. - // `fn foo<'a>() -> impl MyTrait<'a> { ... }` desugars to - // `abstract type MyAnonTy<'b>: MyTrait<'b>;` - // ^ ^ this gets resolved in the scope of - // the exist_ty generics - let (generics, bounds) = match self.tcx.hir.expect_item(item_id.id).node { - hir::ItemExistential(hir::ExistTy{ ref generics, ref bounds, .. }) => ( - generics, - bounds, - ), - ref i => bug!("impl Trait pointed to non-existential type?? {:#?}", i), - }; + let id = self.tcx.hir.as_local_node_id(exist_ty_did).unwrap(); - // We want to start our early-bound indices at the end of the parent scope, - // not including any parent `impl Trait`s. - let mut index = self.next_early_index_for_abstract_type(); - debug!("visit_ty: index = {}", index); + // Resolve the lifetimes in the bounds to the lifetime defs in the generics. + // `fn foo<'a>() -> impl MyTrait<'a> { ... }` desugars to + // `abstract type MyAnonTy<'b>: MyTrait<'b>;` + // ^ ^ this gets resolved in the scope of + // the exist_ty generics + let (generics, bounds) = match self.tcx.hir.expect_item(id).node { + hir::ItemExistential(hir::ExistTy{ ref generics, ref bounds, .. }) => ( + generics, + bounds, + ), + ref i => bug!("impl Trait pointed to non-existential type?? {:#?}", i), + }; - let mut elision = None; - let mut lifetimes = FxHashMap(); - let mut type_count = 0; - for param in &generics.params { - match param.kind { - GenericParamKind::Lifetime { .. } => { - let (name, reg) = Region::early(&self.tcx.hir, &mut index, ¶m); - if let hir::ParamName::Plain(param_name) = name { - if param_name.name == keywords::UnderscoreLifetime.name() { - // Pick the elided lifetime "definition" if one exists - // and use it to make an elision scope. - elision = Some(reg); + // We want to start our early-bound indices at the end of the parent scope, + // not including any parent `impl Trait`s. + let mut index = self.next_early_index_for_abstract_type(); + debug!("visit_ty: index = {}", index); + + let mut elision = None; + let mut lifetimes = FxHashMap(); + let mut type_count = 0; + for param in &generics.params { + match param.kind { + GenericParamKind::Lifetime { .. } => { + let (name, reg) = Region::early(&self.tcx.hir, &mut index, ¶m); + if let hir::ParamName::Plain(param_name) = name { + if param_name.name == keywords::UnderscoreLifetime.name() { + // Pick the elided lifetime "definition" if one exists + // and use it to make an elision scope. + elision = Some(reg); + } else { + lifetimes.insert(name, reg); + } } else { lifetimes.insert(name, reg); } - } else { - lifetimes.insert(name, reg); + } + GenericParamKind::Type { .. } => { + type_count += 1; } } - GenericParamKind::Type { .. } => { - type_count += 1; - } } - } - let next_early_index = index + type_count; + let next_early_index = index + type_count; - if let Some(elision_region) = elision { - let scope = Scope::Elision { - elide: Elide::Exact(elision_region), - s: self.scope, - }; - self.with(scope, |_old_scope, this| { + if let Some(elision_region) = elision { + let scope = Scope::Elision { + elide: Elide::Exact(elision_region), + s: self.scope, + }; + self.with(scope, |_old_scope, this| { + let scope = Scope::Binder { + lifetimes, + next_early_index, + s: this.scope, + track_lifetime_uses: true, + abstract_type_parent: false, + }; + this.with(scope, |_old_scope, this| { + this.visit_generics(generics); + for bound in bounds { + this.visit_param_bound(bound); + } + }); + }); + } else { let scope = Scope::Binder { lifetimes, next_early_index, - s: this.scope, + s: self.scope, track_lifetime_uses: true, abstract_type_parent: false, }; - this.with(scope, |_old_scope, this| { + self.with(scope, |_old_scope, this| { this.visit_generics(generics); for bound in bounds { this.visit_param_bound(bound); } }); - }); + } } else { - let scope = Scope::Binder { - lifetimes, - next_early_index, - s: self.scope, - track_lifetime_uses: true, - abstract_type_parent: false, - }; - self.with(scope, |_old_scope, this| { - this.visit_generics(generics); - for bound in bounds { - this.visit_param_bound(bound); - } - }); + intravisit::walk_ty(self, ty) } } _ => intravisit::walk_ty(self, ty), diff --git a/src/librustc_privacy/lib.rs b/src/librustc_privacy/lib.rs index c55d57cb916..3919ba13076 100644 --- a/src/librustc_privacy/lib.rs +++ b/src/librustc_privacy/lib.rs @@ -229,8 +229,12 @@ impl<'a, 'tcx> Visitor<'tcx> for EmbargoVisitor<'a, 'tcx> { hir::ItemUse(..) => {} // The interface is empty hir::ItemGlobalAsm(..) => {} - // Checked by visit_ty - hir::ItemExistential(..) => {} + hir::ItemExistential(..) => { + if item_level.is_some() { + // Reach the (potentially private) type and the API being exposed + self.reach(item.id).ty().predicates(); + } + } // Visit everything hir::ItemConst(..) | hir::ItemStatic(..) | hir::ItemFn(..) | hir::ItemTy(..) => { @@ -390,17 +394,6 @@ impl<'a, 'tcx> Visitor<'tcx> for EmbargoVisitor<'a, 'tcx> { module_id = self.tcx.hir.get_parent_node(module_id); } } - - fn visit_ty(&mut self, ty: &'tcx hir::Ty) { - if let hir::TyImplTraitExistential(item_id, _, _) = ty.node { - if self.get(item_id.id).is_some() { - // Reach the (potentially private) type and the API being exposed - self.reach(item_id.id).ty().predicates(); - } - } - - intravisit::walk_ty(self, ty); - } } impl<'b, 'a, 'tcx> ReachEverythingInTheInterfaceVisitor<'b, 'a, 'tcx> { @@ -1568,8 +1561,15 @@ impl<'a, 'tcx> Visitor<'tcx> for PrivateItemsInPublicInterfacesVisitor<'a, 'tcx> hir::ItemUse(..) => {} // No subitems hir::ItemGlobalAsm(..) => {} - // Checked in visit_ty - hir::ItemExistential(..) => {} + hir::ItemExistential(..) => { + // Check the traits being exposed, as they're separate, + // e.g. `impl Iterator` has two predicates, + // `X: Iterator` and `::Item == T`, + // where `X` is the `impl Iterator` itself, + // stored in `predicates_of`, not in the `Ty` itself. + + self.check(item.id, self.inner_visibility).predicates(); + } // Subitems of these items have inherited publicity hir::ItemConst(..) | hir::ItemStatic(..) | hir::ItemFn(..) | hir::ItemTy(..) => { @@ -1667,20 +1667,6 @@ impl<'a, 'tcx> Visitor<'tcx> for PrivateItemsInPublicInterfacesVisitor<'a, 'tcx> // handled in `visit_item` above } - fn visit_ty(&mut self, ty: &'tcx hir::Ty) { - if let hir::TyImplTraitExistential(ref exist_item, _, _) = ty.node { - // Check the traits being exposed, as they're separate, - // e.g. `impl Iterator` has two predicates, - // `X: Iterator` and `::Item == T`, - // where `X` is the `impl Iterator` itself, - // stored in `predicates_of`, not in the `Ty` itself. - - self.check(exist_item.id, self.inner_visibility).predicates(); - } - - intravisit::walk_ty(self, ty); - } - // Don't recurse into expressions in array sizes or const initializers fn visit_expr(&mut self, _: &'tcx hir::Expr) {} // Don't recurse into patterns in function arguments diff --git a/src/librustc_typeck/astconv.rs b/src/librustc_typeck/astconv.rs index 762dc5d26f5..2e467d315be 100644 --- a/src/librustc_typeck/astconv.rs +++ b/src/librustc_typeck/astconv.rs @@ -1095,6 +1095,11 @@ impl<'o, 'gcx: 'tcx, 'tcx> AstConv<'gcx, 'tcx>+'o { hir::TyStr => tcx.mk_str() } } + Def::Existential(exist_ty_did) => { + assert!(exist_ty_did.is_local()); + let lifetimes = &path.segments[0].args.as_ref().unwrap().args; + self.impl_trait_ty_to_ty(exist_ty_did, lifetimes) + } Def::Err => { self.set_tainted_by_errors(); return self.tcx().types.err; @@ -1140,9 +1145,6 @@ impl<'o, 'gcx: 'tcx, 'tcx> AstConv<'gcx, 'tcx>+'o { hir::TyTraitObject(ref bounds, ref lifetime) => { self.conv_object_ty_poly_trait_ref(ast_ty.span, bounds, lifetime) } - hir::TyImplTraitExistential(_, def_id, ref lifetimes) => { - self.impl_trait_ty_to_ty(def_id, lifetimes) - } hir::TyPath(hir::QPath::Resolved(ref maybe_qself, ref path)) => { debug!("ast_ty_to_ty: maybe_qself={:?} path={:?}", maybe_qself, path); let opt_self_ty = maybe_qself.as_ref().map(|qself| { @@ -1195,7 +1197,7 @@ impl<'o, 'gcx: 'tcx, 'tcx> AstConv<'gcx, 'tcx>+'o { pub fn impl_trait_ty_to_ty( &self, def_id: DefId, - lifetimes: &[hir::Lifetime], + lifetimes: &[hir::GenericArg], ) -> Ty<'tcx> { debug!("impl_trait_ty_to_ty(def_id={:?}, lifetimes={:?})", def_id, lifetimes); let tcx = self.tcx(); @@ -1208,7 +1210,11 @@ impl<'o, 'gcx: 'tcx, 'tcx> AstConv<'gcx, 'tcx>+'o { // Our own parameters are the resolved lifetimes. match param.kind { GenericParamDefKind::Lifetime => { - self.ast_region_to_region(&lifetimes[i], None).into() + if let hir::GenericArg::Lifetime(lifetime) = &lifetimes[i] { + self.ast_region_to_region(lifetime, None).into() + } else { + bug!() + } } _ => bug!() } diff --git a/src/librustc_typeck/collect.rs b/src/librustc_typeck/collect.rs index 852603ac51c..4931cbfa5ac 100644 --- a/src/librustc_typeck/collect.rs +++ b/src/librustc_typeck/collect.rs @@ -875,10 +875,6 @@ fn generics_of<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, } } - NodeTy(&hir::Ty { node: hir::TyImplTraitExistential(..), .. }) => { - bug!("impl Trait is desugared to existential type items"); - } - _ => &no_generics, };