Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove the dummy cache in DocContext; delete RenderInfo #82018

Merged
merged 3 commits into from
Mar 2, 2021

Conversation

jyn514
Copy link
Member

@jyn514 jyn514 commented Feb 12, 2021

The same information is available everywhere; the only reason the dummy
cache was needed is because it was previously stored in three different
places. This consolidates the info a bit so the cache in DocContext is
used throughout. As a bonus, it also completely removes RenderInfo.

  • Return a Cache from run_global_ctxt, not RenderInfo
  • Remove the unused render_info from run_renderer
  • Remove RenderInfo altogether

Helps with #82014. The next step is to move the populate() call before the collect_intra_doc_links pass, which currently breaks because a) lots of the cache is populated in early passes, and b) intra_doc_links itself sets some info with register_res. I'm working on separate PR for that to avoid making too many big changes at once.

r? @GuillaumeGomez

@jyn514 jyn514 added the T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. label Feb 12, 2021
@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Feb 12, 2021
@jyn514 jyn514 changed the title Remove the dummy cache in DocContext Remove the dummy cache in DocContext; delete RenderInfo Feb 12, 2021
@GuillaumeGomez
Copy link
Member

Looks great overall! Less code makes me happier. :3

@GuillaumeGomez
Copy link
Member

Wanna take a look maybe @ollie27 ?

bors added a commit to rust-lang-ci/rust that referenced this pull request Feb 19, 2021
…eGomez

Make `Clean` take &mut DocContext

- Take `FnMut` in `rustc_trait_selection::find_auto_trait_generics`
- Take `&mut DocContext` in most of `clean`
- Collect the iterator in auto_trait_impls instead of iterating lazily; the lifetimes were really bad.

This combined with rust-lang#82018 should hopefully help with rust-lang#82014 by allowing `cx.cache.exported_traits` to be modified in `register_res`. Previously it had to use interior mutability, which required either adding a RefCell to `cache.exported_traits` on *top* of the existing `RefCell<Cache>` or mixing reads and writes between `cx.exported_traits` and `cx.cache.exported_traits`. I don't currently have that working but I expect it to be reasonably easy to add after this.
@jyn514
Copy link
Member Author

jyn514 commented Feb 22, 2021

I rebased this over #82020 so I could remove the RefCell from DocContext.cache.

@bors

This comment has been minimized.

src/librustdoc/core.rs Outdated Show resolved Hide resolved
@jyn514
Copy link
Member Author

jyn514 commented Feb 23, 2021

r? @camelid

@bors

This comment has been minimized.

@bors

This comment has been minimized.

src/librustdoc/core.rs Outdated Show resolved Hide resolved
src/librustdoc/clean/inline.rs Outdated Show resolved Hide resolved
src/librustdoc/formats/cache.rs Show resolved Hide resolved
src/librustdoc/passes/strip_private.rs Show resolved Hide resolved
src/librustdoc/lib.rs Show resolved Hide resolved
src/librustdoc/core.rs Outdated Show resolved Hide resolved
src/librustdoc/core.rs Show resolved Hide resolved
@bors bors added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Feb 28, 2021
src/librustdoc/core.rs Outdated Show resolved Hide resolved
@camelid camelid added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Feb 28, 2021
The same information is available everywhere; the only reason the dummy
cache was needed is because it waas previously stored in three different
places. This consolidates the info a bit so the cache in `DocContext` is
used throughout. As a bonus, it means `renderinfo` is used much much
less.

- Return a `Cache` from `run_global_ctxt`, not `RenderInfo`
- Remove the unused `render_info` from `run_renderer`
- Remove RefCell around `inlined`
- Add intra-doc links
Previously, `JsonRenderer::after_krate` called `krate.version.clone()`.
The problem was it did that after the version was already moved into the
cache, so it would always be None. The fix was to get the version from
the cache instead.
@jyn514
Copy link
Member Author

jyn514 commented Mar 1, 2021

@bors r=camelid,GuillaumeGomez

@bors
Copy link
Contributor

bors commented Mar 1, 2021

📌 Commit be069a6 has been approved by camelid,GuillaumeGomez

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Mar 1, 2021
bors added a commit to rust-lang-ci/rust that referenced this pull request Mar 2, 2021
…laumeGomez

Rollup of 7 pull requests

Successful merges:

 - rust-lang#80734 (check that first arg to `panic!()` in const is `&str`)
 - rust-lang#81932 (Always compile rustdoc with debug logging enabled when `download-rustc` is set)
 - rust-lang#82018 (Remove the dummy cache in `DocContext`; delete RenderInfo)
 - rust-lang#82598 (Check stability and feature attributes in rustdoc)
 - rust-lang#82655 (Highlight identifier span instead of whole pattern span in `unused` lint)
 - rust-lang#82662 (Warn about unknown doc attributes)
 - rust-lang#82676 (Change twice used large const table to static)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit b51272e into rust-lang:master Mar 2, 2021
