jmacdonald / amp

A complete text editor for your terminal.
https://amp.rs
Other
3.68k stars 105 forks source link

Building master gives me "thread 'main' panicked at 'attempted to leave type `linked_hash_map::Node<yaml::Yaml, yaml::Yaml>` uninitialized, which is invalid'" If I have any syntax definition file on syntaxes folder #205

Closed pbgc closed 3 years ago

pbgc commented 3 years ago

I just tried to build the current master version and unless I delete all syntax files (that work great with 0.6.2) on ~/Library/Application Support/amp/syntaxes folder I get:

thread 'main' panicked at 'attempted to leave type linked_hash_map::Node<yaml::Yaml, yaml::Yaml> uninitialized, which is invalid', /Users/pbeck/.rustup/toolchains/stable-x86_64-apple-darwin/lib/rustlib/src/rust/library/core/src/mem/mod.rs:658:9 stack backtrace: 0: 0x1051336d4 - ::fmt::hcfc48256a5ab8835 1: 0x1051517d0 - core::fmt::write::haf3903118f694c48 2: 0x10512ffb6 - std::io::Write::write_fmt::h7385463ac87804ed 3: 0x1051353ef - std::panicking::default_hook::{{closure}}::h91bd4c58cf71392b 4: 0x1051350bd - std::panicking::default_hook::h7bd29c87df967048 5: 0x1051359bb - std::panicking::rust_panic_with_hook::hae2b05f08a320721 6: 0x10513553b - std::panicking::begin_panic_handler::{{closure}}::h72d68d3a77e0b718 7: 0x105133b48 - std::sys_common::backtrace::__rust_end_short_backtrace::h7c5e286792f94edb 8: 0x1051354fa - _rust_begin_unwind 9: 0x10516303f - core::panicking::panic_fmt::h1b194bb80d76fb10 10: 0x105162f97 - core::panicking::panic::hdb9dddaff64fd68b 11: 0x105057c0e - linked_hash_map::LinkedHashMap<K,V,S>::insert::h9c35a7140c8307a8 12: 0x105055de0 - yaml_rust::yaml::YamlLoader::insert_new_node::h4430aa226acfa521 13: 0x1050557dd - ::on_event::h531a9baef346c4ca 14: 0x10504ae5f - yaml_rust::parser::Parser::load_node::h9aad82c72af03881 15: 0x10504b39e - yaml_rust::parser::Parser::load_mapping::hab4900669fbd6767 16: 0x10504a8fb - yaml_rust::parser::Parser::load::h48b3e155124689af 17: 0x105055f88 - yaml_rust::yaml::YamlLoader::load_from_str::h131fea149d119bed 18: 0x1050003ba - syntect::parsing::yaml_load::::load_from_str::h63dc492d841c75ea 19: 0x104ff1e84 - syntect::parsing::syntax_set::load_syntax_file::hca31e8702393465c 20: 0x104f84118 - syntect::parsing::syntax_set::SyntaxSet::load_syntaxes::hd3d8827f3b310f19 21: 0x104f367e3 - amp::models::application::Application::new::h0ed9f6f35cda2602 22: 0x104f174d0 - amp::main::h0e68d5cbc22626e7 23: 0x104f14fea - std::sys_common::backtrace::__rust_begin_short_backtrace::hdb57f6c8cc0d9e33 24: 0x104f1c3dc - std::rt::lang_start::{{closure}}::h0eee3c05a96f9999 25: 0x105135d34 - std::rt::lang_start_internal::hd38bb63f9540b327 26: 0x104f17879 - _main

christoph-heiss commented 3 years ago

So I investigated this a bit. It seems to be a compiler-related problem. If you switch the compiler to stable-1.47 (can be done via running rustup override set 1.47 in the amp directory) everything works fine again. This seems to be related to this change which was released as part of 1.48.

This is essentially an issue with the yaml_rust crate, used by both syntect and amp. This was already reported upstream.

Proper fixing would than later also involve updating the syntect dependency in both the scribe crate and amp itself, so it's a bunch of work since there were some breaking changes in recent syntect releases.

jmacdonald commented 3 years ago

@pbgc thanks for reporting this, and @christoph-heiss thanks for investigating. I'd started down the road of upgrading syntect a while back but it was non-trivial. My schedule has gotten a little more reasonable as of late and I'll be able to pick that up again.

@christoph-heiss I just wanted to reach out to make sure you weren't already in the middle of doing that before I start; let me know!

christoph-heiss commented 3 years ago

@jmacdonald Funnily enough I already worked on that - I updated both amp and scribe to both syntect 4.5.0, which is the current release (christoph-heiss/amp@syntect-update, christoph-heiss/scribe@syntect-update).

Everything seemed to work during my limited testing, but I didn't really test it thoroughly yet. I will integrate it in my personal build for the next few days to see if something is off.

Problem is - this won't really resolve this particular issue. syntect 4.5.0 uses the same version of yaml-rust as before (and amp) - thus amp still crashes when compiled with Rust > 1.47.0 and using custom syntax files. But updating syntect now will make it easier later.

christoph-heiss commented 3 years ago

@jmacdonald So I just had another look at this it this problem seems to be fixed upstream. When updating yaml-rust to its current version as available on crates.io, running it after building it with rustc 1.50.0 works just fine.

-yaml-rust = "0.3.5"
+yaml-rust = ">=0.4.5"

Since it's basically a one-liner I didn't necessarily want to create a separate pull request for it.

My 2 cents: Since it is more or less a blocking bug, a new patch-level release would be nice to make installs from crates.io work again with current rust.

jmacdonald commented 3 years ago

@christoph-heiss I just ran without bumping yaml-rust on Rust 1.50 and didn't get a panic; is the fix actually with the new Rust release?

jmacdonald commented 3 years ago

Nevermind @christoph-heiss, I had my rustup default pinned to 1.42. :innocent: