Skip to content

Commit

Permalink
Only record the same impl once
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
mitaa committed Apr 22, 2016
1 parent a264f5b commit 8ab2c20
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
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
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
@@ -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
@@ -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 8ab2c20

Please sign in to comment.