From 2a1ea44bdc5a8fd1723686020c1bbe7f4da86d1d Mon Sep 17 00:00:00 2001 From: Oliver Schneider Date: Tue, 2 Oct 2018 10:54:34 +0200 Subject: [PATCH] Nest the `impl Trait` existential item inside the return type --- src/librustc/hir/intravisit.rs | 4 + src/librustc/hir/lowering.rs | 48 +--- src/librustc/hir/mod.rs | 6 + src/librustc/hir/print.rs | 1 + src/librustc/ich/impls_hir.rs | 1 + src/librustc/middle/resolve_lifetime.rs | 211 +++++++++--------- src/librustc_typeck/astconv.rs | 9 +- .../ui/impl-trait/deprecated_annotation.rs | 19 ++ .../ui/privacy/private-type-in-interface.rs | 2 - .../privacy/private-type-in-interface.stderr | 16 +- 10 files changed, 144 insertions(+), 173 deletions(-) create mode 100644 src/test/ui/impl-trait/deprecated_annotation.rs diff --git a/src/librustc/hir/intravisit.rs b/src/librustc/hir/intravisit.rs index 76c2ebc0e4a..942f1cbd665 100644 --- a/src/librustc/hir/intravisit.rs +++ b/src/librustc/hir/intravisit.rs @@ -603,6 +603,10 @@ pub fn walk_ty<'v, V: Visitor<'v>>(visitor: &mut V, typ: &'v Ty) { TyKind::Path(ref qpath) => { visitor.visit_qpath(qpath, typ.hir_id, typ.span); } + TyKind::Def(item_id, ref lifetimes) => { + visitor.visit_nested_item(item_id); + walk_list!(visitor, visit_generic_arg, lifetimes); + } TyKind::Array(ref ty, ref length) => { visitor.visit_ty(ty); visitor.visit_anon_const(length) diff --git a/src/librustc/hir/lowering.rs b/src/librustc/hir/lowering.rs index 62b06f54301..40990917959 100644 --- a/src/librustc/hir/lowering.rs +++ b/src/librustc/hir/lowering.rs @@ -1352,20 +1352,7 @@ fn lower_existential_impl_trait( lctx.items.insert(exist_ty_id.node_id, exist_ty_item); // `impl Trait` now just becomes `Foo<'a, 'b, ..>` - 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::TyKind::Path(hir::QPath::Resolved(None, path)) + hir::TyKind::Def(hir::ItemId { id: exist_ty_id.node_id }, lifetimes) }) } @@ -3207,23 +3194,6 @@ fn lower_mod(&mut self, m: &Mod) -> hir::Mod { } } - /// Lowers `impl Trait` items for a function and appends them to the list - fn lower_fn_impl_trait_ids( - &mut self, - decl: &FnDecl, - header: &FnHeader, - ids: &mut SmallVec<[hir::ItemId; 1]>, - ) { - if let Some(id) = header.asyncness.opt_return_id() { - ids.push(hir::ItemId { id }); - } - let mut visitor = ImplTraitTypeIdVisitor { ids }; - match decl.output { - FunctionRetTy::Default(_) => {}, - FunctionRetTy::Ty(ref ty) => visitor.visit_ty(ty), - } - } - fn lower_item_id(&mut self, i: &Item) -> SmallVec<[hir::ItemId; 1]> { match i.node { ItemKind::Use(ref use_tree) => { @@ -3232,20 +3202,8 @@ fn lower_fn_impl_trait_ids( vec } ItemKind::MacroDef(..) => SmallVec::new(), - ItemKind::Fn(ref decl, ref header, ..) => { - let mut ids = smallvec![hir::ItemId { id: i.id }]; - self.lower_fn_impl_trait_ids(decl, header, &mut ids); - ids - }, - ItemKind::Impl(.., None, _, ref items) => { - let mut ids = smallvec![hir::ItemId { id: i.id }]; - for item in items { - if let ImplItemKind::Method(ref sig, _) = item.node { - self.lower_fn_impl_trait_ids(&sig.decl, &sig.header, &mut ids); - } - } - ids - }, + ItemKind::Fn(..) | + ItemKind::Impl(.., None, _, _) => smallvec![hir::ItemId { id: i.id }], ItemKind::Static(ref ty, ..) => { let mut ids = smallvec![hir::ItemId { id: i.id }]; if self.sess.features_untracked().impl_trait_in_bindings { diff --git a/src/librustc/hir/mod.rs b/src/librustc/hir/mod.rs index 088bee38116..eaf4c8c5eca 100644 --- a/src/librustc/hir/mod.rs +++ b/src/librustc/hir/mod.rs @@ -1723,6 +1723,12 @@ pub enum TyKind { /// /// Type parameters may be stored in each `PathSegment`. Path(QPath), + /// A type definition itself. This is currently only used for the `existential type` + /// item that `impl Trait` in return position desugars to. + /// + /// The generic arg list are the lifetimes (and in the future possibly parameters) that are + /// actually bound on the `impl Trait`. + Def(ItemId, HirVec), /// A trait object type `Bound1 + Bound2 + Bound3` /// where `Bound` is a trait or a lifetime. TraitObject(HirVec, Lifetime), diff --git a/src/librustc/hir/print.rs b/src/librustc/hir/print.rs index 69699d2e4ac..08d46793d97 100644 --- a/src/librustc/hir/print.rs +++ b/src/librustc/hir/print.rs @@ -401,6 +401,7 @@ pub fn print_type(&mut self, ty: &hir::Ty) -> io::Result<()> { self.print_ty_fn(f.abi, f.unsafety, &f.decl, None, &f.generic_params, &f.arg_names[..])?; } + hir::TyKind::Def(..) => {}, hir::TyKind::Path(ref qpath) => { self.print_qpath(qpath, false)? } diff --git a/src/librustc/ich/impls_hir.rs b/src/librustc/ich/impls_hir.rs index bc2eb5f442b..528db59bfaa 100644 --- a/src/librustc/ich/impls_hir.rs +++ b/src/librustc/ich/impls_hir.rs @@ -339,6 +339,7 @@ fn hash_stable(&self, Never, Tup(ts), Path(qpath), + Def(it, lt), TraitObject(trait_refs, lifetime), Typeof(body_id), Err, diff --git a/src/librustc/middle/resolve_lifetime.rs b/src/librustc/middle/resolve_lifetime.rs index cda7d8d6b90..55d5e03d687 100644 --- a/src/librustc/middle/resolve_lifetime.rs +++ b/src/librustc/middle/resolve_lifetime.rs @@ -621,140 +621,135 @@ fn visit_ty(&mut self, ty: &'tcx hir::Ty) { }; self.with(scope, |_, this| this.visit_ty(&mt.ty)); } - hir::TyKind::Path(hir::QPath::Resolved(None, ref path)) => { - if let Def::Existential(exist_ty_did) = path.def { - let id = self.tcx.hir.as_local_node_id(exist_ty_did).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(id).node { - // named existential types don't need these hacks - hir::ItemKind::Existential(hir::ExistTy{ impl_trait_fn: None, .. }) => { - intravisit::walk_ty(self, ty); - return; - }, - hir::ItemKind::Existential(hir::ExistTy{ - ref generics, - ref bounds, - .. - }) => ( - generics, - bounds, - ), - ref i => bug!("impl Trait pointed to non-existential type?? {:#?}", i), - }; + hir::TyKind::Def(item_id, ref lifetimes) => { + // 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 { + // named existential types are reached via TyKind::Path + // this arm is for `impl Trait` in the types of statics, constants and locals + hir::ItemKind::Existential(hir::ExistTy{ impl_trait_fn: None, .. }) => { + intravisit::walk_ty(self, ty); + return; + }, + // RPIT (return position impl trait) + hir::ItemKind::Existential(hir::ExistTy{ + ref generics, + ref bounds, + .. + }) => ( + generics, + bounds, + ), + ref i => bug!("impl Trait pointed to non-existential type?? {:#?}", i), + }; + + // 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 { + if let hir::GenericArg::Lifetime(lifetime) = lifetime { + self.visit_lifetime(lifetime); - 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> 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()); - } + // 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()); } } } } + } - // 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); + // 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); - } + 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); } - } - GenericParamKind::Type { .. } => { - type_count += 1; + } else { + lifetimes.insert(name, reg); } } + 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| { - 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 { + 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: self.scope, + s: this.scope, track_lifetime_uses: true, abstract_type_parent: false, }; - self.with(scope, |_old_scope, this| { + this.with(scope, |_old_scope, this| { this.visit_generics(generics); for bound in bounds { this.visit_param_bound(bound); } }); - } + }); } else { - intravisit::walk_ty(self, ty) + 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), diff --git a/src/librustc_typeck/astconv.rs b/src/librustc_typeck/astconv.rs index 00c6ebafec0..28d2ae413de 100644 --- a/src/librustc_typeck/astconv.rs +++ b/src/librustc_typeck/astconv.rs @@ -1338,10 +1338,7 @@ pub fn def_to_ty(&self, match path.def { Def::Existential(did) => { // check for desugared impl trait - if ty::is_impl_trait_defn(tcx, did).is_some() { - let lifetimes = &path.segments[0].args.as_ref().unwrap().args; - return self.impl_trait_ty_to_ty(did, lifetimes); - } + assert!(ty::is_impl_trait_defn(tcx, did).is_none()); let item_segment = path.segments.split_last().unwrap(); self.prohibit_generics(item_segment.1); let substs = self.ast_path_substs_for_ty(span, did, item_segment.0); @@ -1462,6 +1459,10 @@ pub fn ast_ty_to_ty(&self, ast_ty: &hir::Ty) -> Ty<'tcx> { }); self.def_to_ty(opt_self_ty, path, false) } + hir::TyKind::Def(item_id, ref lifetimes) => { + let did = tcx.hir.local_def_id(item_id.id); + self.impl_trait_ty_to_ty(did, lifetimes) + }, hir::TyKind::Path(hir::QPath::TypeRelative(ref qself, ref segment)) => { debug!("ast_ty_to_ty: qself={:?} segment={:?}", qself, segment); let ty = self.ast_ty_to_ty(qself); diff --git a/src/test/ui/impl-trait/deprecated_annotation.rs b/src/test/ui/impl-trait/deprecated_annotation.rs new file mode 100644 index 00000000000..dd9f5fd4647 --- /dev/null +++ b/src/test/ui/impl-trait/deprecated_annotation.rs @@ -0,0 +1,19 @@ +// compile-pass + +#![deny(warnings)] + +#[deprecated] +trait Deprecated {} + +#[deprecated] +struct DeprecatedTy; + +#[allow(deprecated)] +impl Deprecated for DeprecatedTy {} + +#[allow(deprecated)] +fn foo() -> impl Deprecated { DeprecatedTy } + +fn main() { + foo(); +} diff --git a/src/test/ui/privacy/private-type-in-interface.rs b/src/test/ui/privacy/private-type-in-interface.rs index 4235b4be271..1842790a140 100644 --- a/src/test/ui/privacy/private-type-in-interface.rs +++ b/src/test/ui/privacy/private-type-in-interface.rs @@ -35,8 +35,6 @@ impl Tr1 for ext::Alias {} //~ ERROR type `ext::Priv` is private trait Tr2 {} impl Tr2 for u8 {} fn g() -> impl Tr2 { 0 } //~ ERROR type `m::Priv` is private -//~^ ERROR type `m::Priv` is private fn g_ext() -> impl Tr2 { 0 } //~ ERROR type `ext::Priv` is private -//~^ ERROR type `ext::Priv` is private fn main() {} diff --git a/src/test/ui/privacy/private-type-in-interface.stderr b/src/test/ui/privacy/private-type-in-interface.stderr index 5b12ed5e5f4..38c21a77211 100644 --- a/src/test/ui/privacy/private-type-in-interface.stderr +++ b/src/test/ui/privacy/private-type-in-interface.stderr @@ -46,23 +46,11 @@ error: type `m::Priv` is private LL | fn g() -> impl Tr2 { 0 } //~ ERROR type `m::Priv` is private | ^^^^^^^^^^^^^^^^^^ -error: type `m::Priv` is private - --> $DIR/private-type-in-interface.rs:37:16 - | -LL | fn g() -> impl Tr2 { 0 } //~ ERROR type `m::Priv` is private - | ^^^^^^^^^^^^^ - error: type `ext::Priv` is private - --> $DIR/private-type-in-interface.rs:39:15 + --> $DIR/private-type-in-interface.rs:38:15 | LL | fn g_ext() -> impl Tr2 { 0 } //~ ERROR type `ext::Priv` is private | ^^^^^^^^^^^^^^^^^^^^ -error: type `ext::Priv` is private - --> $DIR/private-type-in-interface.rs:39:20 - | -LL | fn g_ext() -> impl Tr2 { 0 } //~ ERROR type `ext::Priv` is private - | ^^^^^^^^^^^^^^^ - -error: aborting due to 11 previous errors +error: aborting due to 9 previous errors -- 2.44.0