Skip to content

Commit

Permalink
Add stability inheritance
Browse files Browse the repository at this point in the history
This commit makes several changes to the stability index infrastructure:

* Stability levels are now inherited lexically, i.e., each item's
  stability level becomes the default for any nested items.

* The computed stability level for an item is stored as part of the
  metadata. When using an item from an external crate, this data is
  looked up and cached.

* The stability lint works from the computed stability level, rather
  than manual stability attribute annotations. However, the lint still
  checks only a limited set of item uses (e.g., it does not check every
  component of a path on import). This will be addressed in a later PR,
  as part of issue rust-lang#8962.

* The stability lint only applies to items originating from external
  crates, since the stability index is intended as a promise to
  downstream crates.

* The "experimental" lint is now _allow_ by default. This is because
  almost all existing crates have been marked "experimental", pending
  library stabilization. With inheritance in place, this would generate
  a massive explosion of warnings for every Rust program.

  The lint should be changed back to deny-by-default after library
  stabilization is complete.

* The "deprecated" lint still warns by default.

The net result: we can begin tracking stability index for the standard
libraries as we stabilize, without impacting most clients.

Closes rust-lang#13540.
  • Loading branch information
aturon committed Jun 19, 2014
1 parent f05cd6e commit 6008f2c
Show file tree
Hide file tree
Showing 15 changed files with 385 additions and 109 deletions.
33 changes: 24 additions & 9 deletions src/doc/rust.md
Original file line number Diff line number Diff line change
Expand Up @@ -2300,28 +2300,43 @@ One can indicate the stability of an API using the following attributes:
These levels are directly inspired by
[Node.js' "stability index"](http://nodejs.org/api/documentation.html).

There are lints for disallowing items marked with certain levels:
`deprecated`, `experimental` and `unstable`; the first two will warn
by default. Items with not marked with a stability are considered to
be unstable for the purposes of the lint. One can give an optional
Stability levels are inherited, so an items's stability attribute is the
default stability for everything nested underneath it.

There are lints for disallowing items marked with certain levels: `deprecated`,
`experimental` and `unstable`. For now, only `deprecated` warns by default, but
this will change once the standard library has been stabilized.
Stability levels are meant to be promises at the crate
level, so these lints only apply when referencing
items from an _external_ crate, not to items defined within the
current crate. Items with no stability level are considered
to be unstable for the purposes of the lint. One can give an optional
string that will be displayed when the lint flags the use of an item.

This comment has been minimized.

Copy link
@brson

brson Jun 19, 2014

Thanks for updating the manual.


~~~~ {.ignore}
#![warn(unstable)]
For example, if we define one crate called `stability_levels`:

~~~~ {.ignore}
#[deprecated="replaced by `best`"]
fn bad() {
pub fn bad() {
// delete everything
}

fn better() {
pub fn better() {
// delete fewer things
}

#[stable]
fn best() {
pub fn best() {
// delete nothing
}
~~~~

then the lints will work as follows for a client crate:

~~~~ {.ignore}
#![warn(unstable)]
extern crate stability_levels;
use stability_levels::{bad, better, best};

fn main() {
bad(); // "warning: use of deprecated item: replaced by `best`"
Expand Down
7 changes: 5 additions & 2 deletions src/librustc/driver/driver.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ use metadata::common::LinkMeta;
use metadata::creader;
use middle::cfg;
use middle::cfg::graphviz::LabelledCFG;
use middle::{trans, freevars, kind, ty, typeck, lint, reachable};
use middle::{trans, freevars, stability, kind, ty, typeck, lint, reachable};
use middle::dependency_format;
use middle;
use plugin::load::Plugins;
Expand Down Expand Up @@ -312,8 +312,11 @@ pub fn phase_3_run_analysis_passes(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(krate));

let ty_cx = ty::mk_ctxt(sess, def_map, named_region_map, ast_map,
freevars, region_map, lang_items);
freevars, region_map, lang_items, stability_index);

// passes are timed inside typeck
typeck::check_crate(&ty_cx, trait_map, krate);
Expand Down
1 change: 1 addition & 0 deletions src/librustc/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,7 @@ pub mod middle {
pub mod weak_lang_items;
pub mod save;
pub mod intrinsicck;
pub mod stability;
}

pub mod front {
Expand Down
3 changes: 3 additions & 0 deletions src/librustc/metadata/common.rs
Original file line number Diff line number Diff line change
Expand Up @@ -210,6 +210,9 @@ pub static tag_method_argument_name: uint = 0x8f;
pub static tag_reachable_extern_fns: uint = 0x90;
pub static tag_reachable_extern_fn_id: uint = 0x91;

pub static tag_items_data_item_stability: uint = 0x92;


#[deriving(Clone, Show)]
pub struct LinkMeta {
pub crateid: CrateId,
Expand Down
8 changes: 8 additions & 0 deletions src/librustc/metadata/csearch.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ use serialize::ebml::reader;
use std::rc::Rc;
use syntax::ast;
use syntax::ast_map;
use syntax::attr;
use syntax::diagnostic::expect;
use syntax::parse::token;

Expand Down Expand Up @@ -328,3 +329,10 @@ pub fn is_typedef(cstore: &cstore::CStore, did: ast::DefId) -> bool {
let cdata = cstore.get_crate_data(did.krate);
decoder::is_typedef(&*cdata, did.node)
}

pub fn get_stability(cstore: &cstore::CStore,
def: ast::DefId)
-> Option<attr::Stability> {
let cdata = cstore.get_crate_data(def.krate);
decoder::get_stability(&*cdata, def.node)
}
8 changes: 8 additions & 0 deletions src/librustc/metadata/decoder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -439,6 +439,14 @@ pub fn get_type(cdata: Cmd, id: ast::NodeId, tcx: &ty::ctxt)
}
}

pub fn get_stability(cdata: Cmd, id: ast::NodeId) -> Option<attr::Stability> {
let item = lookup_item(id, cdata.data());
reader::maybe_get_doc(item, tag_items_data_item_stability).map(|doc| {
let mut decoder = reader::Decoder::new(doc);
Decodable::decode(&mut decoder).unwrap()
})
}

pub fn get_impl_trait(cdata: Cmd,
id: ast::NodeId,
tcx: &ty::ctxt) -> Option<Rc<ty::TraitRef>>
Expand Down
32 changes: 31 additions & 1 deletion src/librustc/metadata/encoder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -327,6 +327,10 @@ fn encode_enum_variant_info(ecx: &EncodeContext,
encode_parent_item(ebml_w, local_def(id));
encode_visibility(ebml_w, variant.node.vis);
encode_attributes(ebml_w, variant.node.attrs.as_slice());

let stab = ecx.tcx.stability.borrow().lookup_local(variant.node.id);
encode_stability(ebml_w, stab);

match variant.node.kind {
ast::TupleVariantKind(ref args)
if args.len() > 0 && generics.ty_params.len() == 0 => {
Expand Down Expand Up @@ -588,6 +592,7 @@ fn encode_info_for_mod(ecx: &EncodeContext,

encode_path(ebml_w, path.clone());
encode_visibility(ebml_w, vis);
encode_stability(ebml_w, ecx.tcx.stability.borrow().lookup_local(id));

// Encode the reexports of this module, if this module is public.
if vis == Public {
Expand Down Expand Up @@ -717,6 +722,8 @@ fn encode_info_for_struct_ctor(ecx: &EncodeContext,
encode_symbol(ecx, ebml_w, ctor_id);
}

encode_stability(ebml_w, ecx.tcx.stability.borrow().lookup_local(ctor_id));

// indicate that this is a tuple struct ctor, because downstream users will normally want
// the tuple struct definition, but without this there is no way for them to tell that
// they actually have a ctor rather than a normal function
Expand Down Expand Up @@ -761,6 +768,9 @@ fn encode_info_for_method(ecx: &EncodeContext,
encode_method_ty_fields(ecx, ebml_w, m);
encode_parent_item(ebml_w, local_def(parent_id));

let stab = ecx.tcx.stability.borrow().lookup_local(m.def_id.node);
encode_stability(ebml_w, stab);

// The type for methods gets encoded twice, which is unfortunate.
let tpt = lookup_item_type(ecx.tcx, m.def_id);
encode_bounds_and_type(ebml_w, ecx, &tpt);
Expand Down Expand Up @@ -880,6 +890,14 @@ fn encode_sized(ebml_w: &mut Encoder, sized: Sized) {
ebml_w.end_tag();
}

fn encode_stability(ebml_w: &mut Encoder, stab_opt: Option<attr::Stability>) {
stab_opt.map(|stab| {
ebml_w.start_tag(tag_items_data_item_stability);
stab.encode(ebml_w).unwrap();
ebml_w.end_tag();
});
}

fn encode_info_for_item(ecx: &EncodeContext,
ebml_w: &mut Encoder,
item: &Item,
Expand All @@ -900,6 +918,8 @@ fn encode_info_for_item(ecx: &EncodeContext,
ecx.tcx.sess.codemap().span_to_str(item.span));

let def_id = local_def(item.id);
let stab = tcx.stability.borrow().lookup_local(item.id);

match item.node {
ItemStatic(_, m, _) => {
add_to_index(item, ebml_w, index);
Expand All @@ -921,6 +941,7 @@ fn encode_info_for_item(ecx: &EncodeContext,
encode_inlined_item(ecx, ebml_w, IIItemRef(item));
}
encode_visibility(ebml_w, vis);
encode_stability(ebml_w, stab);
ebml_w.end_tag();
}
ItemFn(ref decl, fn_style, _, ref generics, _) => {
Expand All @@ -939,6 +960,7 @@ fn encode_info_for_item(ecx: &EncodeContext,
encode_symbol(ecx, ebml_w, item.id);
}
encode_visibility(ebml_w, vis);
encode_stability(ebml_w, stab);
encode_method_argument_names(ebml_w, &**decl);
ebml_w.end_tag();
}
Expand Down Expand Up @@ -968,6 +990,7 @@ fn encode_info_for_item(ecx: &EncodeContext,
ebml_w.end_tag();
}
encode_visibility(ebml_w, vis);
encode_stability(ebml_w, stab);
ebml_w.end_tag();
}
ItemTy(..) => {
Expand All @@ -979,6 +1002,7 @@ fn encode_info_for_item(ecx: &EncodeContext,
encode_name(ebml_w, item.ident.name);
encode_path(ebml_w, path);
encode_visibility(ebml_w, vis);
encode_stability(ebml_w, stab);
ebml_w.end_tag();
}
ItemEnum(ref enum_definition, ref generics) => {
Expand All @@ -1001,6 +1025,7 @@ fn encode_info_for_item(ecx: &EncodeContext,
encode_inherent_implementations(ecx, ebml_w, def_id);

encode_visibility(ebml_w, vis);
encode_stability(ebml_w, stab);
ebml_w.end_tag();

encode_enum_variant_info(ecx,
Expand Down Expand Up @@ -1035,6 +1060,7 @@ fn encode_info_for_item(ecx: &EncodeContext,
encode_name(ebml_w, item.ident.name);
encode_attributes(ebml_w, item.attrs.as_slice());
encode_path(ebml_w, path.clone());
encode_stability(ebml_w, stab);
encode_visibility(ebml_w, vis);

/* Encode def_ids for each field and method
Expand Down Expand Up @@ -1095,6 +1121,7 @@ fn encode_info_for_item(ecx: &EncodeContext,
encode_impl_vtables(ebml_w, ecx, &impl_vtables);
}
encode_path(ebml_w, path.clone());
encode_stability(ebml_w, stab);
ebml_w.end_tag();

// Iterate down the methods, emitting them. We rely on the
Expand Down Expand Up @@ -1138,6 +1165,7 @@ fn encode_info_for_item(ecx: &EncodeContext,
// should no longer need this ugly little hack either.
encode_sized(ebml_w, sized);
encode_visibility(ebml_w, vis);
encode_stability(ebml_w, stab);
for &method_def_id in ty::trait_method_def_ids(tcx, def_id).iter() {
ebml_w.start_tag(tag_item_trait_method);
encode_def_id(ebml_w, method_def_id);
Expand Down Expand Up @@ -1176,9 +1204,11 @@ fn encode_info_for_item(ecx: &EncodeContext,
ebml_w.start_tag(tag_items_data_item);

encode_method_ty_fields(ecx, ebml_w, &*method_ty);

encode_parent_item(ebml_w, def_id);

let stab = tcx.stability.borrow().lookup_local(method_def_id.node);
encode_stability(ebml_w, stab);

let elem = ast_map::PathName(method_ty.ident.name);
encode_path(ebml_w, path.clone().chain(Some(elem).move_iter()));

Expand Down
40 changes: 11 additions & 29 deletions src/librustc/middle/lint.rs
Original file line number Diff line number Diff line change
Expand Up @@ -372,7 +372,8 @@ static lint_table: &'static [(&'static str, LintSpec)] = &[
LintSpec {
lint: Experimental,
desc: "detects use of #[experimental] items",
default: Warn
// FIXME #6875: Change to Warn after std library stabilization is complete
default: Allow
}),

("unstable",
Expand Down Expand Up @@ -1661,6 +1662,8 @@ fn check_missing_doc_variant(cx: &Context, v: &ast::Variant) {
/// Checks for use of items with #[deprecated], #[experimental] and
/// #[unstable] (or none of them) attributes.
fn check_stability(cx: &Context, e: &ast::Expr) {
let tcx = cx.tcx;

let id = match e.node {
ast::ExprPath(..) | ast::ExprStruct(..) => {
match cx.tcx.def_map.borrow().find(&e.id) {
Expand All @@ -1670,16 +1673,16 @@ fn check_stability(cx: &Context, e: &ast::Expr) {
}
ast::ExprMethodCall(..) => {
let method_call = typeck::MethodCall::expr(e.id);
match cx.tcx.method_map.borrow().find(&method_call) {
match tcx.method_map.borrow().find(&method_call) {
Some(method) => {
match method.origin {
typeck::MethodStatic(def_id) => {
// If this implements a trait method, get def_id
// of the method inside trait definition.
// Otherwise, use the current def_id (which refers
// to the method inside impl).
ty::trait_method_of_method(
cx.tcx, def_id).unwrap_or(def_id)
ty::trait_method_of_method(cx.tcx, def_id)
.unwrap_or(def_id)
}
typeck::MethodParam(typeck::MethodParam {
trait_id: trait_id,
Expand All @@ -1699,32 +1702,11 @@ fn check_stability(cx: &Context, e: &ast::Expr) {
_ => return
};

let stability = if ast_util::is_local(id) {
// this crate
let s = cx.tcx.map.with_attrs(id.node, |attrs| {
attrs.map(|a| attr::find_stability(a.as_slice()))
});
match s {
Some(s) => s,
// stability attributes are promises made across crates; do not
// check anything for crate-local usage.
if ast_util::is_local(id) { return }

// no possibility of having attributes
// (e.g. it's a local variable), so just
// ignore it.
None => return
}
} else {
// cross-crate

let mut s = None;
// run through all the attributes and take the first
// stability one.
csearch::get_item_attrs(&cx.tcx.sess.cstore, id, |attrs| {
if s.is_none() {
s = attr::find_stability(attrs.as_slice())
}
});
s
};
let stability = tcx.stability.borrow_mut().lookup(&tcx.sess.cstore, id);

let (lint, label) = match stability {
// no stability attributes == Unstable
Expand Down
Loading

1 comment on commit 6008f2c

@brson
Copy link

@brson brson commented on 6008f2c Jun 21, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

r+

Please sign in to comment.