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

rustdoc: show struct field docs when inlined #16025

Closed
wants to merge 3 commits into from
Closed

rustdoc: show struct field docs when inlined #16025

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
531a3c6
rustdoc: show struct field docs when inlined
emberian Jul 26, 2014
fc08779
rustdoc: remove extraneous .move_iter().collect()s
emberian Jul 28, 2014
8876ce4
rustc: encode is_sugared_doc on ast::Attribute
emberian Jul 28, 2014
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
6 changes: 6 additions & 0 deletions src/librustc/metadata/common.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -227,3 +227,9 @@ pub static tag_region_param_def_index: uint = 0x94;
pub static tag_unboxed_closures: uint = 0x95;
pub static tag_unboxed_closure: uint = 0x96;
pub static tag_unboxed_closure_type: uint = 0x97;

pub static tag_struct_fields: uint = 0x98;
pub static tag_struct_field: uint = 0x99;
pub static tag_struct_field_id: uint = 0x9a;

pub static tag_attribute_is_sugared_doc: uint = 0x9b;
8 changes: 8 additions & 0 deletions src/librustc/metadata/csearch.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,8 @@ use syntax::attr;
use syntax::diagnostic::expect;
use syntax::parse::token;

use std::collections::hashmap::HashMap;

pub struct StaticMethodInfo {
pub ident: ast::Ident,
pub def_id: ast::DefId,
Expand Down Expand Up @@ -192,6 +194,12 @@ pub fn get_struct_fields(cstore: &cstore::CStore,
decoder::get_struct_fields(cstore.intr.clone(), &*cdata, def.node)
}

pub fn get_struct_field_attrs(cstore: &cstore::CStore, def: ast::DefId) -> HashMap<ast::NodeId,
Vec<ast::Attribute>> {
let cdata = cstore.get_crate_data(def.krate);
decoder::get_struct_field_attrs(&*cdata)
}

pub fn get_type(tcx: &ty::ctxt,
def: ast::DefId)
-> ty::Polytype {
Expand Down
19 changes: 18 additions & 1 deletion src/librustc/metadata/decoder.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ use std::hash::Hash;
use std::hash;
use std::io::extensions::u64_from_be_bytes;
use std::io;
use std::collections::hashmap::HashMap;
use std::rc::Rc;
use std::u64;
use serialize::ebml::reader;
Expand Down Expand Up @@ -963,6 +964,19 @@ pub fn get_item_attrs(cdata: Cmd,
f(get_attributes(item));
}

pub fn get_struct_field_attrs(cdata: Cmd) -> HashMap<ast::NodeId, Vec<ast::Attribute>> {
let data = ebml::Doc::new(cdata.data());
let fields = reader::get_doc(data, tag_struct_fields);
let mut map = HashMap::new();
reader::tagged_docs(fields, tag_struct_field, |field| {
let id = reader::doc_as_u32(reader::get_doc(field, tag_struct_field_id));
let attrs = get_attributes(field);
map.insert(id, attrs);
true
});
map
}

fn struct_field_family_to_visibility(family: Family) -> ast::Visibility {
match family {
PublicField => ast::Public,
Expand Down Expand Up @@ -1042,6 +1056,9 @@ fn get_attributes(md: ebml::Doc) -> Vec<ast::Attribute> {
match reader::maybe_get_doc(md, tag_attributes) {
Some(attrs_d) => {
reader::tagged_docs(attrs_d, tag_attribute, |attr_doc| {
let is_sugared_doc = reader::doc_as_u8(
reader::get_doc(attr_doc, tag_attribute_is_sugared_doc)
) == 1;
let meta_items = get_meta_items(attr_doc);
// Currently it's only possible to have a single meta item on
// an attribute
Expand All @@ -1053,7 +1070,7 @@ fn get_attributes(md: ebml::Doc) -> Vec<ast::Attribute> {
id: attr::mk_attr_id(),
style: ast::AttrOuter,
value: meta_item,
is_sugared_doc: false,
is_sugared_doc: is_sugared_doc,
},
span: codemap::DUMMY_SP
});
Expand Down
26 changes: 26 additions & 0 deletions src/librustc/metadata/encoder.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1490,6 +1490,7 @@ fn encode_attributes(ebml_w: &mut Encoder, attrs: &[Attribute]) {
ebml_w.start_tag(tag_attributes);
for attr in attrs.iter() {
ebml_w.start_tag(tag_attribute);
ebml_w.wr_tagged_u8(tag_attribute_is_sugared_doc, attr.node.is_sugared_doc as u8);
encode_meta_item(ebml_w, attr.node.value);
ebml_w.end_tag();
}
Expand Down Expand Up @@ -1644,6 +1645,29 @@ fn encode_unboxed_closures<'a>(
ebml_w.end_tag();
}

fn encode_struct_field_attrs(ebml_w: &mut Encoder, krate: &Crate) {
struct StructFieldVisitor<'a, 'b> {
ebml_w: &'a mut Encoder<'b>,
}

impl<'a, 'b> Visitor<()> for StructFieldVisitor<'a, 'b> {
fn visit_struct_field(&mut self, field: &ast::StructField, _: ()) {
self.ebml_w.start_tag(tag_struct_field);
self.ebml_w.wr_tagged_u32(tag_struct_field_id, field.node.id);
encode_attributes(self.ebml_w, field.node.attrs.as_slice());
self.ebml_w.end_tag();
}
}

ebml_w.start_tag(tag_struct_fields);
visit::walk_crate(&mut StructFieldVisitor {
ebml_w: ebml_w
}, krate, ());
ebml_w.end_tag();
}
Copy link
Member

Choose a reason for hiding this comment

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

There seems to already be a encode_struct_fields method which seems like this call to encode_attributes should be placed inside (rather than having a separate section). Did you try that and end up having it fall short of what should happen? It may also allow hooking into the existing load_attrs function instead of having to redefined the decode_* variants.

Copy link
Member Author

Choose a reason for hiding this comment

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

encode_struct_fields is only used when encoding complete structs. There's this weird index + custom hashmap that indexes actual items, and the interaction between struct fields and the rest of the struct is weird and I don't really understand it. In particular, struct fields themselves aren't considered items so load_attrs would never see them. Short of sticking the attributes in the field_ty, I don't really see how to handle that.

So yes, I did try that, you rejected it and I wrote this instead 🐈




struct ImplVisitor<'a,'b,'c> {
ecx: &'a EncodeContext<'b>,
ebml_w: &'a mut Encoder<'c>,
Expand Down Expand Up @@ -1928,6 +1952,8 @@ fn encode_metadata_inner(wr: &mut MemWriter, parms: EncodeParams, krate: &Crate)
stats.index_bytes = ebml_w.writer.tell().unwrap() - i;
ebml_w.end_tag();

encode_struct_field_attrs(&mut ebml_w, krate);

stats.total_bytes = ebml_w.writer.tell().unwrap();

if tcx.sess.meta_stats() {
Expand Down
4 changes: 2 additions & 2 deletions src/librustc/middle/ty.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4071,7 +4071,7 @@ pub fn lookup_struct_fields(cx: &ctxt, did: ast::DefId) -> Vec<field_ty> {

let len = results.as_slice().iter().map(|x| x.len()).sum();
let mut result: Vec<field_ty> = Vec::with_capacity(len);
result.extend(results.as_slice().iter().flat_map(|rs| rs.iter().map(|&f| f)));
result.extend(results.as_slice().iter().flat_map(|rs| rs.iter().map(|f| f.clone())));
assert!(result.len() == len);
result
} else {
Expand All @@ -4085,7 +4085,7 @@ pub fn lookup_struct_field(cx: &ctxt,
-> field_ty {
let r = lookup_struct_fields(cx, parent);
match r.iter().find(|f| f.id.node == field_id.node) {
Some(t) => *t,
Some(t) => t.clone(),
None => cx.sess.bug("struct ID not found in parent's fields")
}
}
Expand Down
2 changes: 1 addition & 1 deletion src/librustc/middle/typeck/check/_match.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -322,7 +322,7 @@ pub fn check_struct_pat_fields(pcx: &pat_ctxt,
}
Some(&(index, ref mut used)) => {
*used = true;
let class_field = *class_fields.get(index);
let class_field = class_fields.get(index).clone();
let field_type = ty::lookup_field_type(tcx,
class_id,
class_field.id,
Expand Down
2 changes: 1 addition & 1 deletion src/librustdoc/clean/inline.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -209,7 +209,7 @@ fn build_struct(tcx: &ty::ctxt, did: ast::DefId) -> clean::Struct {
_ => doctree::Plain,
},
generics: (&t.generics, subst::TypeSpace).clean(),
fields: fields.iter().map(|f| f.clean()).collect(),
fields: fields.clean(),
fields_stripped: false,
}
}
Expand Down
62 changes: 36 additions & 26 deletions src/librustdoc/clean/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -347,18 +347,18 @@ impl Clean<Item> for doctree::Module {
}
}
let items: Vec<Vec<Item> > = vec!(
self.structs.clean().move_iter().collect(),
self.enums.clean().move_iter().collect(),
self.fns.clean().move_iter().collect(),
self.structs.clean(),
self.enums.clean(),
self.fns.clean(),
foreigns,
self.mods.clean().move_iter().collect(),
self.typedefs.clean().move_iter().collect(),
self.statics.clean().move_iter().collect(),
self.traits.clean().move_iter().collect(),
self.impls.clean().move_iter().collect(),
self.mods.clean(),
self.typedefs.clean(),
self.statics.clean(),
self.traits.clean(),
self.impls.clean(),
self.view_items.clean().move_iter()
.flat_map(|s| s.move_iter()).collect(),
self.macros.clean().move_iter().collect()
self.macros.clean(),
);

// determine if we should display the inner contents or
Expand Down Expand Up @@ -406,7 +406,7 @@ impl Clean<Attribute> for ast::MetaItem {
match self.node {
ast::MetaWord(ref s) => Word(s.get().to_string()),
ast::MetaList(ref s, ref l) => {
List(s.get().to_string(), l.clean().move_iter().collect())
List(s.get().to_string(), l.clean())
}
ast::MetaNameValue(ref s, ref v) => {
NameValue(s.get().to_string(), lit_to_string(v))
Expand Down Expand Up @@ -460,7 +460,7 @@ impl Clean<TyParam> for ast::TyParam {
TyParam {
name: self.ident.clean(),
did: ast::DefId { krate: ast::LOCAL_CRATE, node: self.id },
bounds: self.bounds.clean().move_iter().collect(),
bounds: self.bounds.clean(),
default: self.default.clean()
}
}
Expand Down Expand Up @@ -688,7 +688,7 @@ impl Clean<Item> for ast::Method {
};
Item {
name: Some(self.pe_ident().clean()),
attrs: self.attrs.clean().move_iter().collect(),
attrs: self.attrs.clean(),
source: self.span.clean(),
def_id: ast_util::local_def(self.id),
visibility: self.pe_vis().clean(),
Expand Down Expand Up @@ -727,7 +727,7 @@ impl Clean<Item> for ast::TypeMethod {
};
Item {
name: Some(self.ident.clean()),
attrs: self.attrs.clean().move_iter().collect(),
attrs: self.attrs.clean(),
source: self.span.clean(),
def_id: ast_util::local_def(self.id),
visibility: None,
Expand Down Expand Up @@ -805,7 +805,7 @@ impl Clean<ClosureDecl> for ast::ClosureTy {
onceness: self.onceness,
fn_style: self.fn_style,
bounds: match self.bounds {
Some(ref x) => x.clean().move_iter().collect(),
Some(ref x) => x.clean(),
None => Vec::new()
},
}
Expand Down Expand Up @@ -1178,7 +1178,7 @@ impl Clean<Type> for ast::Ty {
TyTup(ref tys) => Tuple(tys.iter().map(|x| x.clean()).collect()),
TyPath(ref p, ref tpbs, id) => {
resolve_type(p.clean(),
tpbs.clean().map(|x| x.move_iter().collect()),
tpbs.clean().map(|x| x),
id)
}
TyClosure(ref c, region) => Closure(box c.clean(), region.clean()),
Expand Down Expand Up @@ -1307,7 +1307,7 @@ impl Clean<Item> for ast::StructField {
};
Item {
name: name.clean(),
attrs: self.node.attrs.clean().move_iter().collect(),
attrs: self.node.attrs.clean(),
source: self.span.clean(),
visibility: Some(vis),
stability: get_stability(ast_util::local_def(self.node.id)),
Expand All @@ -1320,16 +1320,26 @@ impl Clean<Item> for ast::StructField {
impl Clean<Item> for ty::field_ty {
fn clean(&self) -> Item {
use syntax::parse::token::special_idents::unnamed_field;
use rustc::metadata::csearch;

let cx = get_cx();
let attrs;

let attr_map = csearch::get_struct_field_attrs(&cx.tcx().sess.cstore, self.id);

let name = if self.name == unnamed_field.name {
attrs = None;
None
} else {
attrs = Some(attr_map.find(&self.id.node).unwrap());
Some(self.name)
};
let cx = get_cx();

let ty = ty::lookup_item_type(cx.tcx(), self.id);

Item {
name: name.clean(),
attrs: inline::load_attrs(cx.tcx(), self.id),
attrs: attrs.unwrap_or(&Vec::new()).clean(),
source: Span::empty(),
visibility: Some(self.vis),
stability: get_stability(self.id),
Expand Down Expand Up @@ -1388,7 +1398,7 @@ impl Clean<VariantStruct> for syntax::ast::StructDef {
fn clean(&self) -> VariantStruct {
VariantStruct {
struct_type: doctree::struct_type_from_def(self),
fields: self.fields.clean().move_iter().collect(),
fields: self.fields.clean(),
fields_stripped: false,
}
}
Expand Down Expand Up @@ -1556,7 +1566,7 @@ impl Clean<Path> for ast::Path {
fn clean(&self) -> Path {
Path {
global: self.global,
segments: self.segments.clean().move_iter().collect(),
segments: self.segments.clean(),
}
}
}
Expand All @@ -1572,8 +1582,8 @@ impl Clean<PathSegment> for ast::PathSegment {
fn clean(&self) -> PathSegment {
PathSegment {
name: self.identifier.clean(),
lifetimes: self.lifetimes.clean().move_iter().collect(),
types: self.types.clean().move_iter().collect()
lifetimes: self.lifetimes.clean(),
types: self.types.clean(),
}
}
}
Expand Down Expand Up @@ -1640,7 +1650,7 @@ impl Clean<BareFunctionDecl> for ast::BareFnTy {
BareFunctionDecl {
fn_style: self.fn_style,
generics: Generics {
lifetimes: self.lifetimes.clean().move_iter().collect(),
lifetimes: self.lifetimes.clean(),
type_params: Vec::new(),
},
decl: self.decl.clean(),
Expand Down Expand Up @@ -1745,7 +1755,7 @@ impl Clean<Vec<Item>> for ast::ViewItem {
let convert = |node: &ast::ViewItem_| {
Item {
name: None,
attrs: self.attrs.clean().move_iter().collect(),
attrs: self.attrs.clean(),
source: self.span.clean(),
def_id: ast_util::local_def(0),
visibility: self.vis.clean(),
Expand Down Expand Up @@ -1840,7 +1850,7 @@ impl Clean<ViewPath> for ast::ViewPath {
GlobImport(resolve_use_source(p.clean(), id)),
ast::ViewPathList(ref p, ref pl, id) => {
ImportList(resolve_use_source(p.clean(), id),
pl.clean().move_iter().collect())
pl.clean())
}
}
}
Expand Down Expand Up @@ -1893,7 +1903,7 @@ impl Clean<Item> for ast::ForeignItem {
};
Item {
name: Some(self.ident.clean()),
attrs: self.attrs.clean().move_iter().collect(),
attrs: self.attrs.clean(),
source: self.span.clean(),
def_id: ast_util::local_def(self.id),
visibility: self.vis.clean(),
Expand Down