Rewrite multibyte_length with slice patterns #1580
Conversation
rust_src/src/multibyte.rs
Outdated
| // The bits a byte "starts" with are the most significant ones in these comments | ||
| match slice { | ||
| // true if a starts with 0 | ||
| &[a] if a & 0x80 == 0 => Some(1), |
There was a problem hiding this comment.
Don't all of the match patterns have to have wildcards at the end? I think this one should be &[a, ..], for example.
There was a problem hiding this comment.
That's true, the original code checks for failure conditions rather than success conditions and that has tripped me up a couple of times.
| &[a, b, c, d, e, ..] | ||
| if a == 0xF8 && b & 0xF0 == 0x80 && (c & 0xC0) | (d & 0xC0) | (e & 0xC0) == 0x80 => | ||
| { | ||
| Some(5) | ||
| } |
There was a problem hiding this comment.
I do hate to just burst in like this, but you've changed the behavior here. This code won't return the same result for
a=0xF8, b=0x80. c=0x00, d=0x00, e=0xC0, your code will return length 5, and the old code length None.
I haven't verified this so feel free to ignore me, but I do have a hunch!
Happy coding! :D
There was a problem hiding this comment.
No I think you're right, the old code required the b to be a continuation byte at an earlier point in the code, but since constraints don't cascade in this version I'll have to add the extra check. Thanks for catching this!
Since advanced slice patterns have been stablized, I've been meaning to rewrite this function using them, because I have a very hard time keeping its logic in its current form straight in my head. I also added comments clarifying exactly which bits are being checked for what.
I seem to have made some kind of logic error though, because the build fails, and I can't figure out where the error is.