rustdoc only: duplicate lang item `panic_halt` #68427

TimNN · 2020-01-21T21:05:34Z

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)

The text was updated successfully, but these errors were encountered:

Mark-Simulacrum · 2020-01-21T21:08:33Z

cc @rust-lang/rustdoc

GuillaumeGomez · 2020-01-21T22:31:05Z

I'll try to take a look in the next days.

pietroalbini · 2020-01-24T11:44:07Z

Note that the stable release will be prepared on monday, so the patch should be merged by then.

ollie27 · 2020-01-24T12:33:37Z

It looks like this was triggered by #66211 (cc. @kinnison). rustdoc now seems to load all crates passed by --extern even if they're unused in the code whereas rustc just ignores them. Adding extern crate panic_halt; or use panic_halt; to panic2 causes 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-halt but 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 · 2020-01-24T12:38:56Z

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 · 2020-01-25T08:47:37Z

If there's a way to get the metadata into rustdoc without fully loading the --extern crates then that'd satisfy rustdoc'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 · 2020-01-25T20:11:17Z

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 · 2020-01-25T22:24:02Z

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 · 2020-01-26T11:36:29Z

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 Resolver data is frozen (to_resolver_outputs is called), so rustdoc should organize its passes accordingly.

petrochenkov · 2020-01-26T11:48:47Z

In other words, if you remove Resolver::clone_outputs while keeping rustdoc working, it will fix this issue and some other issues as well.

GuillaumeGomez · 2020-01-26T15:53:49Z

I'll need more information because none of the Resolver methods you listed are used currently in rustdoc (or at least not directly). Also, how do we resolve those paths if resolving them triggers a panic?

petrochenkov · 2020-01-26T16:30:41Z

The methods are not in rustdoc, but rustdoc is their only user and they exist due to it.
Path resolution won't panic if it is used before the Resolver data is frozen, as I said in a previous comment.

So, rustdoc should work similarly to rustc and organize its passes in such a way that they perform name resolution early.
I don't know how exactly this needs to be done, since I'm not a rustdoc maintainer.

eddyb · 2020-01-26T17:53:28Z

@petrochenkov My understanding is that rustdoc works on the HIR, collects doc comments, and only then attempts to resolve paths in the markdown syntax of doc comments.

Maybe it should have a (much simpler) AST pass that only looks for paths in markdown links in #[doc = ...] attributes, and forces rustc_resolve to load the relevant crates.

I don't think it's possible to fully push rustdoc to before AST->HIR lowering.

GuillaumeGomez · 2020-01-26T20:13:56Z

@petrochenkov My question was more: "how do we do that?". I know very little about the rustc internals generally.

jyn514 · 2020-08-14T02:13:57Z

This is now blocking #75176.

jyn514 · 2020-08-14T02:26:36Z

Instead of Resolver having its own copy of cstore, is it possible to pass one in and have Resolver use that instead? I think TyCtxt::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 · 2020-08-14T02:27:29Z

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 · 2020-08-14T04:59:25Z

I already mentioned my preferred solution:

So, rustdoc should work similarly to rustc and organize its passes in such a way that they perform name resolution early.

+ eddyb's comment - #68427 (comment)

…rochenkov rustdoc: Don't load all extern crates unconditionally Instead, only load the crates that are linked to with intra-doc links. This doesn't help very much with any of rustdoc's fundamental issues with freezing the resolver, but it at least fixes a stable-to-stable regression, and makes the crate loading model somewhat more consistent with rustc's. I tested and it unfortunately does not help at all with rust-lang#82496. Closes rust-lang#68427. Let me know if you want me to open a separate issue for not freezing the resolver. r? `@petrochenkov` cc `@eddyb` `@ollie27`

…rochenkov rustdoc: Don't load all extern crates unconditionally Instead, only load the crates that are linked to with intra-doc links. This doesn't help very much with any of rustdoc's fundamental issues with freezing the resolver, but it at least fixes a stable-to-stable regression, and makes the crate loading model somewhat more consistent with rustc's. I tested and it unfortunately does not help at all with rust-lang#82496. Closes rust-lang#68427. Let me know if you want me to open a separate issue for not freezing the resolver. r? ``@petrochenkov`` cc ``@eddyb`` ``@ollie27``

…crate-load, r=jyn514 Revert "Don't load all extern crates unconditionally" Fixes rust-lang#84738. This reverts rust-lang#83738. For the "smart" load of external crates, we need to be able to access their items in order to check their doc comments, which seems, if not impossible, quite complicated using only the AST. For some context, I first tried to extend the `IntraLinkCrateLoader` visitor by adding `visit_foreign_item`. Unfortunately, it never enters into this call, so definitely not the right place... I then added `visit_use_tree` to then check all the imports outside with something like this: ```rust let mut loader = crate::passes::collect_intra_doc_links::IntraLinkCrateLoader::new(resolver); ast::visit::walk_crate(&mut loader, krate); let mut items = Vec::new(); for import in &loader.imports_to_check { if let Some(item) = krate.items.iter().find(|i| i.id == *import) { items.push(item); } } for item in items { ast::visit::walk_item(&mut item); for attr in &item.attrs { loader.check_attribute(attr); } } ``` This was, of course, a failure. We find the items without problems, but we still can't go into the external crate to check its items' attributes. Finally, `@jyn514` suggested to look into the [`CrateLoader`](https://doc.rust-lang.org/nightly/nightly-rustc/rustc_metadata/creader/struct.CrateLoader.html), but it only seems to provide metadata (I went through [`CStore`](https://doc.rust-lang.org/nightly/nightly-rustc/rustc_metadata/creader/struct.CStore.html) and [`CrateMetadata`](https://doc.rust-lang.org/nightly/nightly-rustc/rustc_metadata/rmeta/decoder/struct.CrateMetadata.html)). I think we are too limited here (with AST only) to be able to determine the crates we actually need to import, but it's very likely that I missed something. Maybe `@petrochenkov` or `@Aaron1011` have an idea? So until we find a way to make it work completely, we need to revert it to fix the ICE. Once merged, we'll need to re-open rust-lang#68427. r? `@jyn514`

