rust-lang / rust

Empowering everyone to build reliable and efficient software.
https://www.rust-lang.org
Other
99.12k stars 12.8k forks source link

ICE with rustdoc json backend #93588

Closed weiznich closed 2 years ago

weiznich commented 2 years ago

Code

https://github.com/diesel-rs/diesel/commit/41404e6344cfce91234cbfdc6513a8634abee59a using the following command:

RUSTDOCFLAGS='-Z unstable-options --output-format=json' cargo +nightly doc --features "postgres"

produces a internal compiler error inside of rustdoc

Meta

rustdoc +nightly --version --verbose:

rustdoc 1.60.0-nightly (6abb6385b 2022-01-26)
binary: rustdoc
commit-hash: 6abb6385b2cb7249f67b9b3ce7522527767dd907
commit-date: 2022-01-26
host: x86_64-unknown-linux-gnu
release: 1.60.0-nightly
LLVM version: 13.0.0

Error output

thread 'rustc' panicked at 'assertion failed: `(left == right)`
  left: `Item { id: Id("0:23812"), crate_id: 0, name: Some("PgMoney"), span: Some(Span { filename: "diesel/src/pg/types/money.rs", begin: (23, 0), end: (23, 28) }), visibility: Public, docs: Some("Money is represented in Postgres as a 64 bit signed integer.  This struct is a dumb wrapper\ntype, meant only to indicate the integer's meaning.  The fractional precision of the value is\ndetermined by the [`lc_monetary` setting of the database](https://www.postgresql.org/docs/9.6/static/datatype-money.html).\nThis struct is re-exported as `Cents` as a convenient and conventional expression of a typical\nunit of 1/100th of currency. For other names or precisions, users might consider a differently\nnamed `use` of the `PgMoney` struct.\n\n```rust\nuse diesel::data_types::PgMoney as Pence; // 1/100th unit of Pound\nuse diesel::data_types::PgMoney as Fils;  // 1/1000th unit of Dinar\n```"), links: {}, attrs: ["#[diesel(sql_type = Money)]"], deprecation: None, inner: Struct(Struct { struct_type: Tuple, generics: Generics { params: [], where_predicates: [] }, fields_stripped: false, fields: [Id("0:23814")], impls: [Id("a:2:8549-0:23812"), Id("a:2:3022-0:23812"), Id("a:2:8548-0:23812"), Id("a:2:3034-0:23812"), Id("a:2:3063-0:23812"), Id("b:0:542-0:23812"), Id("b:0:559-0:23812"), Id("b:0:549-0:23812"), Id("b:0:1745-0:23812"), Id("b:2:3736-0:23812"), Id("b:2:2951-0:23812"), Id("b:2:2630-0:23812"), Id("b:2:2966-0:23812"), Id("b:2:2955-0:23812"), Id("b:2:2627-0:23812"), Id("b:2:2961-0:23812"), Id("b:5:667-0:23812"), Id("0:23726"), Id("0:23728"), Id("0:23730"), Id("0:23731"), Id("0:23732"), Id("0:23735"), Id("0:23736"), Id("0:23738"), Id("0:23740"), Id("0:23765"), Id("0:23769"), Id("0:23783"), Id("0:23787"), Id("0:23790"), Id("0:23807"), Id("0:8373"), Id("0:8375"), Id("0:8378"), Id("0:8381"), Id("0:8383"), Id("0:8386")] }) }`,
 right: `Item { id: Id("0:23812"), crate_id: 0, name: Some("Cents"), span: Some(Span { filename: "diesel/src/pg/types/money.rs", begin: (23, 0), end: (23, 28) }), visibility: Public, docs: Some("Money is represented in Postgres as a 64 bit signed integer.  This struct is a dumb wrapper\ntype, meant only to indicate the integer's meaning.  The fractional precision of the value is\ndetermined by the [`lc_monetary` setting of the database](https://www.postgresql.org/docs/9.6/static/datatype-money.html).\nThis struct is re-exported as `Cents` as a convenient and conventional expression of a typical\nunit of 1/100th of currency. For other names or precisions, users might consider a differently\nnamed `use` of the `PgMoney` struct.\n\n```rust\nuse diesel::data_types::PgMoney as Pence; // 1/100th unit of Pound\nuse diesel::data_types::PgMoney as Fils;  // 1/1000th unit of Dinar\n```"), links: {}, attrs: ["#[diesel(sql_type = Money)]"], deprecation: None, inner: Struct(Struct { struct_type: Tuple, generics: Generics { params: [], where_predicates: [] }, fields_stripped: false, fields: [Id("0:23814")], impls: [Id("a:2:8549-0:23812"), Id("a:2:3022-0:23812"), Id("a:2:8548-0:23812"), Id("a:2:3034-0:23812"), Id("a:2:3063-0:23812"), Id("b:0:542-0:23812"), Id("b:0:559-0:23812"), Id("b:0:549-0:23812"), Id("b:0:1745-0:23812"), Id("b:2:3736-0:23812"), Id("b:2:2951-0:23812"), Id("b:2:2630-0:23812"), Id("b:2:2966-0:23812"), Id("b:2:2955-0:23812"), Id("b:2:2627-0:23812"), Id("b:2:2961-0:23812"), Id("b:5:667-0:23812"), Id("0:23726"), Id("0:23728"), Id("0:23730"), Id("0:23731"), Id("0:23732"), Id("0:23735"), Id("0:23736"), Id("0:23738"), Id("0:23740"), Id("0:23765"), Id("0:23769"), Id("0:23783"), Id("0:23787"), Id("0:23790"), Id("0:23807"), Id("0:8373"), Id("0:8375"), Id("0:8378"), Id("0:8381"), Id("0:8383"), Id("0:8386")] }) }`', src/librustdoc/json/mod.rs:195:17
