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

ICE on rustdoc-json with re-export due to ID collision #80664

Closed
aDotInTheVoid opened this issue Jan 3, 2021 · 7 comments · Fixed by #83055
Closed

ICE on rustdoc-json with re-export due to ID collision #80664

aDotInTheVoid opened this issue Jan 3, 2021 · 7 comments · Fixed by #83055
Labels
A-rustdoc-json Area: Rustdoc JSON backend C-bug Category: This is a bug. glacier ICE tracked in rust-lang/glacier. I-ICE Issue: The compiler panicked, giving an Internal Compilation Error (ICE) ❄️ T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue.

Comments

@aDotInTheVoid
Copy link
Member

Code

mod inner {
    pub struct Public;
}
pub use inner::Public as Reexported;

Meta

Invoke with rustdoc broke.rs --output-format json

rustdoc --version --verbose:

rustdoc 1.51.0-nightly (fde692739 2021-01-02)
binary: rustdoc
commit-hash: fde692739576089729885b7f79aa2232cb9778c5
commit-date: 2021-01-02
host: x86_64-apple-darwin
release: 1.51.0-nightly

Error output

thread 'rustc' panicked at 'assertion failed: `(left == right)`
  left: `Item { id: Id("0:4"), crate_id: 0, name: Some("Reexported"), source: Some(Span { filename: "broke.rs", begin: (2, 4), end: (2, 22) }), visibility: Public, docs: "", links: {}, attrs: [], deprecation: None, kind: Struct, inner: StructItem(Struct { struct_type: Unit, generics: Generics { params: [], where_predicates: [] }, fields_stripped: false, fields: [], impls: [Id("0:9"), Id("0:10"), Id("0:11"), Id("0:13"), Id("0:14")] }) }`,
 right: `Item { id: Id("0:4"), crate_id: 0, name: Some("Public"), source: Some(Span { filename: "broke.rs", begin: (2, 4), end: (2, 22) }), visibility: Public, docs: "", links: {}, attrs: [], deprecation: None, kind: Struct, inner: StructItem(Struct { struct_type: Unit, generics: Generics { params: [], where_predicates: [] }, fields_stripped: false, fields: [], impls: [Id("0:9"), Id("0:10"), Id("0:11"), Id("0:13"), Id("0:14")] }) }`', src/librustdoc/json/mod.rs:167:17
Backtrace

RUST_BACKTRACE=1 rustdoc broke.rs --output-format json
thread 'rustc' panicked at 'assertion failed: `(left == right)`
  left: `Item { id: Id("0:4"), crate_id: 0, name: Some("Reexported"), source: Some(Span { filename: "broke.rs", begin: (2, 4), end: (2, 22) }), visibility: Public, docs: "", links: {}, attrs: [], deprecation: None, kind: Struct, inner: StructItem(Struct { struct_type: Unit, generics: Generics { params: [], where_predicates: [] }, fields_stripped: false, fields: [], impls: [Id("0:9"), Id("0:10"), Id("0:11"), Id("0:13"), Id("0:14")] }) }`,
 right: `Item { id: Id("0:4"), crate_id: 0, name: Some("Public"), source: Some(Span { filename: "broke.rs", begin: (2, 4), end: (2, 22) }), visibility: Public, docs: "", links: {}, attrs: [], deprecation: None, kind: Struct, inner: StructItem(Struct { struct_type: Unit, generics: Generics { params: [], where_predicates: [] }, fields_stripped: false, fields: [], impls: [Id("0:9"), Id("0:10"), Id("0:11"), Id("0:13"), Id("0:14")] }) }`', src/librustdoc/json/mod.rs:167:17
