rollup merge of #21910: Manishearth/missing_stability

Currently, if a `#![staged_api]` crate contains an exported item without a stability marker (or inherited stability),
the item is useless.

This change introduces a check to ensure that all exported items have a defined stability.

it also introduces the `unmarked_api` feature, which lets users import unmarked features. While this PR should in theory forbid these from existing,
in practice we can't be so sure; so this lets users bypass this check instead of having to wait for the library and/or compiler to be fixed (since otherwise this is a hard error).

r? @aturon

Author: alexcrichton

src/doc/reference.md

@@ -2432,6 +2432,8 @@ The currently implemented features of the reference compiler are:
 * `simd` - Allows use of the `#[simd]` attribute, which is overly simple and
            not the SIMD interface we want to expose in the long term.
 
+* `staged_api` - Allows usage of stability markers and `#![staged_api]` in a crate
+
 * `struct_inherit` - Allows using struct inheritance, which is barely
                      implemented and will probably be removed. Don't use this.
 
@@ -2459,6 +2461,11 @@ The currently implemented features of the reference compiler are:
                         which is considered wildly unsafe and will be
                         obsoleted by language improvements.
 
+* `unmarked_api` - Allows use of items within a `#![staged_api]` crate
+                   which have not been marked with a stability marker.
+                   Such items should not be allowed by the compiler to exist,
+                   so if you need this there probably is a compiler bug.
+
 * `associated_types` - Allows type aliases in traits. Experimental.
 
 If a feature is promoted to a language feature, then all existing programs will

src/librustc/middle/stability.rs

@@ -14,6 +14,7 @@
 use session::Session;
 use lint;
 use middle::ty;
+use middle::privacy::PublicItems;
 use metadata::csearch;
 use syntax::parse::token::InternedString;
 use syntax::codemap::{Span, DUMMY_SP};
@@ -44,15 +45,16 @@ pub struct Index {
 // A private tree-walker for producing an Index.
 struct Annotator<'a> {
     sess: &'a Session,
-    index: Index,
-    parent: Option<Stability>
+    index: &'a mut Index,
+    parent: Option<Stability>,
+    export_map: &'a PublicItems,
 }
 
 impl<'a> Annotator<'a> {
     // 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<F>(&mut self, id: NodeId, use_parent: bool,
-                   attrs: &Vec<Attribute>, item_sp: Span, f: F) where
+                   attrs: &Vec<Attribute>, item_sp: Span, f: F, required: bool) where
         F: FnOnce(&mut Annotator),
     {
         match attr::find_stability(self.sess.diagnostic(), attrs.as_slice(), item_sp) {
@@ -70,7 +72,14 @@ impl<'a> Annotator<'a> {
             }
             None => {
                 if use_parent {
-                    self.parent.clone().map(|stab| self.index.local.insert(id, stab));
+                    if let Some(stab) = self.parent.clone() {
+                        self.index.local.insert(id, stab);
+                    } else if self.index.staged_api && required
+                           && self.export_map.contains(&id)
+                           && !self.sess.opts.test {
+                        self.sess.span_err(item_sp,
+                                           "This node does not have a stability attribute");
+                    }
                 }
                 f(self);
             }
@@ -93,11 +102,19 @@ impl<'a, 'v> Visitor<'v> for Annotator<'a> {
             _ => true,
         };
 
-        self.annotate(i.id, use_parent, &i.attrs, i.span, |v| visit::walk_item(v, i));
+        // In case of a `pub use <mod>;`, we should not error since the stability
+        // is inherited from the module itself
+        let required = match i.node {
+            ast::ItemUse(_) => i.vis != ast::Public,
+            _ => true
+        };
+
+        self.annotate(i.id, use_parent, &i.attrs, i.span,
+                      |v| visit::walk_item(v, i), required);
 
         if let ast::ItemStruct(ref sd, _) = i.node {
             sd.ctor_id.map(|id| {
-                self.annotate(id, true, &i.attrs, i.span, |_| {})
+                self.annotate(id, true, &i.attrs, i.span, |_| {}, true)
             });
         }
     }
@@ -106,7 +123,7 @@ impl<'a, 'v> Visitor<'v> for Annotator<'a> {
                 _: &'v Block, sp: Span, _: NodeId) {
         if let FkMethod(_, _, meth) = fk {
             // Methods are not already annotated, so we annotate it
-            self.annotate(meth.id, true, &meth.attrs, sp, |_| {});
+            self.annotate(meth.id, true, &meth.attrs, sp, |_| {}, true);
         }
         // Items defined in a function body have no reason to have
         // a stability attribute, so we don't recurse.
@@ -126,27 +143,41 @@ impl<'a, 'v> Visitor<'v> for Annotator<'a> {
             TypeTraitItem(ref typedef) => (typedef.ty_param.id, &typedef.attrs,
                                            typedef.ty_param.span),
         };
-        self.annotate(id, true, attrs, sp, |v| visit::walk_trait_item(v, t));
+        self.annotate(id, true, attrs, sp, |v| visit::walk_trait_item(v, t), true);
     }
 
     fn visit_variant(&mut self, var: &Variant, g: &'v Generics) {
         self.annotate(var.node.id, true, &var.node.attrs, var.span,
-                      |v| visit::walk_variant(v, var, g))
+                      |v| visit::walk_variant(v, var, g), true)
     }
 
     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));