jyn514 · 2021-07-04T22:20:27Z

#83738 was reverted in #85749.

Mark-Simulacrum added regression-from-stable-to-beta Performance or correctness regression from stable to beta. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. labels Jan 21, 2020

GuillaumeGomez self-assigned this Jan 21, 2020

JohnTitor added A-lang-item Area: Language items C-bug Category: This is a bug. labels Jan 26, 2020

pietroalbini added regression-from-stable-to-stable Performance or correctness regression from one stable version to another. and removed regression-from-stable-to-beta Performance or correctness regression from stable to beta. labels Jan 27, 2020

jyn514 mentioned this issue Aug 14, 2020

Fix intra-doc links for cross-crate re-exports of default trait methods #75176

Merged

jyn514 mentioned this issue Aug 25, 2020

rustdoc panics with Unable to resolve external crate #72381

Closed

jyn514 mentioned this issue Nov 29, 2020

Doc links should count as a "use" with respect to the unused_imports lint #79542

Closed

jyn514 unassigned GuillaumeGomez Dec 12, 2020

jyn514 added the A-intra-doc-links Area: Intra-doc links, the ability to link to items in docs by name label Dec 12, 2020

jyn514 added the A-resolve Area: Name resolution label Dec 12, 2020

Nadrieril mentioned this issue Dec 12, 2020

Can't document compiler crates with --stage 0 #79980

Closed

petrochenkov mentioned this issue Feb 26, 2021

Fix impl being removed excessively #82496

Closed

jyn514 mentioned this issue Mar 19, 2021

Rust docs broken microsoft/windows-rs#614

Closed

This was referenced Apr 1, 2021

rustdoc: Don't load all extern crates unconditionally #83738

Merged

rustdoc: Stop cloning the resolver #83761

Closed

bors closed this as completed in 640ce99 Apr 3, 2021

jyn514 mentioned this issue May 3, 2021

Index out of bounds when running cargo doc in rustc_metadata/src/creader.rs #84738

Closed

GuillaumeGomez mentioned this issue May 27, 2021

Revert "Don't load all extern crates unconditionally" #85749

Merged

jyn514 reopened this Jul 4, 2021

jyn514 mentioned this issue Aug 21, 2021

Reland #83738: "rustdoc: Don't load all extern crates unconditionally" #88215

Merged

bors closed this as completed in 14fb87a Aug 27, 2021

jyn514 mentioned this issue Nov 5, 2021

rustdoc: Go back to loading all external crates unconditionally #90489

Merged

tgross35 mentioned this issue Feb 11, 2023

cargo doc fails on found duplicate lang item 'panic_impl' dependency issue (no_std) #107918

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

rustdoc only: duplicate lang item `panic_halt` #68427

rustdoc only: duplicate lang item `panic_halt` #68427

TimNN commented Jan 21, 2020

Mark-Simulacrum commented Jan 21, 2020

GuillaumeGomez commented Jan 21, 2020

pietroalbini commented Jan 24, 2020

ollie27 commented Jan 24, 2020

TimNN commented Jan 24, 2020

kinnison commented Jan 25, 2020 •

edited

Loading

GuillaumeGomez commented Jan 25, 2020

GuillaumeGomez commented Jan 25, 2020 •

edited

Loading

petrochenkov commented Jan 26, 2020

petrochenkov commented Jan 26, 2020

GuillaumeGomez commented Jan 26, 2020

petrochenkov commented Jan 26, 2020 •

edited

Loading

eddyb commented Jan 26, 2020

GuillaumeGomez commented Jan 26, 2020

jyn514 commented Aug 14, 2020

jyn514 commented Aug 14, 2020

jyn514 commented Aug 14, 2020

petrochenkov commented Aug 14, 2020

jyn514 commented Jul 4, 2021

rustdoc only: duplicate lang item panic_halt #68427

rustdoc only: duplicate lang item panic_halt #68427

Comments

TimNN commented Jan 21, 2020

Mark-Simulacrum commented Jan 21, 2020

GuillaumeGomez commented Jan 21, 2020

pietroalbini commented Jan 24, 2020

ollie27 commented Jan 24, 2020

TimNN commented Jan 24, 2020

kinnison commented Jan 25, 2020 • edited Loading

GuillaumeGomez commented Jan 25, 2020

GuillaumeGomez commented Jan 25, 2020 • edited Loading

petrochenkov commented Jan 26, 2020

petrochenkov commented Jan 26, 2020

GuillaumeGomez commented Jan 26, 2020

petrochenkov commented Jan 26, 2020 • edited Loading

eddyb commented Jan 26, 2020

GuillaumeGomez commented Jan 26, 2020

jyn514 commented Aug 14, 2020

jyn514 commented Aug 14, 2020

jyn514 commented Aug 14, 2020

petrochenkov commented Aug 14, 2020

jyn514 commented Jul 4, 2021

rustdoc only: duplicate lang item `panic_halt` #68427

rustdoc only: duplicate lang item `panic_halt` #68427

kinnison commented Jan 25, 2020 •

edited

Loading

GuillaumeGomez commented Jan 25, 2020 •

edited

Loading

petrochenkov commented Jan 26, 2020 •

edited

Loading