stack backtrace:
   0: _rust_begin_unwind
   1: core::panicking::panic_fmt
   2: <rustdoc::json::JsonRenderer as rustdoc::formats::renderer::FormatRenderer>::item
   3: rustdoc::formats::renderer::run_format
   4: rustdoc::run_renderer
   5: rustc_session::utils::<impl rustc_session::session::Session>::time
   6: rustc_interface::passes::QueryContext::enter
   7: rustc_interface::queries::<impl rustc_interface::interface::Compiler>::enter
   8: rustc_span::with_source_map
   9: rustc_interface::interface::create_compiler_and_run
  10: rustdoc::main_options
  11: scoped_tls::ScopedKey<T>::set
note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace.

error: internal compiler error: unexpected panic

error: Unrecognized option: 'output-format'

Working variants

mod inner { pub struct Public; }
use inner::Public as Reexported;
json
{
    "root": "0:0",
    "crate_version": null,
    "includes_private": false,
    "index": {
        "0:0": {
            "id": "0:0",
            "inner": {
                "is_crate": true,
                "items": []
            },
            "kind": "module",
            "name": "work1",
            "visibility": "public"
        }
    },
    "paths": {
        "0:0": {
            "crate_id": 0,
            "path": ["work1"],
            "kind": "module"
        }
    },
    "external_crates": {},
    "format_version": 1
}
mod inner { pub struct Public; }
pub use inner::Public;
json
{
    "root": "0:0",
    "crate_version": null,
    "includes_private": false,
    "index": {
        "0:0": {
            "id": "0:0",
            "inner": {
                "is_crate": true,
                "items": ["0:4"]
            },
            "kind": "module",
            "name": "work2",
            "visibility": "public"
        },
        "0:4": {
            "id": "0:4",
            "inner": {
                "struct_type": "unit",
                "generics": {
                    "params": [],
                    "where_predicates": []
                },
                "fields_stripped": false,
                "fields": [],
                "impls": ["0:9", "0:10", "0:11", "0:13", "0:14"]
            },
            "kind": "struct",
            "name": "Public",
            "visibility": "public"
        }
    },
    "paths": {
        "0:4": {
            "crate_id": 0,
            "path": ["work2", "Public"],
            "kind": "struct"
        },
        "0:0": {
            "crate_id": 0,
            "path": ["work2"],
            "kind": "module"
        }
    },
    "external_crates": {},
    "format_version": 1
}
pub mod inner { pub struct Public; }
pub use inner::Public as Reexported;
json
{
    "root": "0:0",
    "crate_version": null,
    "includes_private": false,
    "index": {
        "0:6": {
            "id": "0:6",
            "inner": {
                "span": "inner::Public",
                "name": "Reexported",
                "id": "0:4",
                "glob": false
            },
            "kind": "import",
            "name": null,
            "visibility": "public"
        },
        "0:3": {
            "id": "0:3",
            "inner": {
                "is_crate": false,
                "items": ["0:4"]
            },
            "kind": "module",
            "name": "inner",
            "visibility": "public"
        },
        "0:0": {
            "id": "0:0",
            "inner": {
                "is_crate": true,
                "items": ["0:6", "0:3"]
            },
            "kind": "module",
            "name": "work3",
            "visibility": "public"
        },
        "0:4": {
            "id": "0:4",
            "inner": {
                "struct_type": "unit",
                "generics": {
                    "params": [],
                    "where_predicates": []
                },
                "fields_stripped": false,
                "fields": [],
                "impls": ["0:9", "0:10", "0:11", "0:13", "0:14"]
            },
            "kind": "struct",
            "name": "Public",
            "visibility": "public"
        }
    },
    "paths": {
        "0:4": {
            "crate_id": 0,
            "path": ["work3", "inner", "Public"],
            "kind": "struct"
        },
        "0:3": {
            "crate_id": 0,
            "path": ["work3", "inner"],
            "kind": "module"
        },
        "0:0": {
            "crate_id": 0,
            "path": ["work3"],
            "kind": "module"
        }
    },
    "external_crates": {},
    "format_version": 1
}

Whats happening

The panic is here