+                      |v| visit::walk_struct_field(v, s), true);
     }
 
     fn visit_foreign_item(&mut self, i: &ast::ForeignItem) {
-        self.annotate(i.id, true, &i.attrs, i.span, |_| {});
+        self.annotate(i.id, true, &i.attrs, i.span, |_| {}, true);
     }
 }
 
 impl Index {
     /// Construct the stability index for a crate being compiled.
-    pub fn build(sess: &Session, krate: &Crate) -> Index {
+    pub fn build(&mut self, sess: &Session, krate: &Crate, export_map: &PublicItems) {
+        if !self.staged_api {
+            return;
+        }
+        let mut annotator = Annotator {
+            sess: sess,
+            index: self,
+            parent: None,
+            export_map: export_map,
+        };
+        annotator.annotate(ast::CRATE_NODE_ID, true, &krate.attrs, krate.span,
+                           |v| visit::walk_crate(v, krate), true);
+    }
+
+    pub fn new(krate: &Crate) -> Index {
         let mut staged_api = false;
         for attr in &krate.attrs {
             if attr.name().get() == "staged_api" {
@@ -159,22 +190,11 @@ impl Index {
                 }
             }
         }
-        let index = Index {
+        Index {
             staged_api: staged_api,
             local: NodeMap(),
             extern_cache: DefIdMap()
-        };
-        if !staged_api {
-            return index;
         }
-        let mut annotator = Annotator {
-            sess: sess,
-            index: index,
-            parent: None
-        };
-        annotator.annotate(ast::CRATE_NODE_ID, true, &krate.attrs, krate.span,
-                           |v| visit::walk_crate(v, krate));
-        annotator.index
     }
 }
 
@@ -234,10 +254,19 @@ impl<'a, 'tcx> Checker<'a, 'tcx> {
             None => {
                 // This is an 'unmarked' API, which should not exist
                 // in the standard library.
-                self.tcx.sess.span_err(span, "use of unmarked library feature");
-                self.tcx.sess.span_note(span, "this is either a bug in the library you are \
-                                               using or a bug in the compiler - there is \
-                                               no way to use this feature");
+                if self.tcx.sess.features.borrow().unmarked_api {
+                    self.tcx.sess.span_warn(span, "use of unmarked library feature");
+                    self.tcx.sess.span_note(span, "this is either a bug in the library you are \
+                                                   using and a bug in the compiler - please \
+                                                   report it in both places");
+                } else {
+                    self.tcx.sess.span_err(span, "use of unmarked library feature");
+                    self.tcx.sess.span_note(span, "this is either a bug in the library you are \
+                                                   using and a bug in the compiler - please \
+                                                   report it in both places");
+                    self.tcx.sess.span_note(span, "use #![feature(unmarked_api)] in the \
+                                                   crate attributes to override this");
+                }
             }
         }
     }

src/librustc_driver/driver.rs

@@ -594,9 +594,6 @@ pub fn phase_3_run_analysis_passes<'tcx>(sess: Session,
     time(time_passes, "loop checking", (), |_|
          middle::check_loop::check_crate(&sess, krate));
 
-    let stability_index = time(time_passes, "stability index", (), |_|
-                               stability::Index::build(&sess, krate));
-
     time(time_passes, "static item recursion checking", (), |_|
          middle::check_static_recursion::check_crate(&sess, krate, &def_map, &ast_map));
 
@@ -608,7 +605,7 @@ pub fn phase_3_run_analysis_passes<'tcx>(sess: Session,
                             freevars,
                             region_map,
                             lang_items,
-                            stability_index);
+                            stability::Index::new(krate));
 
     // passes are timed inside typeck
     typeck::check_crate(&ty_cx, trait_map);
