diff --git a/src/librustc/middle/stability.rs b/src/librustc/middle/stability.rs index 4cc374431f4..a1691281083 100644 --- a/src/librustc/middle/stability.rs +++ b/src/librustc/middle/stability.rs @@ -12,6 +12,7 @@ //! propagating default levels lexically from parent to children ast nodes. pub use self::StabilityLevel::*; +use self::AnnotationKind::*; use session::Session; use lint; @@ -30,8 +31,8 @@ use syntax::attr::{self, Stability, AttrMetaMethods}; use util::nodemap::{DefIdMap, FnvHashSet, FnvHashMap}; use rustc_front::hir; -use rustc_front::hir::{FnDecl, Block, Crate, Item, Generics, StructField, Variant}; -use rustc_front::visit::{self, FnKind, Visitor}; +use rustc_front::hir::{Block, Crate, Item, Generics, StructField, Variant}; +use rustc_front::visit::{self, Visitor}; use std::mem::replace; use std::cmp::Ordering; @@ -48,6 +49,16 @@ impl StabilityLevel { } } +#[derive(PartialEq)] +enum AnnotationKind { + // Annotation is required if not inherited from unstable parents + AnnRequired, + // Annotation is useless, reject it + AnnProhibited, + // Annotation itself is useless, but it can be propagated to children + AnnContainer, +} + /// A stability index, giving the stability level for items and methods. pub struct Index<'tcx> { /// This is mostly a cache, except the stabilities of local items @@ -64,174 +75,180 @@ struct Annotator<'a, 'tcx: 'a> { index: &'a mut Index<'tcx>, parent: Option<&'tcx Stability>, export_map: &'a PublicItems, + in_trait_impl: bool, + in_enum: bool, } impl<'a, 'tcx: 'a> Annotator<'a, 'tcx> { // Determine the stability for a node based on its attributes and inherited // stability. The stability is recorded in the index and used as the parent. - fn annotate(&mut self, id: NodeId, use_parent: bool, - attrs: &Vec, item_sp: Span, f: F, required: bool) where - F: FnOnce(&mut Annotator), + fn annotate(&mut self, id: NodeId, attrs: &Vec, + item_sp: Span, kind: AnnotationKind, visit_children: F) + where F: FnOnce(&mut Annotator) { if self.index.staged_api[&LOCAL_CRATE] { debug!("annotate(id = {:?}, attrs = {:?})", id, attrs); - match attr::find_stability(self.tcx.sess.diagnostic(), attrs, item_sp) { - Some(mut stab) => { - debug!("annotate: found {:?}", stab); - // if parent is deprecated and we're not, inherit this by merging - // deprecated_since and its reason. - if let Some(parent_stab) = self.parent { - if parent_stab.depr.is_some() - && stab.depr.is_none() { - stab.depr = parent_stab.depr.clone() + if let Some(mut stab) = attr::find_stability(self.tcx.sess.diagnostic(), + attrs, item_sp) { + // Error if prohibited, or can't inherit anything from a container + if kind == AnnProhibited || + kind == AnnContainer && stab.level.is_stable() && stab.depr.is_none() { + self.tcx.sess.span_err(item_sp, "This stability annotation is useless"); + } + + debug!("annotate: found {:?}", stab); + // If parent is deprecated and we're not, inherit this by merging + // deprecated_since and its reason. + if let Some(parent_stab) = self.parent { + if parent_stab.depr.is_some() && stab.depr.is_none() { + stab.depr = parent_stab.depr.clone() + } + } + + let stab = self.tcx.intern_stability(stab); + + // Check if deprecated_since < stable_since. If it is, + // this is *almost surely* an accident. + if let (&Some(attr::Deprecation {since: ref dep_since, ..}), + &attr::Stable {since: ref stab_since}) = (&stab.depr, &stab.level) { + // Explicit version of iter::order::lt to handle parse errors properly + for (dep_v, stab_v) in dep_since.split(".").zip(stab_since.split(".")) { + if let (Ok(dep_v), Ok(stab_v)) = (dep_v.parse::(), stab_v.parse()) { + match dep_v.cmp(&stab_v) { + Ordering::Less => { + self.tcx.sess.span_err(item_sp, "An API can't be stabilized \ + after it is deprecated"); + break + } + Ordering::Equal => continue, + Ordering::Greater => break, + } + } else { + // Act like it isn't less because the question is now nonsensical, + // and this makes us not do anything else interesting. + self.tcx.sess.span_err(item_sp, "Invalid stability or deprecation \ + version found"); + break } } + } - let stab = self.tcx.intern_stability(stab); + let def_id = self.tcx.map.local_def_id(id); + self.index.map.insert(def_id, Some(stab)); - // Check if deprecated_since < stable_since. If it is, - // this is *almost surely* an accident. - let deprecated_predates_stable = match (&stab.depr, &stab.level) { - (&Some(attr::Deprecation {since: ref dep_since, ..}), - &attr::Stable {since: ref stab_since}) => { - // explicit version of iter::order::lt to handle parse errors properly - let mut is_less = false; - for (dep_v, stab_v) in dep_since.split(".").zip(stab_since.split(".")) { - match (dep_v.parse::(), stab_v.parse::()) { - (Ok(dep_v), Ok(stab_v)) => match dep_v.cmp(&stab_v) { - Ordering::Less => { - is_less = true; - break; - } - Ordering::Equal => { continue; } - Ordering::Greater => { break; } - }, - _ => { - self.tcx.sess.span_err(item_sp, - "Invalid stability or deprecation version found"); - // act like it isn't less because the question is now - // nonsensical, and this makes us not do anything else - // interesting. - break; - } - } - } - is_less - }, - _ => false, - }; - - if deprecated_predates_stable { - self.tcx.sess.span_err(item_sp, - "An API can't be stabilized after it is deprecated"); - } - - let def_id = self.tcx.map.local_def_id(id); - self.index.map.insert(def_id, Some(stab)); - - // Don't inherit #[stable(feature = "rust1", since = "1.0.0")] - if !stab.level.is_stable() { - let parent = replace(&mut self.parent, Some(stab)); - f(self); - self.parent = parent; - } else { - f(self); + let parent = replace(&mut self.parent, Some(stab)); + visit_children(self); + self.parent = parent; + } else { + debug!("annotate: not found, parent = {:?}", self.parent); + let mut is_error = kind == AnnRequired && + self.export_map.contains(&id) && + !self.tcx.sess.opts.test; + if let Some(stab) = self.parent { + if stab.level.is_unstable() { + let def_id = self.tcx.map.local_def_id(id); + self.index.map.insert(def_id, Some(stab)); + is_error = false; } } - None => { - debug!("annotate: not found, use_parent = {:?}, parent = {:?}", - use_parent, self.parent); - if use_parent { - if let Some(stab) = self.parent { - let def_id = self.tcx.map.local_def_id(id); - self.index.map.insert(def_id, Some(stab)); - } else if self.index.staged_api[&LOCAL_CRATE] && required - && self.export_map.contains(&id) - && !self.tcx.sess.opts.test { - self.tcx.sess.span_err(item_sp, - "This node does not \ - have a stability attribute"); - } - } - f(self); + if is_error { + self.tcx.sess.span_err(item_sp, "This node does not have \ + a stability attribute"); } + visit_children(self); } } else { - // Emit warnings for non-staged-api crates. These should be errors. + // Emit errors for non-staged-api crates. for attr in attrs { let tag = attr.name(); if tag == "unstable" || tag == "stable" || tag == "deprecated" { attr::mark_used(attr); - self.tcx.sess.span_err(attr.span(), - "stability attributes may not be used outside \ - of the standard library"); + self.tcx.sess.span_err(attr.span(), "stability attributes may not be used \ + outside of the standard library"); } } - f(self); + visit_children(self); } } } impl<'a, 'tcx, 'v> Visitor<'v> for Annotator<'a, 'tcx> { fn visit_item(&mut self, i: &Item) { - // FIXME (#18969): the following is a hack around the fact - // that we cannot currently annotate the stability of - // `deriving`. Basically, we do *not* allow stability - // inheritance on trait implementations, so that derived - // implementations appear to be unannotated. This then allows - // derived implementations to be automatically tagged with the - // stability of the trait. This is WRONG, but expedient to get - // libstd stabilized for the 1.0 release. - let use_parent = match i.node { - hir::ItemImpl(_, _, _, Some(_), _, _) => false, - _ => true, - }; - - // In case of a `pub use ;`, we should not error since the stability - // is inherited from the module itself - let required = match i.node { - hir::ItemUse(_) => i.vis != hir::Public, - _ => true - }; - - self.annotate(i.id, use_parent, &i.attrs, i.span, - |v| visit::walk_item(v, i), required); - - if let hir::ItemStruct(ref sd, _) = i.node { - if !sd.is_struct() { - self.annotate(sd.id(), true, &i.attrs, i.span, |_| {}, true) + let orig_in_trait_impl = self.in_trait_impl; + let orig_in_enum = self.in_enum; + let mut kind = AnnRequired; + match i.node { + // Inherent impls and foreign modules serve only as containers for other items, + // they don't have their own stability. They still can be annotated as unstable + // and propagate this unstability to children, but this annotation is completely + // optional. They inherit stability from their parents when unannotated. + hir::ItemImpl(_, _, _, None, _, _) | hir::ItemForeignMod(..) => { + self.in_trait_impl = false; + kind = AnnContainer; } + hir::ItemImpl(_, _, _, Some(_), _, _) => { + self.in_trait_impl = true; + } + hir::ItemStruct(ref sd, _) => { + self.in_enum = false; + if !sd.is_struct() { + self.annotate(sd.id(), &i.attrs, i.span, AnnRequired, |_| {}) + } + } + hir::ItemEnum(..) => { + self.in_enum = true; + } + _ => {} } - } - fn visit_fn(&mut self, _: FnKind<'v>, _: &'v FnDecl, - _: &'v Block, _: Span, _: NodeId) { - // Items defined in a function body have no reason to have - // a stability attribute, so we don't recurse. + self.annotate(i.id, &i.attrs, i.span, kind, |v| { + visit::walk_item(v, i) + }); + self.in_trait_impl = orig_in_trait_impl; + self.in_enum = orig_in_enum; } fn visit_trait_item(&mut self, ti: &hir::TraitItem) { - self.annotate(ti.id, true, &ti.attrs, ti.span, - |v| visit::walk_trait_item(v, ti), true); + self.annotate(ti.id, &ti.attrs, ti.span, AnnRequired, |v| { + visit::walk_trait_item(v, ti); + }); } fn visit_impl_item(&mut self, ii: &hir::ImplItem) { - self.annotate(ii.id, true, &ii.attrs, ii.span, - |v| visit::walk_impl_item(v, ii), false); + let kind = if self.in_trait_impl { AnnProhibited } else { AnnRequired }; + self.annotate(ii.id, &ii.attrs, ii.span, kind, |v| { + visit::walk_impl_item(v, ii); + }); } fn visit_variant(&mut self, var: &Variant, g: &'v Generics, item_id: NodeId) { - self.annotate(var.node.data.id(), true, &var.node.attrs, var.span, - |v| visit::walk_variant(v, var, g, item_id), true) + self.annotate(var.node.data.id(), &var.node.attrs, var.span, AnnRequired, |v| { + visit::walk_variant(v, var, g, item_id); + }) } fn visit_struct_field(&mut self, s: &StructField) { - self.annotate(s.node.id, true, &s.node.attrs, s.span, - |v| visit::walk_struct_field(v, s), !s.node.kind.is_unnamed()); + // FIXME: This is temporary, can't use attributes with tuple variant fields until snapshot + let kind = if self.in_enum && s.node.kind.is_unnamed() { + AnnProhibited + } else { + AnnRequired + }; + self.annotate(s.node.id, &s.node.attrs, s.span, kind, |v| { + visit::walk_struct_field(v, s); + }); } fn visit_foreign_item(&mut self, i: &hir::ForeignItem) { - self.annotate(i.id, true, &i.attrs, i.span, |_| {}, true); + self.annotate(i.id, &i.attrs, i.span, AnnRequired, |v| { + visit::walk_foreign_item(v, i); + }); + } + + fn visit_macro_def(&mut self, md: &'v hir::MacroDef) { + if md.imported_from.is_none() { + self.annotate(md.id, &md.attrs, md.span, AnnRequired, |_| {}); + } } } @@ -243,21 +260,21 @@ impl<'tcx> Index<'tcx> { index: self, parent: None, export_map: export_map, + in_trait_impl: false, + in_enum: false, }; - annotator.annotate(ast::CRATE_NODE_ID, true, &krate.attrs, krate.span, - |v| visit::walk_crate(v, krate), true); + annotator.annotate(ast::CRATE_NODE_ID, &krate.attrs, krate.span, AnnRequired, + |v| visit::walk_crate(v, krate)); } pub fn new(krate: &Crate) -> Index { let mut is_staged_api = false; for attr in &krate.attrs { - if &attr.name()[..] == "staged_api" { - match attr.node.value.node { - ast::MetaWord(_) => { - attr::mark_used(attr); - is_staged_api = true; - } - _ => (/*pass*/) + if attr.name() == "staged_api" { + if let ast::MetaWord(_) = attr.node.value.node { + attr::mark_used(attr); + is_staged_api = true; + break } } } diff --git a/src/librustc_driver/driver.rs b/src/librustc_driver/driver.rs index 2d7f5544402..eb542adef00 100644 --- a/src/librustc_driver/driver.rs +++ b/src/librustc_driver/driver.rs @@ -739,7 +739,6 @@ pub fn phase_3_run_analysis_passes<'tcx, F, R>(sess: &'tcx Session, lang_items, stability::Index::new(krate), |tcx| { - // passes are timed inside typeck typeck::check_crate(tcx, trait_map); @@ -756,7 +755,7 @@ pub fn phase_3_run_analysis_passes<'tcx, F, R>(sess: &'tcx Session, // Do not move this check past lint time(time_passes, "stability index", || { - tcx.stability.borrow_mut().build(tcx, krate, &public_items) + tcx.stability.borrow_mut().build(tcx, krate, &exported_items) }); time(time_passes, diff --git a/src/librustc_privacy/lib.rs b/src/librustc_privacy/lib.rs index ab9b32383b2..09503bec0c3 100644 --- a/src/librustc_privacy/lib.rs +++ b/src/librustc_privacy/lib.rs @@ -221,9 +221,12 @@ impl<'a, 'tcx, 'v> Visitor<'v> for EmbargoVisitor<'a, 'tcx> { let orig_all_exported = self.prev_exported; match item.node { // impls/extern blocks do not break the "public chain" because they - // cannot have visibility qualifiers on them anyway. They are also not + // cannot have visibility qualifiers on them anyway. Impls are also not // added to public/exported sets based on inherited publicity. - hir::ItemImpl(..) | hir::ItemDefaultImpl(..) | hir::ItemForeignMod(..) => {} + hir::ItemImpl(..) | hir::ItemDefaultImpl(..) => {} + hir::ItemForeignMod(..) => { + self.maybe_insert_id(item.id); + } // Private by default, hence we only retain the "public chain" if // `pub` is explicitly listed. @@ -249,12 +252,17 @@ impl<'a, 'tcx, 'v> Visitor<'v> for EmbargoVisitor<'a, 'tcx> { } } - // Public items in inherent impls for public/exported types are public/exported - // Inherent impls themselves are not public/exported, they are nothing more than - // containers for other items + // Inherent impls for public/exported types and their public items are public/exported hir::ItemImpl(_, _, _, None, ref ty, ref impl_items) => { let (public_ty, exported_ty) = self.is_public_exported_ty(&ty); + if public_ty { + self.public_items.insert(item.id); + } + if exported_ty { + self.exported_items.insert(item.id); + } + for impl_item in impl_items { if impl_item.vis == hir::Public { if public_ty { @@ -1512,6 +1520,8 @@ pub fn check_crate(tcx: &ty::ctxt, prev_exported: true, prev_public: true, }; + visitor.exported_items.insert(ast::CRATE_NODE_ID); + visitor.public_items.insert(ast::CRATE_NODE_ID); loop { let before = (visitor.exported_items.len(), visitor.public_items.len()); visit::walk_crate(&mut visitor, krate);