Backtrace

``` stack backtrace: 0: 0x7fdbf675869c - std::backtrace_rs::backtrace::libunwind::trace::h9c95184963fef745 at /rustc/6abb6385b2cb7249f67b9b3ce7522527767dd907/library/std/src/../../backtrace/src/backtrace/libunwind.rs:93:5 1: 0x7fdbf675869c - std::backtrace_rs::backtrace::trace_unsynchronized::h5d21f8fdfc262588 at /rustc/6abb6385b2cb7249f67b9b3ce7522527767dd907/library/std/src/../../backtrace/src/backtrace/mod.rs:66:5 2: 0x7fdbf675869c - std::sys_common::backtrace::_print_fmt::h6ce732896d3b7bc2 at /rustc/6abb6385b2cb7249f67b9b3ce7522527767dd907/library/std/src/sys_common/backtrace.rs:67:5 3: 0x7fdbf675869c - ::fmt::h56a1b948079bb60c at /rustc/6abb6385b2cb7249f67b9b3ce7522527767dd907/library/std/src/sys_common/backtrace.rs:46:22 4: 0x7fdbf67b987c - core::fmt::write::ha787ab3b9068d913 at /rustc/6abb6385b2cb7249f67b9b3ce7522527767dd907/library/core/src/fmt/mod.rs:1168:17 5: 0x7fdbf6747d43 - std::io::Write::write_fmt::h4913c71f24efdd75 at /rustc/6abb6385b2cb7249f67b9b3ce7522527767dd907/library/std/src/io/mod.rs:1653:15 6: 0x7fdbf675cac2 - std::sys_common::backtrace::_print::h8ba799ac9853e240 at /rustc/6abb6385b2cb7249f67b9b3ce7522527767dd907/library/std/src/sys_common/backtrace.rs:49:5 7: 0x7fdbf675cac2 - std::sys_common::backtrace::print::hc1060288e0e5dcfe at /rustc/6abb6385b2cb7249f67b9b3ce7522527767dd907/library/std/src/sys_common/backtrace.rs:36:9 8: 0x7fdbf675cac2 - std::panicking::default_hook::{{closure}}::h5623b336a8b38b2c at /rustc/6abb6385b2cb7249f67b9b3ce7522527767dd907/library/std/src/panicking.rs:290:50 9: 0x7fdbf675c6a5 - std::panicking::default_hook::h7f9b0932f349911f at /rustc/6abb6385b2cb7249f67b9b3ce7522527767dd907/library/std/src/panicking.rs:307:9 10: 0x7fdbf6f39f41 - rustc_driver[d4f1704680c1e064]::DEFAULT_HOOK::{closure#0}::{closure#0} 11: 0x7fdbe79723f3 - as core::ops::function::Fn>::call::h0599b93be78093ff at /rustc/6abb6385b2cb7249f67b9b3ce7522527767dd907/library/alloc/src/boxed.rs:1868:9 12: 0x7fdbe79aa70c - proc_macro::bridge::client::::enter::{{closure}}::{{closure}}::h45db41ffd4e35ab1 at /rustc/6abb6385b2cb7249f67b9b3ce7522527767dd907/library/proc_macro/src/bridge/client.rs:319:21 13: 0x7fdbe795a920 - std::panicking::update_hook::{{closure}}::hfc35d8f8dfcea4c0 at /rustc/6abb6385b2cb7249f67b9b3ce7522527767dd907/library/std/src/panicking.rs:257:41 14: 0x7fdbf675d2db - std::panicking::rust_panic_with_hook::h87cab73890184b28 at /rustc/6abb6385b2cb7249f67b9b3ce7522527767dd907/library/std/src/panicking.rs:695:17 15: 0x7fdbf675cf97 - std::panicking::begin_panic_handler::{{closure}}::h541a05bf9df15ade at /rustc/6abb6385b2cb7249f67b9b3ce7522527767dd907/library/std/src/panicking.rs:581:13 16: 0x7fdbf6758b44 - std::sys_common::backtrace::__rust_end_short_backtrace::h18b7b64c8e3dc986 at /rustc/6abb6385b2cb7249f67b9b3ce7522527767dd907/library/std/src/sys_common/backtrace.rs:139:18 17: 0x7fdbf675cca9 - rust_begin_unwind at /rustc/6abb6385b2cb7249f67b9b3ce7522527767dd907/library/std/src/panicking.rs:577:5 18: 0x7fdbf67242e3 - core::panicking::panic_fmt::h1720bda5e8269e46 at /rustc/6abb6385b2cb7249f67b9b3ce7522527767dd907/library/core/src/panicking.rs:135:14 19: 0x7fdbf67b67b8 - core::panicking::assert_failed_inner::h449b64e1f2903a2a 20: 0x55fbaa6bbccb - core[66374c829b964992]::panicking::assert_failed:: 21: 0x55fbaa97d885 - ::item 22: 0x55fbaa978af9 - ::item 23: 0x55fbaa978af9 - ::item 24: 0x55fbaa978af9 - ::item 25: 0x55fbaa99bf7b - rustdoc[3bca669b999d636e]::formats::renderer::run_format:: 26: 0x55fbaa6dc798 - rustdoc[3bca669b999d636e]::run_renderer:: 27: 0x55fbaa6f7b4b - ::time::, rustdoc[3bca669b999d636e]::main_options::{closure#0}::{closure#0}::{closure#1}::{closure#2}> 28: 0x55fbaa893885 - ::enter::> 29: 0x55fbaa91a1cc - rustc_span[342a111d7a8110f2]::with_source_map::, rustc_interface[7eb630a73ce93593]::interface::create_compiler_and_run, rustdoc[3bca669b999d636e]::main_options::{closure#0}>::{closure#1}> 30: 0x55fbaa886277 - rustc_interface[7eb630a73ce93593]::interface::create_compiler_and_run::, rustdoc[3bca669b999d636e]::main_options::{closure#0}> 31: 0x55fbaa6de0de - rustdoc[3bca669b999d636e]::main_options 32: 0x55fbaa6f604d - >::set::>::{closure#0}::{closure#0}, core[66374c829b964992]::result::Result<(), rustc_errors[86224a19c1a30ce1]::ErrorReported>> 33: 0x55fbaa8c4b59 - std[ecfd9b74f7001338]::sys_common::backtrace::__rust_begin_short_backtrace::>::{closure#0}, core[66374c829b964992]::result::Result<(), rustc_errors[86224a19c1a30ce1]::ErrorReported>> 34: 0x55fbaa6f5839 - <::spawn_unchecked_>::{closure#0}, core[66374c829b964992]::result::Result<(), rustc_errors[86224a19c1a30ce1]::ErrorReported>>::{closure#1} as core[66374c829b964992]::ops::function::FnOnce<()>>::call_once::{shim:vtable#0} 35: 0x7fdbf6768e83 - as core::ops::function::FnOnce>::call_once::h055a74f58759b602 at /rustc/6abb6385b2cb7249f67b9b3ce7522527767dd907/library/alloc/src/boxed.rs:1854:9 36: 0x7fdbf6768e83 - as core::ops::function::FnOnce>::call_once::hb194029d87e03ea5 at /rustc/6abb6385b2cb7249f67b9b3ce7522527767dd907/library/alloc/src/boxed.rs:1854:9 37: 0x7fdbf6768e83 - std::sys::unix::thread::Thread::new::thread_start::h79b5483319eb1ce5 at /rustc/6abb6385b2cb7249f67b9b3ce7522527767dd907/library/std/src/sys/unix/thread.rs:108:17 38: 0x7fdbf63f5927 - start_thread at ./nptl/./nptl/pthread_create.c:435:8 39: 0x7fdbf64859e4 - __clone at ./misc/../sysdeps/unix/sysv/linux/x86_64/clone.S:100 40: 0x0 - error: internal compiler error: unexpected panic ```

