rust-lang / rustfmt

Format Rust code
https://rust-lang.github.io/rustfmt/
Apache License 2.0
6.05k stars 888 forks source link

cargo fmt crashes in Rust 1.81.0 #6333

Closed ajewellamz closed 3 weeks ago

ajewellamz commented 2 months ago

lib.rs.gz

Un-gzip the attached lib.rs make it the only file in a crate run cargo fmt See

thread 'main' panicked at library/core/src/slice/sort/shared/smallsort.rs:862:5:
user-provided comparison function does not correctly implement a total order
stack backtrace:
   0:        0x103e69aa3 - <std::sys::backtrace::BacktraceLock::print::DisplayBacktrace as core::fmt::Display>::fmt::haf41a54e7ed5fe07
   1:        0x103eb45cb - core::fmt::write::h526161fad96c5ad5
   2:        0x103e5f69e - std::io::Write::write_fmt::h4de0398536c8b0e4

As an aside - with previous versions of Rust, cargo fmt also failed on this file. It didn't crash, but it left trailing spaces on some lines, and then reported failure.

ytmimi commented 2 months ago

Hey @ajewellamz what exact version of cargo fmt / rustfmt are you using. You can run cargo fmt --version to get that info.

Also, any chance you can get this down to a reproducible code snippet that you can add inline? That would be best. rustfmt can run just fine on code that doesn't compile (as long as it parses) so my suggestion would be to slowly remove function, struct, enum, macro, and trait definitions / impls from the file until you're able to narrow down what's causing the issue. It would be a great help 🙏🏼

ajewellamz commented 2 months ago

rustfmt 1.7.1-stable (eeb90cda 2024-09-04)

I won't be able to do any work on this until next week at the earliest. I was assuming you could just run it in the debugger or something, if you had a fail case.

ajewellamz commented 2 months ago

Ok, here's a minimal fail case. I've tried deleting various parts, and it seems to always succeed.

pub use ::dafny_runtime::Sequence;
pub use ::std::rc::Rc;
pub use crate::r#_StructuredEncryptionUtil_Compile::CanonCryptoItem;
pub use ::dafny_runtime::itertools::Itertools;
pub use crate::software::amazon::cryptography::dbencryptionsdk::structuredencryption::internaldafny::types::CryptoAction;
pub use crate::software::amazon::cryptography::materialproviders::internaldafny::types::EncryptedDataKey;
pub use ::dafny_runtime::Object;
pub use crate::software::amazon::cryptography::primitives::internaldafny::types::IAwsCryptographicPrimitivesClient;
pub use crate::software::amazon::cryptography::materialproviders::internaldafny::types::AlgorithmSuiteInfo;
pub use ::dafny_runtime::DafnyCharUTF16;
pub use crate::r#_StructuredEncryptionUtil_Compile::MessageID;
pub use crate::software::amazon::cryptography::materialproviders::internaldafny::types::EncryptionMaterials;
pub use crate::r#_Wrappers_Compile::Outcome;
pub use ::dafny_runtime::string_utf16_of;
pub use ::dafny_runtime::int;
pub use ::dafny_runtime::MapBuilder;
pub use ::dafny_runtime::_System::nat;
pub use crate::software::amazon::cryptography::primitives::internaldafny::types::HMacInput;
pub use ::dafny_runtime::rd;
pub use ::dafny_runtime::truncate;
pub use ::dafny_runtime::DafnyTypeEq;
pub use ::dafny_runtime::DafnyType;
ytmimi commented 2 months ago

Thanks! confirming that with rustfmt +1.81 (rustfmt 1.7.1-stable (eeb90cda 2024-09-04)) I'm able to reproduce the panic with the above example:

Backtrace ``` thread 'main' panicked at library/core/src/slice/sort/shared/smallsort.rs:862:5: user-provided comparison function does not correctly implement a total order stack backtrace: 0: 0x10316373c - ::fmt::h243268f17d714c7f 1: 0x1031a6688 - core::fmt::write::hb3cfb8a30e72d7ff 2: 0x103159720 - std::io::Write::write_fmt::hfb2314975de9ecf1 3: 0x103165c4c - std::panicking::default_hook::{{closure}}::h14c7718ccf39d316 4: 0x103165870 - std::panicking::default_hook::hc62e60da3be2f352 5: 0x10cd5c5b8 - as core[f827f14b5e761a5d]::ops::function::Fn<(&dyn for<'a, 'b> core[f827f14b5e761a5d]::ops::function::Fn<(&'a std[4f7d7c3ef984657a]::panic::PanicHookInfo<'b>,), Output = ()> + core[f827f14b5e761a5d]::marker::Sync + core[f827f14b5e761a5d]::marker::Send, &std[4f7d7c3ef984657a]::panic::PanicHookInfo)>>::call 6: 0x103166868 - std::panicking::rust_panic_with_hook::h09e8a656f11e82b2 7: 0x103166150 - std::panicking::begin_panic_handler::{{closure}}::h1230eb3cc91b241c 8: 0x103163bc8 - std::sys::backtrace::__rust_end_short_backtrace::hc3491307aceda2c2 9: 0x103165e40 - _rust_begin_unwind 10: 0x1031c0ad8 - core::panicking::panic_fmt::ha4b80a05b9fff47a 11: 0x1031a8c84 - core::slice::sort::shared::smallsort::panic_on_ord_violation::h9a9b9a4ea28b52bf 12: 0x1020fed34 - core[f827f14b5e761a5d]::slice::sort::shared::smallsort::small_sort_general_with_scratch::::lt> 13: 0x1021469a4 - core[f827f14b5e761a5d]::slice::sort::stable::quicksort::quicksort::::lt> 14: 0x1021a1108 - core[f827f14b5e761a5d]::slice::sort::stable::drift::sort::::lt> 15: 0x1020bdedc - core[f827f14b5e761a5d]::slice::sort::stable::driftsort_main::::lt, alloc[47bc6d386d7ae45f]::vec::Vec> 16: 0x102192af8 - rustfmt_nightly[2afeb25615a25c73]::reorder::rewrite_reorderable_or_regroupable_items 17: 0x1021ae510 - ::visit_items_with_reordering 18: 0x1021b9dd4 - ::format_separate_mod 19: 0x102047714 - rustfmt_nightly[2afeb25615a25c73]::formatting::format_project::> 20: 0x102045adc - >::with::<>::format_input_inner::{closure#0}, core[f827f14b5e761a5d]::result::Result> 21: 0x102043bd4 - >::set::, >::format_input_inner::{closure#0}>::{closure#0}, core[f827f14b5e761a5d]::result::Result> 22: 0x102047060 - rustc_span[ab16d476329f5d04]::create_session_if_not_set_then::, >::format_input_inner::{closure#0}> 23: 0x1020325a8 - rustfmt[4ddeb7d779b6c40f]::format_and_emit_report:: 24: 0x102031c40 - rustfmt[4ddeb7d779b6c40f]::execute 25: 0x10202ffac - rustfmt[4ddeb7d779b6c40f]::main 26: 0x10204a668 - std[4f7d7c3ef984657a]::sys::backtrace::__rust_begin_short_backtrace:: 27: 0x102042734 - std[4f7d7c3ef984657a]::rt::lang_start::<()>::{closure#0} 28: 0x10314c0b8 - std::rt::lang_start_internal::hdd117cb81a316264 29: 0x102033080 - _main error: the compiler unexpectedly panicked. this is a bug. note: we would appreciate a bug report: https://github.com/rust-lang/rustfmt/issues/new?labels=bug ```
mitsuhiko commented 2 months ago

I just ran into this trying to rustfmt some pretty terrifying expanded macro code. I found the reduced example above more useful to debug than the mess I had. I was not particularly successful in finding the issue but the issue appears to be in Ord for UseSegment somewhere. I'm guessing the issue might be that the different kind pairs are not given a total order?

https://github.com/rust-lang/rustfmt/blob/6157568e1cec55ce6df09342ba0ae9686dec99c9/src/imports.rs#L922-L933

ajewellamz commented 1 month ago

Still broken in Rust 1.82, not surprising since rustfmt is still 1.7.1-stable

ajewellamz commented 1 month ago

I was trying to fix this myself, but I'm having trouble building it.

I assumed that all I had to do was

git clone git@github.com:rust-lang/rust.git
cd rust/src/tools/rustfmt
git checkout 1.82.0 # same failure with or without this step
cargo build

but that fails to build, with 40 compile time errors in various file within src/tools/rustfmt/src

Am I missing something?

ytmimi commented 1 month ago

You want to clone the rustfmt repo, not the rust-lang/rust repo.

git clone https://github.com/rust-lang/rustfmt.git
cd rustfmt
cargo build # should automatically install the relevant toolchain / components
ajewellamz commented 1 month ago

Thanks, that works.

I've found the problem, but I'm not sure of the proper fix Ord for UseSegment is not transitive.

In imports.rs there exists impl Ord for UseSegment { within that function is

match (&self.kind, &other.kind) {
   ...
   (Ident(ref pia, ref aa), Ident(ref pib, ref ab)) => {
   ...
          1)        if ia.starts_with(char::is_uppercase) && ib.starts_with(char::is_lowercase) {
                        return Ordering::Greater;
                    }
          2)        if ia.starts_with(char::is_lowercase) && ib.starts_with(char::is_uppercase) {
                        return Ordering::Less;
                    }
          3)      if is_upper_snake_case(ia) && !is_upper_snake_case(ib) {
                        return Ordering::Greater;
                    }
          4)      if !is_upper_snake_case(ia) && is_upper_snake_case(ib) {
                        return Ordering::Less;
                    }
          5)         ia.cmp(ib)

let A = DafnyType let B = _System let C = truncate

A < B by 5) B < C by 5) A > C by 1)

hence user-provided comparison function does not correctly implement a total order

ajewellamz commented 1 month ago

Fixed in https://github.com/rust-lang/rustfmt/pull/6375