@rustbot rustbot added this to the 1.52.0 milestone Mar 2, 2021
@jyn514 jyn514 deleted the no-dummy-cache branch March 2, 2021 13:52
JohnTitor added a commit to JohnTitor/rust that referenced this pull request Mar 6, 2021
…laumeGomez

Remove RefCell around `module_trait_cache`

This builds on rust-lang#82018 and should not be merged before.

## Don't require a `DocContext` for `report_diagnostic`

This is needed for the next commit, which needs mutable access to the `cx` from
within the `decorate` closure.

- Change `as_local_hir_id` to an associated function, since it only
  needs a `TyCtxt`
- Change `source_span_for_markdown_range` to only take a `TyCtxt`

##  Remove RefCell around module_trait_cache

This is mostly just changing lots of functions from `&DocContext` to `&mut DocContext`.
JohnTitor added a commit to JohnTitor/rust that referenced this pull request Mar 6, 2021
…laumeGomez

Remove RefCell around `module_trait_cache`

This builds on rust-lang#82018 and should not be merged before.

## Don't require a `DocContext` for `report_diagnostic`

This is needed for the next commit, which needs mutable access to the `cx` from
within the `decorate` closure.

- Change `as_local_hir_id` to an associated function, since it only
  needs a `TyCtxt`
- Change `source_span_for_markdown_range` to only take a `TyCtxt`

##  Remove RefCell around module_trait_cache

This is mostly just changing lots of functions from `&DocContext` to `&mut DocContext`.
m-ou-se added a commit to m-ou-se/rust that referenced this pull request Mar 6, 2021
…laumeGomez

Remove RefCell around `module_trait_cache`

This builds on rust-lang#82018 and should not be merged before.

## Don't require a `DocContext` for `report_diagnostic`

This is needed for the next commit, which needs mutable access to the `cx` from
within the `decorate` closure.

- Change `as_local_hir_id` to an associated function, since it only
  needs a `TyCtxt`
- Change `source_span_for_markdown_range` to only take a `TyCtxt`

##  Remove RefCell around module_trait_cache

This is mostly just changing lots of functions from `&DocContext` to `&mut DocContext`.
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this pull request Mar 6, 2021
…laumeGomez

Remove RefCell around `module_trait_cache`

This builds on rust-lang#82018 and should not be merged before.

## Don't require a `DocContext` for `report_diagnostic`

This is needed for the next commit, which needs mutable access to the `cx` from
within the `decorate` closure.

- Change `as_local_hir_id` to an associated function, since it only
  needs a `TyCtxt`
- Change `source_span_for_markdown_range` to only take a `TyCtxt`

##  Remove RefCell around module_trait_cache

This is mostly just changing lots of functions from `&DocContext` to `&mut DocContext`.
JohnTitor added a commit to JohnTitor/rust that referenced this pull request Mar 7, 2021
…laumeGomez

Remove RefCell around `module_trait_cache`

This builds on rust-lang#82018 and should not be merged before.

## Don't require a `DocContext` for `report_diagnostic`

This is needed for the next commit, which needs mutable access to the `cx` from
within the `decorate` closure.

- Change `as_local_hir_id` to an associated function, since it only
  needs a `TyCtxt`
- Change `source_span_for_markdown_range` to only take a `TyCtxt`

##  Remove RefCell around module_trait_cache

This is mostly just changing lots of functions from `&DocContext` to `&mut DocContext`.
bors added a commit to rust-lang-ci/rust that referenced this pull request Mar 10, 2021
Remove `masked_crates` from `clean::Crate`

Previously, `masked_crates` existed both on `Cache` and on
`clean::Crate`. During cache population, the `clean::Crate` version was
`take`n and moved to `Cache`.

This change removes the version on `clean::Crate` and instead directly
mutates `Cache.masked_crates` to initialize it. This has the advantage
of avoiding duplication and avoiding unnecessary allocation, as well as
making the flow of information through rustdoc less confusing.

The one downside I see is that `clean::utils::krate()` now uses the side
effect of mutating `DocContext.cache` instead of returning the data
directly, but it already mutated the `Cache` for other things (e.g.,
`deref_trait_did`) so it's not really new behavior. Also,
`clean::utils::krate()` is only called once (and is meant to only be
called once since it performs expensive and potentially destructive
operations) so the mutation shouldn't be an issue.

Follow-up to rust-lang#82018 (comment).

cc `@jyn514`
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants