Skip to content

Commit

Permalink
Auto merge of #33153 - mitaa:rdoc-dejavu, r=alexcrichton
Browse files Browse the repository at this point in the history
rustdoc: Only record the same impl once

Due to inlining it is possible to visit the same module multiple times during `<Cache as DocFolder>::fold_crate`, so we keep track of the modules we've already visited.

fixes #33054

r? @alexcrichton
  • Loading branch information
bors committed Apr 24, 2016
2 parents 23ccadd + 8ab2c20 commit 8d0dd78
Show file tree
Hide file tree
Showing 4 changed files with 87 additions and 27 deletions.
9 changes: 6 additions & 3 deletions src/etc/htmldocck.py
Original file line number Diff line number Diff line change
Expand Up @@ -342,9 +342,9 @@ def check_tree_text(tree, path, pat, regexp):
return ret


def check_tree_count(tree, path, count):
def get_tree_count(tree, path):
path = normalize_xpath(path)
return len(tree.findall(path)) == count
return len(tree.findall(path))

def stderr(*args):
print(*args, file=sys.stderr)
Expand Down Expand Up @@ -393,7 +393,10 @@ def check_command(c, cache):

elif c.cmd == 'count': # count test
if len(c.args) == 3: # @count <path> <pat> <count> = count test
ret = check_tree_count(cache.get_tree(c.args[0]), c.args[1], int(c.args[2]))
expected = int(c.args[2])
found = get_tree_count(cache.get_tree(c.args[0]), c.args[1])
cerr = "Expected {} occurrences but found {}".format(expected, found)
ret = expected == found
else:
raise InvalidCheck('Invalid number of @{} arguments'.format(c.cmd))
elif c.cmd == 'valid-html':
Expand Down
62 changes: 38 additions & 24 deletions src/librustdoc/html/render.rs
Original file line number Diff line number Diff line change
Expand Up @@ -258,6 +258,8 @@ pub struct Cache {
parent_stack: Vec<DefId>,
parent_is_trait_impl: bool,
search_index: Vec<IndexItem>,
seen_modules: HashSet<DefId>,
seen_mod: bool,
stripped_mod: bool,
deref_trait_did: Option<DefId>,

Expand Down Expand Up @@ -520,6 +522,8 @@ pub fn run(mut krate: clean::Crate,
parent_is_trait_impl: false,
extern_locations: HashMap::new(),
primitive_locations: HashMap::new(),
seen_modules: HashSet::new(),
seen_mod: false,
stripped_mod: false,
access_levels: krate.access_levels.clone(),
orphan_methods: Vec::new(),
Expand Down Expand Up @@ -976,13 +980,20 @@ impl DocFolder for Cache {
// we don't want it or its children in the search index.
let orig_stripped_mod = match item.inner {
clean::StrippedItem(box clean::ModuleItem(..)) => {
let prev = self.stripped_mod;
self.stripped_mod = true;
prev
mem::replace(&mut self.stripped_mod, true)
}
_ => self.stripped_mod,
};

// Inlining can cause us to visit the same item multiple times.
// (i.e. relevant for gathering impls and implementors)
let orig_seen_mod = if item.is_mod() {
let seen_this = self.seen_mod || !self.seen_modules.insert(item.def_id);
mem::replace(&mut self.seen_mod, seen_this)
} else {
self.seen_mod
};

// Register any generics to their corresponding string. This is used
// when pretty-printing types
match item.inner {
Expand All @@ -998,20 +1009,22 @@ impl DocFolder for Cache {
_ => {}
}

// Propagate a trait methods' documentation to all implementors of the
// trait
if let clean::TraitItem(ref t) = item.inner {
self.traits.insert(item.def_id, t.clone());
}
if !self.seen_mod {
// Propagate a trait methods' documentation to all implementors of the
// trait
if let clean::TraitItem(ref t) = item.inner {
self.traits.insert(item.def_id, t.clone());
}

// Collect all the implementors of traits.
if let clean::ImplItem(ref i) = item.inner {
if let Some(did) = i.trait_.def_id() {
self.implementors.entry(did).or_insert(vec![]).push(Implementor {
def_id: item.def_id,
stability: item.stability.clone(),
impl_: i.clone(),
});
// Collect all the implementors of traits.
if let clean::ImplItem(ref i) = item.inner {
if let Some(did) = i.trait_.def_id() {
self.implementors.entry(did).or_insert(vec![]).push(Implementor {
def_id: item.def_id,
stability: item.stability.clone(),
impl_: i.clone(),
});
}
}
}

Expand Down Expand Up @@ -1183,7 +1196,6 @@ impl DocFolder for Cache {
} => {
Some(did)
}

ref t => {
t.primitive_type().and_then(|t| {
self.primitive_locations.get(&t).map(|n| {
Expand All @@ -1193,13 +1205,14 @@ impl DocFolder for Cache {
})
}
};

if let Some(did) = did {
self.impls.entry(did).or_insert(vec![]).push(Impl {
impl_: i,
dox: attrs.value("doc").map(|s|s.to_owned()),
stability: item.stability.clone(),
});
if !self.seen_mod {
if let Some(did) = did {
self.impls.entry(did).or_insert(vec![]).push(Impl {
impl_: i,
dox: attrs.value("doc").map(|s|s.to_owned()),
stability: item.stability.clone(),
});
}
}
None
} else {
Expand All @@ -1209,6 +1222,7 @@ impl DocFolder for Cache {

if pushed { self.stack.pop().unwrap(); }
if parent_pushed { self.parent_stack.pop().unwrap(); }
self.seen_mod = orig_seen_mod;
self.stripped_mod = orig_stripped_mod;
self.parent_is_trait_impl = orig_parent_is_trait_impl;
return ret;
Expand Down
22 changes: 22 additions & 0 deletions src/test/rustdoc/duplicate_impls/impls.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
// Copyright 2016 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 <LICENSE-APACHE or
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
// option. This file may not be copied, modified, or distributed
// except according to those terms.

pub struct Foo;

// just so that `Foo` doesn't show up on `Bar`s sidebar
pub mod bar {
pub trait Bar {}
}

impl Foo {
pub fn new() -> Foo { Foo }
}

impl bar::Bar for Foo {}
21 changes: 21 additions & 0 deletions src/test/rustdoc/duplicate_impls/issue-33054.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
// Copyright 2016 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 <LICENSE-APACHE or
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
// option. This file may not be copied, modified, or distributed
// except according to those terms.

// @has issue_33054/impls/struct.Foo.html
// @has - '//code' 'impl Foo'
// @has - '//code' 'impl Bar for Foo'
// @count - '//*[@class="impl"]' 2
// @has issue_33054/impls/bar/trait.Bar.html
// @has - '//code' 'impl Bar for Foo'
// @count - '//*[@class="struct"]' 1
pub mod impls;

#[doc(inline)]
pub use impls as impls2;

0 comments on commit 8d0dd78

Please sign in to comment.