buffer::move_to_end_of_word_whitespace fails randomly
Opened this issue · 12 comments
The test buffer::move_to_end_of_word_whitespace sometimes fails on my machine if I just repeatedly run the tests without changing anything:
---- buffer::move_to_end_of_word_whitespace stdout ----
thread 'buffer::move_to_end_of_word_whitespace' panicked at 'assertion failed: `(left == right)` (left: `17`, right: `16`)', src/buffer.rs:595
Hm, this is very weird, since the tests are meant to be deterministic. Let me try to reproduce...
@lgvz I can't reproduce it on my machine. Would you be able to provide me with a bit more details on your machine (cpu, os, etc.) and rust compiler?
In the meantime, I have added more assertions to that failing test. Could you try to rerun the test and see which assertions you run into (are they different ones, or just the same one all the time)?
It's the same all the time with the exact same result. In case it helps, here's some versions:
rustc 1.6.0 (c30b771ad 2016-01-19)
cargo 0.7.0-nightly (1af03be 2015-12-08)
Linux hp 3.19.0-51-generic #58~14.04.1-Ubuntu SMP Fri Feb 26 22:02:58 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux
Sorry, I wasn't clear: Here's a new version of the failing test, with additional assertions, in master: https://github.com/srijs/rust-copperline/blob/master/src/buffer.rs#L585
Would you be able to tell me what assertions you run into when you try to run those new tests?
What operating system are you using, btw?
With the new asserts:
---- buffer::move_to_end_of_word_whitespace stdout ----
thread 'buffer::move_to_end_of_word_whitespace' panicked at 'assertion failed: `(left == right)` (left: `16`, right: `17`)', src/buffer.rs:591
Okay, using rust 1.6.0 I can reproduce it (was using an earlier version before). Will now try to narrow it down...
I'm now on Rust 1.7.0, getting the same errors and, in fact, some other tests fail similarly quite randomly.
It seems to me these problems are coming through the strcursor crate. When I changed buffer::char_pos from:
UnicodeWidthStr::width(self.cursor().slice_before())to:
UnicodeWidthStr::width(&self.front_buf[..self.pos])I stopped getting errors from eg. buffer::move_to_end_of_word_whitespace. But I still get these errors:
---- buffer::move_and_insert_cyrillic stdout ----
thread 'buffer::move_and_insert_cyrillic' panicked at 'assertion failed: `(left == right)` (left: `"\u{47e}\u{419}\u{40a}\u{496}"`, right: `"\u{47e}\u{447}\u{40a}\u{496}"`)', src/buffer.rs:489
---- buffer::replace_chars_at_cursor stdout ----
thread 'buffer::replace_chars_at_cursor' panicked at 'assertion failed: `(left == right)` (left: `3`, right: `4`)', src/buffer.rs:631
The buffer::replace_chars_at_cursor has failed on me just once. And some test sometimes hangs.
Btw, isn't self.pos always kept at appropriate character (or grapheme?) boundaries, so that the change I made to char_pos is perfectly ok?
Thanks for the update! I played around with this for a bit as well, and it seems that the more tests I disable the less likely a failure of one of the "fragile" tests becomes.
I have a gut feeling that it might be caused by the strcursor crate which is using a lot of unsafe blocks. It could be that with each Rust version more optimisations are enabled that break some unsafe invariant in those ways.
I believe you're right in that self.pos is always going to be aligned with a glyph boundary, I guess I just wanted to play safe here. But if my suspicion is right, that is ironic because the function I used to be safe is in fact causing the unsafeness here. Oh well...
Heh, yeah people do that a lot, they "play safe" and pick up some library. Like for parsing, nom seems good right, it's a fancy parser comibator lib and popular and everything, what could go wrong? Then, instead of the shiny readme, you look at the source code (aka. the truth) and you see that nom::hex_u32 (the first function I looked at!) will silently overflow when you give it a long hex string (and the is_a it calls is a complete misnomer...).
And I haven't even reported this to them, I'm such an asshole.
Okay, in order to fix this I completely removed all usage of the strcursor crate, and replaced it by a hand-written (currently slightly hacky) version.
Now only the move_and_insert_cjk test fails (I'll look into it soon), everything else works fine.
Commit is here, in case you're interested: 8ef040b
Wen't through the commit, couldn't see anything bad in it. Thanks! I actually noticed that in vi mode D didn't always delete all the chars it should but now it does!