CraftSpider commented 2 years ago

This looks like a duplicate of #83720, and as such should be fixed by #93518

weiznich commented 2 years ago

Any news on this? I keep hitting this (with different types) on the current nightly (rustdoc 1.62.0-nightly (34a6c9f26 2022-04-13))

Enselic commented 2 years ago

I don't think this is a duplicate of either #83720 or #83057, because making all modules public and removing all #[doc(inline)] attributes does not get rid of the ICE. This is what I did to try this:

git clone https://github.com/diesel-rs/diesel.git && cd diesel # commit 983209a61 for me

# Make all modules public
git ls-files | grep '\.rs' | xargs perl -pi -e 's/^ *mod /pub mod /'

# Undo partially to fix : error: `proc-macro` crate types currently cannot export any items other than functions tagged with `#[proc_macro]`
git restore diesel_derives/src/lib.rs diesel_migrations/migrations_macros/src/lib.rs 

# Remove all #[doc(inline)]
git ls-files | grep '\.rs$' | xargs perl -pi -e 's/.*#\[doc\(inline\)\]//'

but I still get the same ICE as I get before my changes (in my case it is row_metadata that has duplicate items).

I have tried to debug this further, but no luck so far. I have tried to:

Maybe someone with better knowledge of diesel could help to reduce the code in diesel to a minimal example that can be used to reproduce and debug the ICE?

