Skip to content

Commit

Permalink
Auto merge of #84176 - GuillaumeGomez:src-to-definition, r=jyn514
Browse files Browse the repository at this point in the history
Generate links to definition in rustdoc source code pages

## Description

This PR adds an option (disabled by default) to add links in the source code page on ident. So for for example:

```rust
mod other_module;
struct Foo;
fn bar() {}

fn x<T: other_module::Trait>(f: Foo, g: other_module::Whatever, t: &T) {
    let f: Foo = Foo;
    bar();
    f.some_method();
}
```

In the example (mostly in the `x` function), `other_module::Trait`, `Foo`, `other_module::Whatever`, `bar` and `some_method` are now links (and `other_module` at the top too).

In case there is a type coming from another crate, it'll link to its documentation page and not its definition (but you can then click on `[src]` so I guess it's fine).

Another important detail: I voluntarily didn't add links for primitive types. I think we can discuss about adding links on them or not in a later PR (adding the support for them would require only a few lines).

Here is a video summing up everything I wrote above:

https://user-images.githubusercontent.com/3050060/114622354-21307b00-9cae-11eb-834d-f6d8178a37bd.mp4

## Performance impact

So, on my computer, the performance remains more or less the same (which is quite surprising but that's a nice surprise). Here are the numbers:

Without the option:
 * core:  1m 21s
 * alloc: 26.78s
 * std: 27.30s
 * proc_macro: 4.50s

With source to definition links generation (I enabled by default the option):
 * core: 1m 25s
 * alloc: 25.76s
 * std: 27.07s
 * proc_macro: 4.66s

So no real change here (again, I'm very surprised by this fact).

For the size of the generated source files (only taking into account the `src` folder here since it's the only one impacted) by running `du -shc .` (when I am in the source folder).

Without the option: 11.939 MB
With the option: 12.611 MB

So not a big change here either. In all those docs, I ran `grep -nR '<a class=' . | wc -l` and got 43917. So there are quite a lot of links added. :)

cc `@rust-lang/rustdoc`
r? `@jyn514`
  • Loading branch information
bors committed Aug 5, 2021
2 parents 2f07ae4 + ba11dc7 commit e2f7957
Show file tree
Hide file tree
Showing 30 changed files with 707 additions and 131 deletions.
17 changes: 16 additions & 1 deletion compiler/rustc_errors/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -342,6 +342,9 @@ struct HandlerInner {
deduplicated_warn_count: usize,

future_breakage_diagnostics: Vec<Diagnostic>,

/// If set to `true`, no warning or error will be emitted.
quiet: bool,
}

/// A key denoting where from a diagnostic was stashed.
Expand Down Expand Up @@ -456,10 +459,19 @@ impl Handler {
emitted_diagnostics: Default::default(),
stashed_diagnostics: Default::default(),
future_breakage_diagnostics: Vec::new(),
quiet: false,
}),
}
}

pub fn with_disabled_diagnostic<T, F: FnOnce() -> T>(&self, f: F) -> T {
let prev = self.inner.borrow_mut().quiet;
self.inner.borrow_mut().quiet = true;
let ret = f();
self.inner.borrow_mut().quiet = prev;
ret
}

