Repository navigation
Labels
A-intra-doc-linksArea: Intra-doc links, the ability to link to items in docs by nameArea: Intra-doc links, the ability to link to items in docs by nameA-lang-itemArea: Language itemsArea: Language itemsA-resolveArea: Name/path resolution done by `rustc_resolve` specificallyArea: Name/path resolution done by `rustc_resolve` specificallyC-bugCategory: This is a bug.Category: This is a bug.T-rustdocRelevant to the rustdoc team, which will review and decide on the PR/issue.Relevant to the rustdoc team, which will review and decide on the PR/issue.regression-from-stable-to-stablePerformance or correctness regression from one stable version to another.Performance or correctness regression from one stable version to another.
Description
This is a regression from rustdoc 1.40.0 (73528e339 2019-12-16) to rustdoc 1.41.0-beta.3 (fb3056216 2020-01-19).
If rustdoc attempts to document a crate that has at least two dependencies which implement a panic_handler, it fails with an error similar to
error: duplicate lang item in crate `panic_halt` (which `panic2` depends on): `panic_impl`.
|
= note: first defined in crate `std` (which `panic2` depends on).
Note that the crate builds fine, it's just documenting that is broken.
Steps to reproduce
- Create a new project, e.g.
cargo new panic2 - Add a dependency on
panic-halt = "0.2.0", e.g.cargo add panic-halt - Attempt to document the crate, e.g.
cargo doc
An example of the different behavior between `stable`/`beta` and `build`/`doc`.
$ cargo +stable clean
$ cargo +beta clean
$ cargo +stable build
Compiling panic-halt v0.2.0
Compiling panic2 v0.1.0 (/Users/logic/Projects/tmp/panic2)
Finished dev [unoptimized + debuginfo] target(s) in 0.35s
$ cargo +stable doc
Documenting panic-halt v0.2.0
Checking panic-halt v0.2.0
Documenting panic2 v0.1.0 (/Users/logic/Projects/tmp/panic2)
Finished dev [unoptimized + debuginfo] target(s) in 1.40s
$ cargo +beta build
Compiling panic-halt v0.2.0
Compiling panic2 v0.1.0 (/Users/logic/Projects/tmp/panic2)
Finished dev [unoptimized + debuginfo] target(s) in 0.27s
$ cargo +beta doc
Documenting panic-halt v0.2.0
Checking panic-halt v0.2.0
Documenting panic2 v0.1.0 (/Users/logic/Projects/tmp/panic2)
error: duplicate lang item in crate `panic_halt` (which `panic2` depends on): `panic_impl`.
|
= note: first defined in crate `std` (which `panic2` depends on).
error: aborting due to previous error
error: Could not document `panic2`.
Caused by:
process didn't exit successfully: `rustdoc --edition=2018 --crate-type bin --crate-name panic2 src/main.rs -o /Users/logic/Projects/tmp/panic2/target/doc --error-format=json --json=diagnostic-rendered-ansi --document-private-items -L dependency=/Users/logic/Projects/tmp/panic2/target/debug/deps --extern panic_halt=/Users/logic/Projects/tmp/panic2/target/debug/deps/libpanic_halt-6d0ca720c3c355fb.rmeta` (exit code: 1)Metadata
Metadata
Assignees
Labels
A-intra-doc-linksArea: Intra-doc links, the ability to link to items in docs by nameArea: Intra-doc links, the ability to link to items in docs by nameA-lang-itemArea: Language itemsArea: Language itemsA-resolveArea: Name/path resolution done by `rustc_resolve` specificallyArea: Name/path resolution done by `rustc_resolve` specificallyC-bugCategory: This is a bug.Category: This is a bug.T-rustdocRelevant to the rustdoc team, which will review and decide on the PR/issue.Relevant to the rustdoc team, which will review and decide on the PR/issue.regression-from-stable-to-stablePerformance or correctness regression from one stable version to another.Performance or correctness regression from one stable version to another.
Activity
Mark-Simulacrum commentedon Jan 21, 2020
cc @rust-lang/rustdoc
GuillaumeGomez commentedon Jan 21, 2020
I'll try to take a look in the next days.
emilyalbini commentedon Jan 24, 2020
Note that the stable release will be prepared on monday, so the patch should be merged by then.
ollie27 commentedon Jan 24, 2020
It looks like this was triggered by #66211 (cc. @kinnison). rustdoc now seems to load all crates passed by
--externeven if they're unused in the code whereas rustc just ignores them. Addingextern crate panic_halt;oruse panic_halt;topanic2causes stable rustdoc and rustc to produce the same error message.Is this issue affecting any real world code? It seems weird to list a dependency on
panic-haltbut not actually use it.#66211 was to fix an ICE using the intra-doc-links feature but that's only available on nightly so it should be safe to revert #66211 on beta for now if needed.
TimNN commentedon Jan 24, 2020
I guess it is unlikely that this will be triggered in many real world scenarios. I came across this problem because I was experimenting with embedded programming: I had multiple panic implementations in the dependencies and then multiple binaries which were using different panic implementations (to see their different behaviors).
kinnison commentedon Jan 25, 2020
If there's a way to get the metadata into
rustdocwithout fully loading the--externcrates then that'd satisfyrustdoc's usecase without causing this. I do not object to backing this out of beta so long as there's some idea of how we'd fix it without entirely backing it out and reintroducing the ICE. @GuillaumeGomez how do you feel about that?GuillaumeGomez commentedon Jan 25, 2020
We can revert it on the beta to prevent the crash but we definitely need a way to get the metadata from the crates.
GuillaumeGomez commentedon Jan 25, 2020
So after a discussion with @eddyb, it appears that @petrochenkov is one of the last person working on this part.
cc @petrochenkov (at @eddyb's suggestion)
petrochenkov commentedon Jan 26, 2020
If you don't want to resolve all crates passed with
--extern(with which I agree), then resolve paths in intra-doc links, the crates will be used on demand then.All the paths need to be resolved before the
Resolverdata is frozen (to_resolver_outputsis called), sorustdocshould organize its passes accordingly.petrochenkov commentedon Jan 26, 2020
In other words, if you remove
Resolver::clone_outputswhile keepingrustdocworking, it will fix this issue and some other issues as well.7 remaining items
jyn514 commentedon Aug 14, 2020
Instead of
Resolverhaving its own copy ofcstore, is it possible to pass one in and haveResolveruse that instead? I thinkTyCtxt::cstore()might come from the resolver in the first place so maybe it should be a public field or something so that having a cstore on hand is optional.jyn514 commentedon Aug 14, 2020
Another idea is to store the resolver in the TyCtxt itself so that it never has to be cloned in the first place: #75176 (comment)
petrochenkov commentedon Aug 14, 2020
I already mentioned my preferred solution:
+ eddyb's comment - #68427 (comment)
Rollup merge of rust-lang#83738 - jyn514:only-load-some-crates, r=pet…
Rollup merge of rust-lang#83738 - jyn514:only-load-some-crates, r=pet…
Auto merge of rust-lang#83738 - jyn514:only-load-some-crates, r=petro…
Rollup merge of rust-lang#85749 - GuillaumeGomez:revert-smart-extern-…
jyn514 commentedon Jul 4, 2021
#83738 was reverted in #85749.
Rollup merge of rust-lang#88215 - jyn514:lazy-loading, r=petrochenkov