From a25bbcc27fcbddb005b3025086f87940937b7619 Mon Sep 17 00:00:00 2001 From: Alex Crichton Date: Mon, 21 Oct 2013 10:37:36 -0700 Subject: [PATCH 1/3] Propagate reachability through reexported impls When re-exporting a trait/structure/enum, then we need to propagate the reachability of the type through the methods that are defined on it. Closes #9906 Closes #9968 --- src/librustc/middle/reachable.rs | 107 +++++++++++++++++++++---------- src/test/auxiliary/issue-9906.rs | 28 ++++++++ src/test/auxiliary/issue-9968.rs | 32 +++++++++ src/test/run-pass/issue-9906.rs | 19 ++++++ src/test/run-pass/issue-9968.rs | 21 ++++++ 5 files changed, 172 insertions(+), 35 deletions(-) create mode 100644 src/test/auxiliary/issue-9906.rs create mode 100644 src/test/auxiliary/issue-9968.rs create mode 100644 src/test/run-pass/issue-9906.rs create mode 100644 src/test/run-pass/issue-9968.rs diff --git a/src/librustc/middle/reachable.rs b/src/librustc/middle/reachable.rs index 3c78c86d43603..f65eeee49d510 100644 --- a/src/librustc/middle/reachable.rs +++ b/src/librustc/middle/reachable.rs @@ -21,9 +21,9 @@ use middle::privacy; use middle::resolve; use std::hashmap::HashSet; -use syntax::ast::*; +use syntax::ast; use syntax::ast_map; -use syntax::ast_util::{def_id_of_def, is_local}; +use syntax::ast_util::{def_id_of_def, is_local, local_def}; use syntax::attr; use syntax::parse::token; use syntax::visit::Visitor; @@ -31,26 +31,26 @@ use syntax::visit; // Returns true if the given set of attributes contains the `#[inline]` // attribute. -fn attributes_specify_inlining(attrs: &[Attribute]) -> bool { +fn attributes_specify_inlining(attrs: &[ast::Attribute]) -> bool { attr::contains_name(attrs, "inline") } // Returns true if the given set of generics implies that the item it's // associated with must be inlined. -fn generics_require_inlining(generics: &Generics) -> bool { +fn generics_require_inlining(generics: &ast::Generics) -> bool { !generics.ty_params.is_empty() } // Returns true if the given item must be inlined because it may be // monomorphized or it was marked with `#[inline]`. This will only return // true for functions. -fn item_might_be_inlined(item: @item) -> bool { +fn item_might_be_inlined(item: @ast::item) -> bool { if attributes_specify_inlining(item.attrs) { return true } match item.node { - item_fn(_, _, _, ref generics, _) => { + ast::item_fn(_, _, _, ref generics, _) => { generics_require_inlining(generics) } _ => false, @@ -59,17 +59,17 @@ fn item_might_be_inlined(item: @item) -> bool { // Returns true if the given type method must be inlined because it may be // monomorphized or it was marked with `#[inline]`. -fn ty_method_might_be_inlined(ty_method: &TypeMethod) -> bool { +fn ty_method_might_be_inlined(ty_method: &ast::TypeMethod) -> bool { attributes_specify_inlining(ty_method.attrs) || generics_require_inlining(&ty_method.generics) } // Returns true if the given trait method must be inlined because it may be // monomorphized or it was marked with `#[inline]`. -fn trait_method_might_be_inlined(trait_method: &trait_method) -> bool { +fn trait_method_might_be_inlined(trait_method: &ast::trait_method) -> bool { match *trait_method { - required(ref ty_method) => ty_method_might_be_inlined(ty_method), - provided(_) => true + ast::required(ref ty_method) => ty_method_might_be_inlined(ty_method), + ast::provided(_) => true } } @@ -81,27 +81,27 @@ struct ReachableContext { // methods they've been resolved to. method_map: typeck::method_map, // The set of items which must be exported in the linkage sense. - reachable_symbols: @mut HashSet, + reachable_symbols: @mut HashSet, // A worklist of item IDs. Each item ID in this worklist will be inlined // and will be scanned for further references. - worklist: @mut ~[NodeId], + worklist: @mut ~[ast::NodeId], // Known reexports of modules exp_map2: resolve::ExportMap2, } struct MarkSymbolVisitor { - worklist: @mut ~[NodeId], + worklist: @mut ~[ast::NodeId], method_map: typeck::method_map, tcx: ty::ctxt, - reachable_symbols: @mut HashSet, + reachable_symbols: @mut HashSet, } impl Visitor<()> for MarkSymbolVisitor { - fn visit_expr(&mut self, expr:@Expr, _:()) { + fn visit_expr(&mut self, expr:@ast::Expr, _:()) { match expr.node { - ExprPath(_) => { + ast::ExprPath(_) => { let def = match self.tcx.def_map.find(&expr.id) { Some(&def) => def, None => { @@ -118,7 +118,7 @@ impl Visitor<()> for MarkSymbolVisitor { } self.reachable_symbols.insert(def_id.node); } - ExprMethodCall(*) => { + ast::ExprMethodCall(*) => { match self.method_map.find(&expr.id) { Some(&typeck::method_map_entry { origin: typeck::method_static(def_id), @@ -162,9 +162,9 @@ impl ReachableContext { // Returns true if the given def ID represents a local item that is // eligible for inlining and false otherwise. - fn def_id_represents_local_inlined_item(tcx: ty::ctxt, def_id: DefId) + fn def_id_represents_local_inlined_item(tcx: ty::ctxt, def_id: ast::DefId) -> bool { - if def_id.crate != LOCAL_CRATE { + if def_id.crate != ast::LOCAL_CRATE { return false } @@ -172,14 +172,14 @@ impl ReachableContext { match tcx.items.find(&node_id) { Some(&ast_map::node_item(item, _)) => { match item.node { - item_fn(*) => item_might_be_inlined(item), + ast::item_fn(*) => item_might_be_inlined(item), _ => false, } } Some(&ast_map::node_trait_method(trait_method, _, _)) => { match *trait_method { - required(_) => false, - provided(_) => true, + ast::required(_) => false, + ast::provided(_) => true, } } Some(&ast_map::node_method(method, impl_did, _)) => { @@ -189,11 +189,11 @@ impl ReachableContext { } else { // Check the impl. If the generics on the self type of the // impl require inlining, this method does too. - assert!(impl_did.crate == LOCAL_CRATE); + assert!(impl_did.crate == ast::LOCAL_CRATE); match tcx.items.find(&impl_did.node) { Some(&ast_map::node_item(item, _)) => { match item.node { - item_impl(ref generics, _, _, _) => { + ast::item_impl(ref generics, _, _, _) => { generics_require_inlining(generics) } _ => false @@ -231,7 +231,7 @@ impl ReachableContext { } } - fn propagate_mod(&self, id: NodeId) { + fn propagate_mod(&self, id: ast::NodeId) { match self.exp_map2.find(&id) { Some(l) => { for reexport in l.iter() { @@ -262,7 +262,7 @@ impl ReachableContext { match self.tcx.items.find(&search_item) { Some(&ast_map::node_item(item, _)) => { match item.node { - item_fn(_, _, _, _, ref search_block) => { + ast::item_fn(_, _, _, _, ref search_block) => { visit::walk_block(&mut visitor, search_block, ()) } // Our recursion into modules involves looking up their @@ -270,13 +270,50 @@ impl ReachableContext { // exports. Privacy will put them in the worklist, but // we won't find them in the ast_map, so this is where // we deal with publicly re-exported items instead. - item_mod(*) => { self.propagate_mod(item.id); } + ast::item_mod(*) => self.propagate_mod(item.id), + + // Implementations of exported structs/enums need to get + // added to the worklist (as all their methods should be + // accessible) + ast::item_struct(*) | ast::item_enum(*) => { + let def = local_def(item.id); + let impls = match self.tcx.inherent_impls.find(&def) { + Some(&impls) => impls, + None => continue + }; + for imp in impls.iter() { + if is_local(imp.did) { + self.worklist.push(imp.did.node); + } + } + } + + // Propagate through this impl + ast::item_impl(_, _, _, ref methods) => { + for method in methods.iter() { + self.worklist.push(method.id); + } + } + + // Default methods of exported traits need to all be + // accessible. + ast::item_trait(_, _, ref methods) => { + for method in methods.iter() { + match *method { + ast::required(*) => {} + ast::provided(ref method) => { + self.worklist.push(method.id); + } + } + } + } + // These are normal, nothing reachable about these // inherently and their children are already in the // worklist - item_struct(*) | item_impl(*) | item_static(*) | - item_enum(*) | item_ty(*) | item_trait(*) | - item_foreign_mod(*) => {} + ast::item_static(*) | ast::item_ty(*) | + ast::item_foreign_mod(*) => {} + _ => { self.tcx.sess.span_bug(item.span, "found non-function item \ @@ -286,10 +323,10 @@ impl ReachableContext { } Some(&ast_map::node_trait_method(trait_method, _, _)) => { match *trait_method { - required(*) => { + ast::required(*) => { // Keep going, nothing to get exported } - provided(ref method) => { + ast::provided(ref method) => { visit::walk_block(&mut visitor, &method.body, ()) } } @@ -310,7 +347,7 @@ impl ReachableContext { worklist: {}", desc)) } - None if search_item == CRATE_NODE_ID => { + None if search_item == ast::CRATE_NODE_ID => { self.propagate_mod(search_item); } None => { @@ -329,7 +366,7 @@ impl ReachableContext { // reachability, which might result in a compile time loss. fn mark_destructors_reachable(&self) { for (_, destructor_def_id) in self.tcx.destructor_for_type.iter() { - if destructor_def_id.crate == LOCAL_CRATE { + if destructor_def_id.crate == ast::LOCAL_CRATE { self.reachable_symbols.insert(destructor_def_id.node); } } @@ -340,7 +377,7 @@ pub fn find_reachable(tcx: ty::ctxt, method_map: typeck::method_map, exp_map2: resolve::ExportMap2, exported_items: &privacy::ExportedItems) - -> @mut HashSet { + -> @mut HashSet { // XXX(pcwalton): We only need to mark symbols that are exported. But this // is more complicated than just looking at whether the symbol is `pub`, // because it might be the target of a `pub use` somewhere. For now, I diff --git a/src/test/auxiliary/issue-9906.rs b/src/test/auxiliary/issue-9906.rs new file mode 100644 index 0000000000000..09ce8f0097ea6 --- /dev/null +++ b/src/test/auxiliary/issue-9906.rs @@ -0,0 +1,28 @@ +// Copyright 2013 The Rust Project Developers. See the COPYRIGHT +// file at the top-level directory of this distribution and at +// http://rust-lang.org/COPYRIGHT. +// +// Licensed under the Apache License, Version 2.0 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +// xfail-fast windows doesn't like extern mod +// aux-build:issue-9906.rs + +pub use other::FooBar; +pub use other::foo; + +mod other { + pub struct FooBar{value: int} + impl FooBar{ + pub fn new(val: int) -> FooBar { + FooBar{value: val} + } + } + + pub fn foo(){ + 1+1; + } +} diff --git a/src/test/auxiliary/issue-9968.rs b/src/test/auxiliary/issue-9968.rs new file mode 100644 index 0000000000000..d04d761e11256 --- /dev/null +++ b/src/test/auxiliary/issue-9968.rs @@ -0,0 +1,32 @@ +// Copyright 2013 The Rust Project Developers. See the COPYRIGHT +// file at the top-level directory of this distribution and at +// http://rust-lang.org/COPYRIGHT. +// +// Licensed under the Apache License, Version 2.0 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +pub use internal::core::{Trait, Struct}; + +mod internal { + pub mod core { + pub struct Struct; + impl Struct { + pub fn init() -> Struct { + Struct + } + } + + pub trait Trait { + fn test(&self) { + private(); + } + } + + impl Trait for Struct {} + + fn private() { } + } +} diff --git a/src/test/run-pass/issue-9906.rs b/src/test/run-pass/issue-9906.rs new file mode 100644 index 0000000000000..287000d2fb1d4 --- /dev/null +++ b/src/test/run-pass/issue-9906.rs @@ -0,0 +1,19 @@ +// Copyright 2013 The Rust Project Developers. See the COPYRIGHT +// file at the top-level directory of this distribution and at +// http://rust-lang.org/COPYRIGHT. +// +// Licensed under the Apache License, Version 2.0 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +// xfail-fast windows doesn't like extern mod +// aux-build:issue-9906.rs + +extern mod testmod(name = "issue-9906"); + +fn main() { + testmod::foo(); + testmod::FooBar::new(1); +} diff --git a/src/test/run-pass/issue-9968.rs b/src/test/run-pass/issue-9968.rs new file mode 100644 index 0000000000000..70338bd0f32b4 --- /dev/null +++ b/src/test/run-pass/issue-9968.rs @@ -0,0 +1,21 @@ +// Copyright 2013 The Rust Project Developers. See the COPYRIGHT +// file at the top-level directory of this distribution and at +// http://rust-lang.org/COPYRIGHT. +// +// Licensed under the Apache License, Version 2.0 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +// xfail-fast windows doesn't like extern mod +// aux-build:issue-9968.rs + +extern mod lib(name = "issue-9968"); + +use lib::{Trait, Struct}; + +fn main() +{ + Struct::init().test(); +} From bf6bb01a22ee82f96256e84d263c851dd163b1fb Mon Sep 17 00:00:00 2001 From: Alex Crichton Date: Mon, 21 Oct 2013 10:48:57 -0700 Subject: [PATCH 2/3] rustdoc: Don't overflow long type/module names Closes #9862 --- src/librustdoc/html/static/main.css | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/librustdoc/html/static/main.css b/src/librustdoc/html/static/main.css index f24110a62eb1b..eea776c115661 100644 --- a/src/librustdoc/html/static/main.css +++ b/src/librustdoc/html/static/main.css @@ -110,6 +110,13 @@ body { margin-bottom: 10px; } .block h2 { margin-top: 0; } +.block a { + display: inline-block; + width: 100%; + text-overflow: ellipsis; + overflow: hidden; + line-height: 15px; +} .content { background: #f3f3f3; From 41f6e97a445f736ab437b2f64756b967cf69d763 Mon Sep 17 00:00:00 2001 From: Alex Crichton Date: Mon, 21 Oct 2013 11:33:04 -0700 Subject: [PATCH 3/3] rustdoc: Render default methods for impls as well This does not work for cross-crate implementations of traits. Cross-crate implementations are a separate issue that should be addressed separately. Basically when an implementation of an external trait is detected, the trait would have to be loaded at that time (or possibly sooner...). Rustdoc currently doesn't have the proper infrastructure for adding this. Closes #9985 cc #9999 --- src/librustdoc/html/render.rs | 85 +++++++++++++++++++++++------------ 1 file changed, 56 insertions(+), 29 deletions(-) diff --git a/src/librustdoc/html/render.rs b/src/librustdoc/html/render.rs index 53c3cf3a10c79..8b089e76f3a0b 100644 --- a/src/librustdoc/html/render.rs +++ b/src/librustdoc/html/render.rs @@ -149,10 +149,9 @@ pub struct Cache { /// This map contains information about all known traits of this crate. /// Implementations of a crate should inherit the documentation of the - /// parent trait if no extra documentation is specified, and this map is - /// keyed on trait id with a value of a 'method name => documentation' - /// mapping. - traits: HashMap>, + /// parent trait if no extra documentation is specified, and default methods + /// should show up in documentation about trait implementations. + traits: HashMap, /// When rendering traits, it's often useful to be able to list all /// implementors of the trait, and this mapping is exactly, that: a mapping @@ -488,18 +487,7 @@ impl DocFolder for Cache { // trait match item.inner { clean::TraitItem(ref t) => { - let mut dox = HashMap::new(); - for meth in t.methods.iter() { - let it = meth.item(); - match it.doc_value() { - None => {} - Some(s) => { - dox.insert(it.name.get_ref().to_owned(), - s.to_owned()); - } - } - } - self.traits.insert(item.id, dox); + self.traits.insert(item.id, t.clone()); } _ => {} } @@ -1480,18 +1468,25 @@ fn render_impl(w: &mut io::Writer, i: &clean::Impl, dox: &Option<~str>) { } None => {} } - write!(w, "
"); - for meth in i.methods.iter() { + + fn docmeth(w: &mut io::Writer, item: &clean::Item) -> bool { write!(w, "

", - *meth.name.get_ref()); - render_method(w, meth, false); + *item.name.get_ref()); + render_method(w, item, false); write!(w, "

\n"); - match meth.doc_value() { + match item.doc_value() { Some(s) => { write!(w, "
{}
", Markdown(s)); - continue + true } - None => {} + None => false + } + } + + write!(w, "
"); + for meth in i.methods.iter() { + if docmeth(w, meth) { + continue } // No documentation? Attempt to slurp in the trait's documentation @@ -1501,13 +1496,19 @@ fn render_impl(w: &mut io::Writer, i: &clean::Impl, dox: &Option<~str>) { }; do local_data::get(cache_key) |cache| { do cache.unwrap().read |cache| { - let name = meth.name.get_ref().as_slice(); match cache.traits.find(&trait_id) { - Some(m) => { - match m.find_equiv(&name) { - Some(s) => { - write!(w, "
{}
", - Markdown(s.as_slice())); + Some(t) => { + let name = meth.name.clone(); + match t.methods.iter().find(|t| t.item().name == name) { + Some(method) => { + match method.item().doc_value() { + Some(s) => { + write!(w, + "
{}
", + Markdown(s)); + } + None => {} + } } None => {} } @@ -1517,6 +1518,32 @@ fn render_impl(w: &mut io::Writer, i: &clean::Impl, dox: &Option<~str>) { } } } + + // If we've implemented a trait, then also emit documentation for all + // default methods which weren't overridden in the implementation block. + match trait_id { + None => {} + Some(id) => { + do local_data::get(cache_key) |cache| { + do cache.unwrap().read |cache| { + match cache.traits.find(&id) { + Some(t) => { + for method in t.methods.iter() { + let n = method.item().name.clone(); + match i.methods.iter().find(|m| m.name == n) { + Some(*) => continue, + None => {} + } + + docmeth(w, method.item()); + } + } + None => {} + } + } + } + } + } write!(w, "
"); }