let removed = self.index.borrow_mut().insert(id.into(), new_item.clone());
// FIXME(adotinthevoid): Currently, the index is duplicated. This is a sanity check
// to make sure the items are unique.
if let Some(old_item) = removed {
assert_eq!(old_item, new_item);
}
}

inner::Public is being added to the index when is shouldn't (I think), so when Reexported is added, the IDs colide, and we bail.

I think the solution is that inner::Public should never have been added into the index.

@aDotInTheVoid aDotInTheVoid added C-bug Category: This is a bug. I-ICE Issue: The compiler panicked, giving an Internal Compilation Error (ICE) ❄️ T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Jan 3, 2021
@aDotInTheVoid
Copy link
Member Author

aDotInTheVoid commented Jan 3, 2021

@rustbot modify labels: -T-compiler +T-rustdoc

@rustbot rustbot added T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. and removed T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Jan 3, 2021
@jyn514
Copy link
Member

jyn514 commented Jan 3, 2021

I still think the right fix for this is for JSON to have its own IDs that are separate from DefIds. The main issue is that you want the functionality of both HirIds and DefIds, basically, and some things with the same DefId have different HirIds. You can't just use HirIds because they're only available for the current crate; you wouldn't be able to serialize things from other crates.

@aDotInTheVoid
Copy link
Member Author

@rustbot modify labels +A-rustdoc-json

@rustbot rustbot added the A-rustdoc-json Area: Rustdoc JSON backend label Jan 24, 2021
@rust-lang-glacier-bot rust-lang-glacier-bot added the glacier ICE tracked in rust-lang/glacier. label Feb 5, 2021
JohnTitor added a commit to JohnTitor/rust that referenced this issue Mar 11, 2021
…ftSpider

Rustdoc Json: Add tests for Reexports, and improve jsondocck

The two changes are orthognal, so you can land just one if you want, but the improved errors realy helped write the tests.

Notably does not have the case from rust-lang#80664, but I want to have all the ajacent cases tested before starting work on that to ensure I dont break anything.

Improves rust-lang#81359

cc `@CraftSpider`

r? `@jyn514`

`@rustbot` modify labels: +A-testsuite +T-rustdoc +A-rustdoc-json
JohnTitor added a commit to JohnTitor/rust that referenced this issue Mar 11, 2021
…ftSpider

Rustdoc Json: Add tests for Reexports, and improve jsondocck

The two changes are orthognal, so you can land just one if you want, but the improved errors realy helped write the tests.

Notably does not have the case from rust-lang#80664, but I want to have all the ajacent cases tested before starting work on that to ensure I dont break anything.

Improves rust-lang#81359

cc ``@CraftSpider``

r? ``@jyn514``

``@rustbot`` modify labels: +A-testsuite +T-rustdoc +A-rustdoc-json
JohnTitor added a commit to JohnTitor/rust that referenced this issue Mar 11, 2021
…ftSpider

Rustdoc Json: Add tests for Reexports, and improve jsondocck

The two changes are orthognal, so you can land just one if you want, but the improved errors realy helped write the tests.

Notably does not have the case from rust-lang#80664, but I want to have all the ajacent cases tested before starting work on that to ensure I dont break anything.

Improves rust-lang#81359

cc ```@CraftSpider```

r? ```@jyn514```

```@rustbot``` modify labels: +A-testsuite +T-rustdoc +A-rustdoc-json
@aDotInTheVoid
Copy link
Member Author

No, it's not an ID problem, as Public shouldn't be in in index in the first place. The problem is

let module = match *item.kind {
clean::StrippedItem(box clean::ModuleItem(m)) | clean::ModuleItem(m) => m,
_ => unreachable!(),
};
for it in module.items {
debug!("Adding {:?} to worklist", it.name);
work.push((cx.make_child_renderer(), it));
}

where we currently render the children of stripped items. Changing this fixes the problem for json (gives expected output for this test case, and all existing test cases pass), but breaks the html is ways I do not understand.

