Skip to content

Commit

Permalink
Auto merge of #84030 - jyn514:no-blanket-impls, r=GuillaumeGomez
Browse files Browse the repository at this point in the history
rustdoc: Don't generate blanket impls when running --show-coverage

`get_blanket_impls` is the slowest part of rustdoc, and the coverage pass
completely ignores blanket impls. This stops running it at all, and also
removes some unnecessary checks in `calculate_doc_coverage` that ignored
the impl anyway.

We don't currently measure --show-coverage in perf.rlo, but I tested
this locally on cargo and it brought the time down from 2.9 to 1.6
seconds.

This also adds back a commented-out test; Rustdoc has been able to deal with `impl trait` for almost a year now.

r? `@GuillaumeGomez`
  • Loading branch information
bors committed Apr 9, 2021
2 parents bc66b92 + f67103b commit e43c200
Show file tree
Hide file tree
Showing 4 changed files with 16 additions and 53 deletions.
50 changes: 7 additions & 43 deletions src/librustdoc/passes/calculate_doc_coverage.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ use crate::passes::Pass;
use rustc_lint::builtin::MISSING_DOCS;
use rustc_middle::lint::LintLevelSource;
use rustc_session::lint;
use rustc_span::symbol::sym;
use rustc_span::FileName;
use serde::Serialize;

Expand Down Expand Up @@ -193,48 +192,13 @@ impl<'a, 'b> fold::DocFolder for CoverageCalculator<'a, 'b> {
// don't count items in stripped modules
return Some(i);
}
clean::ImportItem(..) | clean::ExternCrateItem { .. } => {
// docs on `use` and `extern crate` statements are not displayed, so they're not
// worth counting
return Some(i);
}
clean::ImplItem(ref impl_)
if i.attrs
.other_attrs
.iter()
.any(|item| item.has_name(sym::automatically_derived))
|| impl_.synthetic
|| impl_.blanket_impl.is_some() =>
{
// built-in derives get the `#[automatically_derived]` attribute, and
// synthetic/blanket impls are made up by rustdoc and can't be documented
// FIXME(misdreavus): need to also find items that came out of a derive macro
return Some(i);
}
clean::ImplItem(ref impl_) => {
let filename = i.span.filename(self.ctx.sess());
if let Some(ref tr) = impl_.trait_ {
debug!(
"impl {:#} for {:#} in {}",
tr.print(&self.ctx.cache, self.ctx.tcx),
impl_.for_.print(&self.ctx.cache, self.ctx.tcx),
filename,
);

// don't count trait impls, the missing-docs lint doesn't so we shouldn't
// either
return Some(i);
} else {
// inherent impls *can* be documented, and those docs show up, but in most
// cases it doesn't make sense, as all methods on a type are in one single
// impl block
debug!(
"impl {:#} in {}",
impl_.for_.print(&self.ctx.cache, self.ctx.tcx),
filename
);
}
}
// docs on `use` and `extern crate` statements are not displayed, so they're not
// worth counting
clean::ImportItem(..) | clean::ExternCrateItem { .. } => {}
// Don't count trait impls, the missing-docs lint doesn't so we shouldn't either.
// Inherent impls *can* be documented, and those docs show up, but in most cases it
// doesn't make sense, as all methods on a type are in one single impl block
clean::ImplItem(_) => {}
_ => {
let has_docs = !i.attrs.doc_strings.is_empty();
let mut tests = Tests { found_tests: 0 };
Expand Down
1 change: 0 additions & 1 deletion src/librustdoc/passes/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,6 @@ crate const DEFAULT_PASSES: &[ConditionalPass] = &[

/// The list of default passes run when `--doc-coverage` is passed to rustdoc.
crate const COVERAGE_PASSES: &[ConditionalPass] = &[
ConditionalPass::always(COLLECT_TRAIT_IMPLS),
ConditionalPass::new(STRIP_HIDDEN, WhenNotDocumentHidden),
ConditionalPass::new(STRIP_PRIVATE, WhenNotDocumentPrivate),
ConditionalPass::always(CALCULATE_DOC_COVERAGE),
Expand Down
14 changes: 7 additions & 7 deletions src/test/rustdoc-ui/coverage/traits.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
// check-pass

#![feature(trait_alias)]
#![feature(min_type_alias_impl_trait)]

/// look at this trait right here
pub trait ThisTrait {
Expand All @@ -16,6 +17,7 @@ pub trait ThisTrait {
}

/// so what happens if we take some struct...
#[derive(Clone)]
pub struct SomeStruct;

/// ...and slap this trait on it?
Expand All @@ -29,10 +31,8 @@ impl ThisTrait for SomeStruct {
/// but what about those aliases? i hear they're pretty exotic
pub trait MyAlias = ThisTrait + Send + Sync;

// FIXME(58624): once rustdoc can process opaque `impl Trait` types,
// we need to make sure they're counted
// /// woah, getting all opaque in here
// pub type ThisExists = impl ThisTrait;
//
// /// why don't we get a little more concrete
// pub fn defines() -> ThisExists { SomeStruct {} }
/// woah, getting all opaque in here
pub type ThisExists = impl ThisTrait;

/// why don't we get a little more concrete
pub fn defines() -> ThisExists { SomeStruct {} }
4 changes: 2 additions & 2 deletions src/test/rustdoc-ui/coverage/traits.stdout
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
+-------------------------------------+------------+------------+------------+------------+
| File | Documented | Percentage | Examples | Percentage |
+-------------------------------------+------------+------------+------------+------------+
| ...st/rustdoc-ui/coverage/traits.rs | 6 | 85.7% | 0 | 0.0% |
| ...st/rustdoc-ui/coverage/traits.rs | 8 | 88.9% | 0 | 0.0% |
+-------------------------------------+------------+------------+------------+------------+
| Total | 6 | 85.7% | 0 | 0.0% |
| Total | 8 | 88.9% | 0 | 0.0% |
+-------------------------------------+------------+------------+------------+------------+

0 comments on commit e43c200

Please sign in to comment.