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

rustdoc: don't crash when an external trait's docs needs to import another trait #48415

Merged
merged 4 commits into from
Feb 25, 2018

Conversation

QuietMisdreavus
Copy link
Member

Fixes #48414

When resolving intra-paths for an item, rustdoc needs to have information about their items on hand, for proper bookkeeping. When loading a path for an external item, it needs to load these items from their host crate, since their information isn't otherwise available. This includes resolving paths for those docs. which can cause this process to recurse. Rustdoc keeps a map of external traits in a RefCell<HashMap<DefId, Trait>>, and it keeps a borrow of this active when importing an external trait. In the linked crash, this led to a RefCell borrow error, panic, and ICE.

This PR manually releases the borrow while importing the trait, and also keeps a list of traits being imported at the given moment. The latter keeps rustdoc from infinitely recursing as it tries to import the same trait repeatedly.

@rust-highfive
Copy link
Collaborator

r? @GuillaumeGomez

(rust_highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Feb 22, 2018
@Manishearth
Copy link
Member

r=me but needs a test

@Manishearth
Copy link
Member

The test I had was

pub trait Foo {
  /// [x](::Bar)
 fn foo();
}
pub trait Bar {
  /// [x](::Foo)
  fn foo();
}

and then

pub use aux::Foo;
pub use aux::Bar;

@Manishearth
Copy link
Member

I have a test at https://github.com/manishearth/rust/tree/rustdoc-trait-ice , but haven't run it yet

@QuietMisdreavus
Copy link
Member Author

The test i wrote up was:

/// Woah, this trait links to [OtherTrait](OtherTrait)!
pub trait SomeTrait {}

/// Woah, this trait links to [SomeTrait](SomeTrait)!
pub trait OtherTrait {}
#[doc(inline)]
pub use issue_48414::{SomeTrait, OtherTrait};

...which still stack-overflows. >_> Need to investigate.

@Manishearth
Copy link
Member

wooooooooooo

@QuietMisdreavus
Copy link
Member Author

It turns out, i wrote a && when i needed to have written a ||. 😓

@Manishearth
Copy link
Member

@bors r+

@bors
Copy link
Contributor

bors commented Feb 22, 2018

📌 Commit 8872e7b has been approved by Manishearth

@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-review Status: Awaiting review from the assignee but also interested parties. labels Feb 22, 2018
Manishearth added a commit to Manishearth/rust that referenced this pull request Feb 24, 2018
…-traits, r=Manishearth

rustdoc: don't crash when an external trait's docs needs to import another trait

Fixes rust-lang#48414

When resolving intra-paths for an item, rustdoc needs to have information about their items on hand, for proper bookkeeping. When loading a path for an external item, it needs to load these items from their host crate, since their information isn't otherwise available. This includes resolving paths for those docs. which can cause this process to recurse. Rustdoc keeps a map of external traits in a `RefCell<HashMap<DefId, Trait>>`, and it keeps a borrow of this active when importing an external trait. In the linked crash, this led to a RefCell borrow error, panic, and ICE.

This PR manually releases the borrow while importing the trait, and also keeps a list of traits being imported at the given moment. The latter keeps rustdoc from infinitely recursing as it tries to import the same trait repeatedly.
bors added a commit that referenced this pull request Feb 25, 2018
Rollup of 15 pull requests

- Successful merges: #47689, #48110, #48197, #48296, #48386, #48392, #48404, #48415, #48441, #48448, #48452, #48481, #48490, #48499, #48503
- Failed merges:
@bors bors merged commit 8872e7b into rust-lang:master Feb 25, 2018
@QuietMisdreavus QuietMisdreavus deleted the traits-on-traits-on-traits branch February 26, 2018 23:19
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.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants