From 8ab2c20d8c018c310a0c286aeb2622c41ef357a1 Mon Sep 17 00:00:00 2001 From: mitaa Date: Fri, 22 Apr 2016 17:53:15 +0200 Subject: [PATCH] Only record the same impl once Due to inlining it is possible to visit the same module multiple times during `::fold_crate`, so we keep track of the modules we've already visited. --- src/etc/htmldocck.py | 9 ++- src/librustdoc/html/render.rs | 62 ++++++++++++------- src/test/rustdoc/duplicate_impls/impls.rs | 22 +++++++ .../rustdoc/duplicate_impls/issue-33054.rs | 21 +++++++ 4 files changed, 87 insertions(+), 27 deletions(-) create mode 100644 src/test/rustdoc/duplicate_impls/impls.rs create mode 100644 src/test/rustdoc/duplicate_impls/issue-33054.rs diff --git a/src/etc/htmldocck.py b/src/etc/htmldocck.py index 8362c239b655d..a930a0d083367 100644 --- a/src/etc/htmldocck.py +++ b/src/etc/htmldocck.py @@ -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) @@ -393,7 +393,10 @@ def check_command(c, cache): elif c.cmd == 'count': # count test if len(c.args) == 3: # @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': diff --git a/src/librustdoc/html/render.rs b/src/librustdoc/html/render.rs index c08d917589d3c..824265bc3b303 100644 --- a/src/librustdoc/html/render.rs +++ b/src/librustdoc/html/render.rs @@ -258,6 +258,8 @@ pub struct Cache { parent_stack: Vec, parent_is_trait_impl: bool, search_index: Vec, + seen_modules: HashSet, + seen_mod: bool, stripped_mod: bool, deref_trait_did: Option, @@ -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(), @@ -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 { @@ -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(), + }); + } } } @@ -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| { @@ -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 { @@ -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; diff --git a/src/test/rustdoc/duplicate_impls/impls.rs b/src/test/rustdoc/duplicate_impls/impls.rs new file mode 100644 index 0000000000000..72e54fe733cdb --- /dev/null +++ b/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 or the MIT license +// , 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 {} diff --git a/src/test/rustdoc/duplicate_impls/issue-33054.rs b/src/test/rustdoc/duplicate_impls/issue-33054.rs new file mode 100644 index 0000000000000..df6ebcae10756 --- /dev/null +++ b/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 or the MIT license +// , 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;