@@ -628,6 +625,10 @@ pub fn phase_3_run_analysis_passes<'tcx>(sess: Session,
             time(time_passes, "privacy checking", maps, |(a, b)|
                  rustc_privacy::check_crate(&ty_cx, &export_map, a, b));
 
+    // Do not move this check past lint
+    time(time_passes, "stability index", (), |_|
+         ty_cx.stability.borrow_mut().build(&ty_cx.sess, krate, &public_items));
+
     time(time_passes, "intrinsic checking", (), |_|
          middle::intrinsicck::check_crate(&ty_cx));
 

src/librustc_driver/test.rs

@@ -125,7 +125,6 @@ fn test_env<F>(source_string: &str,
         resolve::resolve_crate(&sess, &ast_map, &lang_items, krate, resolve::MakeGlobMap::No);
     let named_region_map = resolve_lifetime::krate(&sess, krate, &def_map);
     let region_map = region::resolve_crate(&sess, krate);
-    let stability_index = stability::Index::build(&sess, krate);
     let tcx = ty::mk_ctxt(sess,
                           &arenas,
                           def_map,
@@ -134,7 +133,7 @@ fn test_env<F>(source_string: &str,
                           freevars,
                           region_map,
                           lang_items,
-                          stability_index);
+                          stability::Index::new(krate));
     let infcx = infer::new_infer_ctxt(&tcx);
     body(Env { infcx: &infcx });
     infcx.resolve_regions_and_report_errors(ast::CRATE_NODE_ID);

src/libstd/thread_local/mod.rs

@@ -45,6 +45,7 @@ pub mod scoped;
 
 // Sure wish we had macro hygiene, no?
 #[doc(hidden)]
+#[stable(feature = "rust1", since = "1.0.0")]
 pub mod __impl {
     pub use super::imp::Key as KeyInner;
     pub use super::imp::destroy_value;

src/libsyntax/feature_gate.rs

@@ -119,6 +119,9 @@ static KNOWN_FEATURES: &'static [(&'static str, &'static str, Status)] = &[
 
     // Allows use of #[staged_api]
     ("staged_api", "1.0.0", Active),
+
+    // Allows using items which are missing stability attributes
+    ("unmarked_api", "1.0.0", Active)
 ];
 
 enum Status {
@@ -145,6 +148,7 @@ pub struct Features {
     pub quote: bool,
     pub old_orphan_check: bool,
     pub simd_ffi: bool,
+    pub unmarked_api: bool,
     pub lib_features: Vec<(InternedString, Span)>
 }
 
@@ -157,6 +161,7 @@ impl Features {
             quote: false,
             old_orphan_check: false,
             simd_ffi: false,
+            unmarked_api: false,
             lib_features: Vec::new()
         }
     }
@@ -566,6 +571,7 @@ fn check_crate_inner<F>(cm: &CodeMap, span_handler: &SpanHandler, krate: &ast::C
         quote: cx.has_feature("quote"),
         old_orphan_check: cx.has_feature("old_orphan_check"),
         simd_ffi: cx.has_feature("simd_ffi"),
+        unmarked_api: cx.has_feature("unmarked_api"),
         lib_features: unknown_features
     }
 }