// This is here to not allow mutation of flags;
// as of this writing it's only used in tests in librustc_middle.
pub fn can_emit_warnings(&self) -> bool {
Expand Down Expand Up @@ -818,7 +830,7 @@ impl HandlerInner {
}

fn emit_diagnostic(&mut self, diagnostic: &Diagnostic) {
if diagnostic.cancelled() {
if diagnostic.cancelled() || self.quiet {
return;
}

Expand Down Expand Up @@ -1035,6 +1047,9 @@ impl HandlerInner {
}

fn delay_as_bug(&mut self, diagnostic: Diagnostic) {
if self.quiet {
return;
}
if self.flags.report_delayed_bugs {
self.emit_diagnostic(&diagnostic);
}
Expand Down
4 changes: 4 additions & 0 deletions compiler/rustc_session/src/session.rs
Original file line number Diff line number Diff line change
Expand Up @@ -500,6 +500,10 @@ impl Session {
&self.parse_sess.span_diagnostic
}

pub fn with_disabled_diagnostic<T, F: FnOnce() -> T>(&self, f: F) -> T {
self.parse_sess.span_diagnostic.with_disabled_diagnostic(f)
}

/// Analogous to calling methods on the given `DiagnosticBuilder`, but
/// deduplicates on lint ID, span (if any), and message for this `Session`
fn diag_once<'a, 'b>(
Expand Down
2 changes: 1 addition & 1 deletion src/librustdoc/clean/blanket_impl.rs
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ impl<'a, 'tcx> BlanketImplFinder<'a, 'tcx> {
visibility: Inherited,
def_id: ItemId::Blanket { impl_id: impl_def_id, for_: item_def_id },
kind: box ImplItem(Impl {
span: Span::from_rustc_span(self.cx.tcx.def_span(impl_def_id)),
span: Span::new(self.cx.tcx.def_span(impl_def_id)),
unsafety: hir::Unsafety::Normal,
generics: (
self.cx.tcx.generics_of(impl_def_id),
Expand Down
2 changes: 1 addition & 1 deletion src/librustdoc/clean/inline.rs
Original file line number Diff line number Diff line change
Expand Up @@ -517,7 +517,7 @@ fn build_module(
}
}

let span = clean::Span::from_rustc_span(cx.tcx.def_span(did));
let span = clean::Span::new(cx.tcx.def_span(did));
clean::Module { items, span }
}

Expand Down
3 changes: 2 additions & 1 deletion src/librustdoc/clean/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,8 @@ impl Clean<Item> for doctree::Module<'_> {

// determine if we should display the inner contents or
// the outer `mod` item for the source code.
let span = Span::from_rustc_span({

let span = Span::new({
let where_outer = self.where_outer(cx.tcx);
let sm = cx.sess().source_map();
let outer = sm.lookup_char_pos(where_outer.lo());
Expand Down
11 changes: 6 additions & 5 deletions src/librustdoc/clean/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -343,7 +343,7 @@ crate struct Item {
rustc_data_structures::static_assert_size!(Item, 56);

crate fn rustc_span(def_id: DefId, tcx: TyCtxt<'_>) -> Span {
Span::from_rustc_span(def_id.as_local().map_or_else(
Span::new(def_id.as_local().map_or_else(
|| tcx.def_span(def_id),
|local| {
let hir = tcx.hir();
Expand Down Expand Up @@ -1943,10 +1943,11 @@ crate enum Variant {
crate struct Span(rustc_span::Span);

impl Span {
crate fn from_rustc_span(sp: rustc_span::Span) -> Self {
// Get the macro invocation instead of the definition,
// in case the span is result of a macro expansion.
// (See rust-lang/rust#39726)
/// Wraps a [`rustc_span::Span`]. In case this span is the result of a macro expansion, the
/// span will be updated to point to the macro invocation instead of the macro definition.
///
/// (See rust-lang/rust#39726)
crate fn new(sp: rustc_span::Span) -> Self {
Self(sp.source_callsite())
}

Expand Down
12 changes: 12 additions & 0 deletions src/librustdoc/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -276,6 +276,8 @@ crate struct RenderOptions {
crate show_type_layout: bool,
crate unstable_features: rustc_feature::UnstableFeatures,
crate emit: Vec<EmitType>,
/// If `true`, HTML source pages will generate links for items to their definition.
crate generate_link_to_definition: bool,
}

#[derive(Copy, Clone, Debug, PartialEq, Eq)]
Expand Down Expand Up @@ -655,6 +657,15 @@ impl Options {
let generate_redirect_map = matches.opt_present("generate-redirect-map");
let show_type_layout = matches.opt_present("show-type-layout");
let nocapture = matches.opt_present("nocapture");
let generate_link_to_definition = matches.opt_present("generate-link-to-definition");

if generate_link_to_definition && (show_coverage || output_format != OutputFormat::Html) {
diag.struct_err(
"--generate-link-to-definition option can only be used with HTML output format",
)
.emit();
return Err(1);
}

let (lint_opts, describe_lints, lint_cap) =
get_cmd_lint_options(matches, error_format, &debugging_opts);
Expand Down Expand Up @@ -721,6 +732,7 @@ impl Options {
crate_name.as_deref(),
),
emit,
generate_link_to_definition,
},
crate_name,
output_format,
Expand Down
18 changes: 17 additions & 1 deletion src/librustdoc/html/format.rs
Original file line number Diff line number Diff line change
Expand Up @@ -484,7 +484,11 @@ crate enum HrefError {
NotInExternalCache,
}

crate fn href(did: DefId, cx: &Context<'_>) -> Result<(String, ItemType, Vec<String>), HrefError> {
crate fn href_with_root_path(
did: DefId,
cx: &Context<'_>,
root_path: Option<&str>,
) -> Result<(String, ItemType, Vec<String>), HrefError> {
let cache = &cx.cache();
let relative_to = &cx.current;
fn to_module_fqp(shortty: ItemType, fqp: &[String]) -> &[String] {
Expand All @@ -495,6 +499,7 @@ crate fn href(did: DefId, cx: &Context<'_>) -> Result<(String, ItemType, Vec<Str
return Err(HrefError::Private);
}

let mut is_remote = false;
let (fqp, shortty, mut url_parts) = match cache.paths.get(&did) {
Some(&(ref fqp, shortty)) => (fqp, shortty, {
let module_fqp = to_module_fqp(shortty, fqp);
Expand All @@ -508,6 +513,7 @@ crate fn href(did: DefId, cx: &Context<'_>) -> Result<(String, ItemType, Vec<Str
shortty,
match cache.extern_locations[&did.krate] {
ExternalLocation::Remote(ref s) => {
is_remote = true;
let s = s.trim_end_matches('/');
let mut s = vec![s];
s.extend(module_fqp[..].iter().map(String::as_str));
Expand All @@ -522,6 +528,12 @@ crate fn href(did: DefId, cx: &Context<'_>) -> Result<(String, ItemType, Vec<Str
}
}
};
if !is_remote {
if let Some(root_path) = root_path {
let root = root_path.trim_end_matches('/');
url_parts.insert(0, root);
}
}
let last = &fqp.last().unwrap()[..];
let filename;
match shortty {
Expand All @@ -536,6 +548,10 @@ crate fn href(did: DefId, cx: &Context<'_>) -> Result<(String, ItemType, Vec<Str
Ok((url_parts.join("/"), shortty, fqp.to_vec()))
}

crate fn href(did: DefId, cx: &Context<'_>) -> Result<(String, ItemType, Vec<String>), HrefError> {
href_with_root_path(did, cx, None)
}

/// Both paths should only be modules.
/// This is because modules get their own directories; that is, `std::vec` and `std::vec::Vec` will
/// both need `../iter/trait.Iterator.html` to get at the iterator trait.
Expand Down
Loading

0 comments on commit e2f7957

Please sign in to comment.