Disclaimer: My methodology might be flawed, because I'm still no veteran when it comes to debugging problems like these.

Enselic commented 2 years ago

I figured out how to make rustdoc not cause an ICE when generating rustdoc JSON for the main crate diesel/diesel. Maybe this gives someone enough of a clue to figure out what is wrong?

See https://github.com/Enselic/diesel/pull/1/files for the diff that makes rustdoc JSON work. For example, I had to change

impl<T, ST> AsExpression<ST> for T
where
    T: Expression<SqlType = ST>,
    ST: SqlType + TypedExpressionType,
{
    type Expression = Self;

    fn as_expression(self) -> Self {
        self
    }
}

to

impl<T, ST> AsExpression<ST> for T
where
    T: Expression<SqlType = ST>,
    ST: SqlType + TypedExpressionType,
{
    type Expression = T;

    fn as_expression(self) -> T {
        self
    }
}

This fixes the error where one instance of the item with the same ID contains T whereas the other contains Self. But that is just one example.

I haven't yet figured out a minimal way to reproduce. When I rip out this code and put it in a minimal crate, there is no ICE. There is probably some complicated construct that is causing these problems.

CC @aDotInTheVoid who usually is interested in rustdoc JSON stuff.

aDotInTheVoid commented 2 years ago

Started a minification at https://github.com/aDotInTheVoid/diesel-93588, not made much progress.

Enselic commented 2 years ago

I just wanted to add an observation. One of causes of the ICEs is that the typedef item corresponding to type Expression = Self (see https://github.com/rust-lang/rust/issues/93588#issuecomment-1138690037) also can be represented as type Expression = T, but ends up having the same ID. But, in this context, T and Self is equivalent! So failing on the assert_eq!(old_item, new_item) in src/librustdoc/json/mod.rs doesn't make sense semantically. Because the items are in fact the same.

So to fix this class of ICE problems, it should work to normalise the representation so that the two instance of the same typedef item is the same. This "trick" should in fact fix all cases of ICE in diesel/diesel that I pinpoint with https://github.com/Enselic/diesel/pull/1/files. Because in all cases it is about the same item having different but equivalent representations.

Probably easier said than done to actually implement though... But maybe the code for this already exists for other uses somewhere in the compiler.

Enselic commented 2 years ago

Heads-up if you are debugging this via diesel: The project is (understandably) working around this bug by making local changes (see e.g. https://github.com/diesel-rs/diesel/pull/3190 which was just merged), so to reproduce this problem with diesel, you need to remain on a commit before these changes. For example ecd22fbe38.

Enselic commented 2 years ago

@rustbot label -T-compiler +T-rustdoc

Enselic commented 2 years ago

I found at least one variant of a minimal reproducer: https://github.com/rust-lang/rust/issues/97986

Not sure solving that will solve all of the ICEs we see with diesel though. I actually used https://github.com/serde-rs/serde/tree/master/serde to find this reproducer, because serde suffers from this problem too.

Enselic commented 2 years ago

I found another minimal reproducer based on the diesel code base: https://github.com/rust-lang/rust/issues/98547

With the (proposed) fix for that issue included in a rustdoc build, I get no ICE anymore with diesel/diesel, neither with https://github.com/diesel-rs/diesel/commit/41404e6344cfce91234cbfdc6513a8634abee59a nor with https://github.com/diesel-rs/diesel/commit/ecd22fbe3.

Enselic commented 2 years ago

With latest nightly I do not get any ICEs any longer.

So I think we can close this issue now, right?

Enselic commented 2 years ago

Regarding the E-needs-test label: I'm not sure what more tests to write? All relevant ICE fixes has already added regression tests to the best of my knowledge. Notably:

aDotInTheVoid commented 2 years ago

Yeah, I think this can be closed