src/test/auxiliary/lint_stability.rs

@@ -11,6 +11,7 @@
 #![crate_type = "lib"]
 #![feature(staged_api)]
 #![staged_api]
+#![stable(feature = "lint_stability", since = "1.0.0")]
 
 #[stable(feature = "test_feature", since = "1.0.0")]
 #[deprecated(since = "1.0.0")]
@@ -31,8 +32,6 @@ pub fn unstable() {}
 #[unstable(feature = "test_feature", reason = "text")]
 pub fn unstable_text() {}
 
-pub fn unmarked() {}
-
 #[stable(feature = "rust1", since = "1.0.0")]
 pub fn stable() {}
 #[stable(feature = "rust1", since = "1.0.0", reason = "text")]
@@ -61,8 +60,6 @@ impl MethodTester {
     #[unstable(feature = "test_feature", reason = "text")]
     pub fn method_unstable_text(&self) {}
 
-    pub fn method_unmarked(&self) {}
-
     #[stable(feature = "rust1", since = "1.0.0")]
     pub fn method_stable(&self) {}
     #[stable(feature = "rust1", since = "1.0.0", reason = "text")]
@@ -79,6 +76,7 @@ impl MethodTester {
     pub fn method_frozen_text(&self) {}
 }
 
+#[stable(feature = "test_feature", since = "1.0.0")]
 pub trait Trait {
     #[stable(feature = "test_feature", since = "1.0.0")]
     #[deprecated(since = "1.0.0")]
@@ -99,8 +97,6 @@ pub trait Trait {
     #[unstable(feature = "test_feature", reason = "text")]
     fn trait_unstable_text(&self) {}
 
-    fn trait_unmarked(&self) {}
-
     #[stable(feature = "rust1", since = "1.0.0")]
     fn trait_stable(&self) {}
     #[stable(feature = "rust1", since = "1.0.0", reason = "text")]
@@ -130,7 +126,6 @@ pub struct DeprecatedStruct { pub i: int }
 pub struct DeprecatedUnstableStruct { pub i: int }
 #[unstable(feature = "test_feature")]
 pub struct UnstableStruct { pub i: int }
-pub struct UnmarkedStruct { pub i: int }
 #[stable(feature = "rust1", since = "1.0.0")]
 pub struct StableStruct { pub i: int }
 
@@ -142,10 +137,10 @@ pub struct DeprecatedUnitStruct;
 pub struct DeprecatedUnstableUnitStruct;
 #[unstable(feature = "test_feature")]
 pub struct UnstableUnitStruct;
-pub struct UnmarkedUnitStruct;
 #[stable(feature = "rust1", since = "1.0.0")]
 pub struct StableUnitStruct;
 
+#[stable(feature = "test_feature", since = "1.0.0")]
 pub enum Enum {
     #[stable(feature = "test_feature", since = "1.0.0")]
     #[deprecated(since = "1.0.0")]
@@ -156,7 +151,6 @@ pub enum Enum {
     #[unstable(feature = "test_feature")]
     UnstableVariant,
 
-    UnmarkedVariant,
     #[stable(feature = "rust1", since = "1.0.0")]
     StableVariant,
 }
@@ -169,7 +163,6 @@ pub struct DeprecatedTupleStruct(pub int);
 pub struct DeprecatedUnstableTupleStruct(pub int);
 #[unstable(feature = "test_feature")]
 pub struct UnstableTupleStruct(pub int);
-pub struct UnmarkedTupleStruct(pub int);
 #[stable(feature = "rust1", since = "1.0.0")]
 pub struct StableTupleStruct(pub int);