Specificly these tests fail

failures:
    [rustdoc] rustdoc/inline_local/glob-extern.rs
    [rustdoc] rustdoc/inline_local/glob-private.rs
    [rustdoc] rustdoc/redirect-const.rs
    [rustdoc] rustdoc/redirect-rename.rs
    [rustdoc] rustdoc/redirect.rs

Full log

The fix is to add an item to FormatRenderer to controll if clean::StrippedItem gets called

@jyn514
Copy link
Member

jyn514 commented Mar 22, 2021

No, it's not an ID problem, as Public shouldn't be in in index in the first place. The problem is where we currently render the children of stripped items. Changing this fixes the problem for json (gives expected output for this test case, and all existing test cases pass), but breaks the html is ways I do not understand.

I looked into why the HTML still documents the stripped items - AFAICT it doesn't seem to be necessary as long as the item is inlined, and all doc(no_inline) does when the module is private is to remove the link altogether:
image

The file being generated in the private module is just a redirect to the inlined file. It looks like this was added in 7ec6df5 (part of #14513) because rustdoc doesn't know inlining decisions it made in other crates 🤦 presumably it needs the HIR available or something like that. I don't think this should affect JSON at all because it doesn't have links or any concept of inlining, right? The primary key is just the DefId?

Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this issue Mar 25, 2021
…doc, r=jyn514

[rustdoc] Don't document stripped items in JSON renderer.

Fixes rust-lang#80664, see [my comment there](rust-lang#80664 (comment)) for why

Note that we already do something similar in `convert_item`:

https://github.com/rust-lang/rust/blob/bb4cdf8ec034dca5c056ec9295f38062e5b7e871/src/librustdoc/json/conversions.rs#L28-L31

`@rustbot` modify labels: +T-rustdoc +A-rustdoc-json

r? `@jyn514`
cc `@CraftSpider`
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this issue Mar 26, 2021
…doc, r=jyn514

[rustdoc] Don't document stripped items in JSON renderer.

Fixes rust-lang#80664, see [my comment there](rust-lang#80664 (comment)) for why

Note that we already do something similar in `convert_item`:

https://github.com/rust-lang/rust/blob/bb4cdf8ec034dca5c056ec9295f38062e5b7e871/src/librustdoc/json/conversions.rs#L28-L31

``@rustbot`` modify labels: +T-rustdoc +A-rustdoc-json

r? ``@jyn514``
cc ``@CraftSpider``
@bors bors closed this as completed in 9ba9297 Mar 26, 2021
@euclio
Copy link
Contributor

euclio commented Apr 17, 2021

This assertion is still triggered if you try to document libcore with JSON output.

$ RUSTDOCFLAGS="--output-format json" ./x.py doc
Updating only changed submodules
Submodules updated in 0.02 seconds
    Finished dev [unoptimized + debuginfo] target(s) in 0.08s
Generating unstable book md files (x86_64-unknown-linux-gnu)
Building stage0 tool unstable-book-gen (x86_64-unknown-linux-gnu)
    Finished release [optimized] target(s) in 0.08s
Building stage0 tool rustbook (x86_64-unknown-linux-gnu)
    Finished release [optimized] target(s) in 0.09s
Rustbook (x86_64-unknown-linux-gnu) - unstable-book
Documenting standalone (x86_64-unknown-linux-gnu)
Documenting book redirect pages (x86_64-unknown-linux-gnu)
Documenting stage0 std (x86_64-unknown-linux-gnu)
Building stage0 std artifacts (x86_64-unknown-linux-gnu -> x86_64-unknown-linux-gnu)
    Finished release [optimized] target(s) in 0.08s
Copying stage0 std from stage0 (x86_64-unknown-linux-gnu -> x86_64-unknown-linux-gnu / x86_64-unknown-linux-gnu)
 Documenting core v0.0.0 (/home/euclio/repos/rust/library/core)
thread 'rustc' panicked at 'assertion failed: `(left == right)`
  left: `Item { id: Id("0:2436"), crate_id: 0, name: Some("borrow"), source: Some(Span { filename: "library/core/src/borrow.rs", begin: (210, 4), end: (212, 5) }), visibility: Public, docs: None, links: {}, attrs: ["#[rustc_diagnostic_item = \"noop_method_borrow\"]"], deprecation: None, inner: Method(Method { decl: FnDecl { inputs: [("", BorrowedRef { lifetime: None, mutable: false, type_: Generic("Self") })], output: Some(BorrowedRef { lifetime: None, mutable: false, type_: Generic("T") }), c_variadic: false }, generics: Generics { params: [], where_predicates: [] }, header: {}, abi: "\"Rust\"", has_body: true }) }`,
 right: `Item { id: Id("0:2436"), crate_id: 0, name: Some("borrow"), source: Some(Span { filename: "library/core/src/borrow.rs", begin: (210, 4), end: (212, 5) }), visibility: Default, docs: None, links: {}, attrs: ["#[rustc_diagnostic_item = \"noop_method_borrow\"]"], deprecation: None, inner: Method(Method { decl: FnDecl { inputs: [("self", BorrowedRef { lifetime: None, mutable: false, type_: Generic("Self") })], output: Some(BorrowedRef { lifetime: None, mutable: false, type_: Generic("T") }), c_variadic: false }, generics: Generics { params: [], where_predicates: [] }, header: {}, abi: "\"Rust\"", has_body: true }) }`', src/librustdoc/json/mod.rs:175:17
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

error: internal compiler error: unexpected panic

error: Unrecognized option: 'markdown-css'

error: could not document `core`

Caused by:
  process didn't exit successfully: `/home/euclio/repos/rust/build/bootstrap/debug/rustdoc --edition=2018 --crate-type lib --crate-name core library/core/src/lib.rs --target x86_64-unknown-linux-gnu -o /home/euclio/repos/rust/build/x86_64-unknown-linux-gnu/stage0-std/x86_64-unknown-linux-gnu/doc --error-format=json --json=diagnostic-rendered-ansi --markdown-css rust.css --markdown-no-toc -Z unstable-options --resource-suffix 1.53.0 --index-page /home/euclio/repos/rust/src/doc/index.md -L dependency=/home/euclio/repos/rust/build/x86_64-unknown-linux-gnu/stage0-std/x86_64-unknown-linux-gnu/release/deps -L dependency=/home/euclio/repos/rust/build/x86_64-unknown-linux-gnu/stage0-std/release/deps --cfg=bootstrap --output-format json -Dwarnings '-Wrustdoc::invalid_codeblock_attributes' --crate-version 1.53.0-dev` (exit code: 1)


command did not execute successfully: "/home/euclio/repos/rust/build/x86_64-unknown-linux-gnu/stage0/bin/cargo" "rustdoc" "--target" "x86_64-unknown-linux-gnu" "-Zbinary-dep-depinfo" "-j" "24" "--release" "--features" "panic-unwind backtrace compiler-builtins-c" "--manifest-path" "/home/euclio/repos/rust/library/test/Cargo.toml" "-p" "core" "--" "--markdown-css" "rust.css" "--markdown-no-toc" "-Z" "unstable-options" "--resource-suffix" "1.53.0" "--index-page" "/home/euclio/repos/rust/src/doc/index.md"
expected success, got: exit code: 101


failed to run: /home/euclio/repos/rust/build/bootstrap/debug/bootstrap doc

@jyn514
Copy link
Member

jyn514 commented Apr 17, 2021

@euclio that looks like #83718

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-rustdoc-json Area: Rustdoc JSON backend C-bug Category: This is a bug. glacier ICE tracked in rust-lang/glacier. I-ICE Issue: The compiler panicked, giving an Internal Compilation Error (ICE) ❄️ T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants