Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Detect stability attributes on methods. #10990

Merged
merged 1 commit into from
Dec 17, 2013
Merged

Detect stability attributes on methods. #10990

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/librustc/driver/driver.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -312,7 +312,7 @@ pub fn phase_3_run_analysis_passes(sess: Session,
&exported_items, reachable_map, crate));

time(time_passes, "lint checking", (), |_|
lint::check_crate(ty_cx, &exported_items, crate));
lint::check_crate(ty_cx, method_map, &exported_items, crate));

CrateAnalysis {
exp_map2: exp_map2,
Expand Down
8 changes: 7 additions & 1 deletion src/librustc/metadata/encoder.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -812,6 +812,10 @@ fn encode_info_for_method(ecx: &EncodeContext,
encode_bounds_and_type(ebml_w, ecx, &tpt);

encode_path(ecx, ebml_w, impl_path, ast_map::path_name(m.ident));
match ast_method_opt {
Some(ast_method) => encode_attributes(ebml_w, ast_method.attrs),
None => ()
}

for ast_method in ast_method_opt.iter() {
let num_params = tpt.generics.type_param_defs.len();
Expand Down Expand Up @@ -1205,11 +1209,13 @@ fn encode_info_for_item(ecx: &EncodeContext,
}

match ms[i] {
required(_) => {
required(ref tm) => {
encode_attributes(ebml_w, tm.attrs);
encode_method_sort(ebml_w, 'r');
}

provided(m) => {
encode_attributes(ebml_w, m.attrs);
// If this is a static method, we've already encoded
// this.
if method_ty.explicit_self != sty_static {
Expand Down
46 changes: 38 additions & 8 deletions src/librustc/middle/lint.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ use driver::session;
use middle::privacy;
use middle::trans::adt; // for `adt::is_ffi_safe`
use middle::ty;
use middle::typeck;
use middle::pat_util;
use metadata::csearch;
use util::ppaux::{ty_to_str};
Expand Down Expand Up @@ -359,6 +360,9 @@ struct Context<'a> {
cur: SmallIntMap<(level, LintSource)>,
// context we're checking in (used to access fields like sess)
tcx: ty::ctxt,
// maps from an expression id that corresponds to a method call to the
// details of the method to be invoked
method_map: typeck::method_map,
// Items exported by the crate; used by the missing_doc lint.
exported_items: &'a privacy::ExportedItems,
// The id of the current `ast::struct_def` being walked.
Expand Down Expand Up @@ -1176,20 +1180,43 @@ 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 def = match e.node {
ast::ExprMethodCall(..) |
ast::ExprPath(..) |
ast::ExprStruct(..) => {
let id = match e.node {
ast::ExprPath(..) | ast::ExprStruct(..) => {
match cx.tcx.def_map.find(&e.id) {
Some(&def) => def,
Some(&def) => ast_util::def_id_of_def(def),
None => return
}
}
ast::ExprMethodCall(..) => {
match cx.method_map.find(&e.id) {
Some(&typeck::method_map_entry { origin, .. }) => {
match origin {
typeck::method_static(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
Copy link
Member

Choose a reason for hiding this comment

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

To check my understanding: this is basically saying that in

trait Foo {
     fn bar(&self); // *
}

struct SomeType;

impl Foo for SomeType { fn bar(&self) {} }
impl SomeType { fn baz(&self) {} } // +

SomeType.bar() will be checking the attributes on the method marked *, while SomeType.baz() will just be checking the line marked + (I guess the second statement is obvious, since there isn't anything else to check anyway).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yea, that's what I had in mind.

// to the method inside impl).
ty::trait_method_of_method(
cx.tcx, def_id).unwrap_or(def_id)
}
typeck::method_param(typeck::method_param {
trait_id: trait_id,
method_num: index,
..
})
| typeck::method_object(typeck::method_object {
trait_id: trait_id,
method_num: index,
..
}) => ty::trait_method(cx.tcx, trait_id, index).def_id
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Is this correct?

Copy link
Member

Choose a reason for hiding this comment

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

It looks ok to me, although it might be worth adding tests like:

trait Foo {
    #[deprecated]
    fn bar(&self) {}
}

fn param<F: Foo>(f: F) {
    f.bar()
}
fn object(f: &Foo) {
    f.bar()
}

to check that both of these work properly.

}
}
None => return
}
}
_ => return
};

let id = ast_util::def_id_of_def(def);

let stability = if ast_util::is_local(id) {
// this crate
match cx.tcx.items.find(&id.node) {
Expand All @@ -1208,7 +1235,8 @@ fn check_stability(cx: &Context, e: &ast::Expr) {
None => return
}
}
_ => cx.tcx.sess.bug(format!("handle_def: {:?} not found", id))
_ => cx.tcx.sess.span_bug(e.span,
format!("handle_def: {:?} not found", id))
}
} else {
// cross-crate
Expand Down Expand Up @@ -1395,12 +1423,14 @@ impl<'a> IdVisitingOperation for Context<'a> {
}

pub fn check_crate(tcx: ty::ctxt,
method_map: typeck::method_map,
exported_items: &privacy::ExportedItems,
crate: &ast::Crate) {
let mut cx = Context {
dict: @get_lint_dict(),
cur: SmallIntMap::new(),
tcx: tcx,
method_map: method_map,
exported_items: exported_items,
cur_struct_def_id: -1,
is_doc_hidden: false,
Expand Down
68 changes: 53 additions & 15 deletions src/librustc/middle/ty.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4513,29 +4513,67 @@ pub fn populate_implementations_for_trait_if_necessary(
tcx.populated_external_traits.insert(trait_id);
}

/// If the given def ID describes a trait method, returns the ID of the trait
/// that the method belongs to. Otherwise, returns `None`.
/// Given the def_id of an impl, return the def_id of the trait it implements.
/// If it implements no trait, return `None`.
pub fn trait_id_of_impl(tcx: ctxt,
def_id: ast::DefId) -> Option<ast::DefId> {
let node = match tcx.items.find(&def_id.node) {
Some(node) => node,
None => return None
};
match node {
&ast_map::node_item(item, _) => {
match item.node {
ast::item_impl(_, Some(ref trait_ref), _, _) => {
Some(node_id_to_trait_ref(tcx, trait_ref.ref_id).def_id)
}
_ => None
}
}
_ => None
}
}

/// If the given def ID describes a method belonging to a trait (either a
/// default method or an implementation of a trait method), return the ID of
/// the trait that the method belongs to. Otherwise, return `None`.
pub fn trait_of_method(tcx: ctxt, def_id: ast::DefId)
-> Option<ast::DefId> {
if def_id.crate != LOCAL_CRATE {
return csearch::get_trait_of_method(tcx.cstore, def_id, tcx);
}
match tcx.methods.find(&def_id) {
Some(method_descriptor) => {
match method_descriptor.container {
TraitContainer(id) => return Some(id),
_ => {}
Some(method) => {
match method.container {
TraitContainer(def_id) => Some(def_id),
ImplContainer(def_id) => trait_id_of_impl(tcx, def_id),
}
}
None => {}
None => None
}
}

// If the method was in the local crate, then if we got here we know the
// answer is negative.
if def_id.crate == LOCAL_CRATE {
return None
/// If the given def ID describes a method belonging to a trait, (either a
/// default method or an implementation of a trait method), return the ID of
/// the method inside trait definition (this means that if the given def ID
/// is already that of the original trait method, then the return value is
/// the same).
/// Otherwise, return `None`.
pub fn trait_method_of_method(tcx: ctxt,
def_id: ast::DefId) -> Option<ast::DefId> {
let name = match tcx.methods.find(&def_id) {
Some(method) => method.ident.name,
None => return None
};
match trait_of_method(tcx, def_id) {
Some(trait_did) => {
let trait_methods = ty::trait_methods(tcx, trait_did);
trait_methods.iter()
.position(|m| m.ident.name == name)
.map(|idx| ty::trait_method(tcx, trait_did, idx).def_id)
}
None => None
}

let result = csearch::get_trait_of_method(tcx.cstore, def_id, tcx);

result
}

/// Creates a hash of the type `t` which will be the same no matter what crate
Expand Down
100 changes: 72 additions & 28 deletions src/test/compile-fail/lint-stability.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,32 +26,32 @@ mod cross_crate {
let foo = MethodTester;

deprecated(); //~ ERROR use of deprecated item
foo.method_deprecated(); // ~ ERROR use of deprecated item
foo.trait_deprecated(); // ~ ERROR use of deprecated item
foo.method_deprecated(); //~ ERROR use of deprecated item
foo.trait_deprecated(); //~ ERROR use of deprecated item

deprecated_text(); //~ ERROR use of deprecated item: text
foo.method_deprecated_text(); // ~ ERROR use of deprecated item: text
foo.trait_deprecated_text(); // ~ ERROR use of deprecated item: text
foo.method_deprecated_text(); //~ ERROR use of deprecated item: text
foo.trait_deprecated_text(); //~ ERROR use of deprecated item: text

experimental(); //~ ERROR use of experimental item
foo.method_experimental(); // ~ ERROR use of experimental item
foo.trait_experimental(); // ~ ERROR use of experimental item
foo.method_experimental(); //~ ERROR use of experimental item
foo.trait_experimental(); //~ ERROR use of experimental item

experimental_text(); //~ ERROR use of experimental item: text
foo.method_experimental_text(); // ~ ERROR use of experimental item: text
foo.trait_experimental_text(); // ~ ERROR use of experimental item: text
foo.method_experimental_text(); //~ ERROR use of experimental item: text
foo.trait_experimental_text(); //~ ERROR use of experimental item: text

unstable(); //~ ERROR use of unstable item
foo.method_unstable(); // ~ ERROR use of unstable item
foo.trait_unstable(); // ~ ERROR use of unstable item
foo.method_unstable(); //~ ERROR use of unstable item
foo.trait_unstable(); //~ ERROR use of unstable item

unstable_text(); //~ ERROR use of unstable item: text
foo.method_unstable_text(); // ~ ERROR use of unstable item: text
foo.trait_unstable_text(); // ~ ERROR use of unstable item: text
foo.method_unstable_text(); //~ ERROR use of unstable item: text
foo.trait_unstable_text(); //~ ERROR use of unstable item: text

unmarked(); //~ ERROR use of unmarked item
foo.method_unmarked(); // ~ ERROR use of unmarked item
foo.trait_unmarked(); // ~ ERROR use of unmarked item
foo.method_unmarked(); //~ ERROR use of unmarked item
foo.trait_unmarked(); //~ ERROR use of unmarked item

stable();
foo.method_stable();
Expand Down Expand Up @@ -102,6 +102,28 @@ mod cross_crate {
let _ = FrozenVariant;
let _ = LockedVariant;
}

fn test_method_param<F: Trait>(foo: F) {
foo.trait_deprecated(); //~ ERROR use of deprecated item
foo.trait_deprecated_text(); //~ ERROR use of deprecated item: text
foo.trait_experimental(); //~ ERROR use of experimental item
foo.trait_experimental_text(); //~ ERROR use of experimental item: text
foo.trait_unstable(); //~ ERROR use of unstable item
foo.trait_unstable_text(); //~ ERROR use of unstable item: text
foo.trait_unmarked(); //~ ERROR use of unmarked item
foo.trait_stable();
}

fn test_method_object(foo: &Trait) {
foo.trait_deprecated(); //~ ERROR use of deprecated item
foo.trait_deprecated_text(); //~ ERROR use of deprecated item: text
foo.trait_experimental(); //~ ERROR use of experimental item
foo.trait_experimental_text(); //~ ERROR use of experimental item: text
foo.trait_unstable(); //~ ERROR use of unstable item
foo.trait_unstable_text(); //~ ERROR use of unstable item: text
foo.trait_unmarked(); //~ ERROR use of unmarked item
foo.trait_stable();
}
}

mod this_crate {
Expand Down Expand Up @@ -259,32 +281,32 @@ mod this_crate {
let foo = MethodTester;

deprecated(); //~ ERROR use of deprecated item
foo.method_deprecated(); // ~ ERROR use of deprecated item
foo.trait_deprecated(); // ~ ERROR use of deprecated item
foo.method_deprecated(); //~ ERROR use of deprecated item
foo.trait_deprecated(); //~ ERROR use of deprecated item

deprecated_text(); //~ ERROR use of deprecated item: text
foo.method_deprecated_text(); // ~ ERROR use of deprecated item: text
foo.trait_deprecated_text(); // ~ ERROR use of deprecated item: text
foo.method_deprecated_text(); //~ ERROR use of deprecated item: text
foo.trait_deprecated_text(); //~ ERROR use of deprecated item: text

experimental(); //~ ERROR use of experimental item
foo.method_experimental(); // ~ ERROR use of experimental item
foo.trait_experimental(); // ~ ERROR use of experimental item
foo.method_experimental(); //~ ERROR use of experimental item
foo.trait_experimental(); //~ ERROR use of experimental item

experimental_text(); //~ ERROR use of experimental item: text
foo.method_experimental_text(); // ~ ERROR use of experimental item: text
foo.trait_experimental_text(); // ~ ERROR use of experimental item: text
foo.method_experimental_text(); //~ ERROR use of experimental item: text
foo.trait_experimental_text(); //~ ERROR use of experimental item: text

unstable(); //~ ERROR use of unstable item
foo.method_unstable(); // ~ ERROR use of unstable item
foo.trait_unstable(); // ~ ERROR use of unstable item
foo.method_unstable(); //~ ERROR use of unstable item
foo.trait_unstable(); //~ ERROR use of unstable item

unstable_text(); //~ ERROR use of unstable item: text
foo.method_unstable_text(); // ~ ERROR use of unstable item: text
foo.trait_unstable_text(); // ~ ERROR use of unstable item: text
foo.method_unstable_text(); //~ ERROR use of unstable item: text
foo.trait_unstable_text(); //~ ERROR use of unstable item: text

unmarked(); //~ ERROR use of unmarked item
foo.method_unmarked(); // ~ ERROR use of unmarked item
foo.trait_unmarked(); // ~ ERROR use of unmarked item
foo.method_unmarked(); //~ ERROR use of unmarked item
foo.trait_unmarked(); //~ ERROR use of unmarked item

stable();
foo.method_stable();
Expand Down Expand Up @@ -335,6 +357,28 @@ mod this_crate {
let _ = FrozenVariant;
let _ = LockedVariant;
}

fn test_method_param<F: Trait>(foo: F) {
foo.trait_deprecated(); //~ ERROR use of deprecated item
foo.trait_deprecated_text(); //~ ERROR use of deprecated item: text
foo.trait_experimental(); //~ ERROR use of experimental item
foo.trait_experimental_text(); //~ ERROR use of experimental item: text
foo.trait_unstable(); //~ ERROR use of unstable item
foo.trait_unstable_text(); //~ ERROR use of unstable item: text
foo.trait_unmarked(); //~ ERROR use of unmarked item
foo.trait_stable();
}

fn test_method_object(foo: &Trait) {
foo.trait_deprecated(); //~ ERROR use of deprecated item
foo.trait_deprecated_text(); //~ ERROR use of deprecated item: text
foo.trait_experimental(); //~ ERROR use of experimental item
foo.trait_experimental_text(); //~ ERROR use of experimental item: text
foo.trait_unstable(); //~ ERROR use of unstable item
foo.trait_unstable_text(); //~ ERROR use of unstable item: text
foo.trait_unmarked(); //~ ERROR use of unmarked item
foo.trait_stable();
}
}

fn main() {}