From cd4245d318b04c8b44aed7e682c49b0507086d6c Mon Sep 17 00:00:00 2001 From: Maybe Waffle Date: Thu, 27 Jan 2022 00:25:17 +0300 Subject: [PATCH 1/4] Make char::DecodeUtf16::size_hist more precise New implementation takes into account contents of `self.buf` and rounds lower bound up instead of down. --- library/core/src/char/decode.rs | 18 +++++++++++++++--- 1 file changed, 15 insertions(+), 3 deletions(-) diff --git a/library/core/src/char/decode.rs b/library/core/src/char/decode.rs index 5dd8c5ef78941..f3fef85ef1de5 100644 --- a/library/core/src/char/decode.rs +++ b/library/core/src/char/decode.rs @@ -120,9 +120,21 @@ impl> Iterator for DecodeUtf16 { #[inline] fn size_hint(&self) -> (usize, Option) { let (low, high) = self.iter.size_hint(); - // we could be entirely valid surrogates (2 elements per - // char), or entirely non-surrogates (1 element per char) - (low / 2, high) + + // `self.buf` will never contain the first part of a surrogate, + // so the presence of `buf == Some(...)` always means +1 + // on lower and upper bound. + let addition_from_buf = self.buf.is_some() as usize; + + // `self.iter` could contain entirely valid surrogates (2 elements per + // char), or entirely non-surrogates (1 element per char). + // + // On odd lower bound, at least one element must stay unpaired + // (with other elements from `self.iter`), so we round up. + let low = low.div_ceil(2) + addition_from_buf; + let high = high.and_then(|h| h.checked_add(addition_from_buf)); + + (low, high) } } From 9c8cd1ff376568ffe41ee94f2c99a0d3d83262d5 Mon Sep 17 00:00:00 2001 From: Maybe Waffle Date: Thu, 27 Jan 2022 00:50:34 +0300 Subject: [PATCH 2/4] Add a test for `char::DecodeUtf16::size_hint` --- library/core/tests/char.rs | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/library/core/tests/char.rs b/library/core/tests/char.rs index 2b857a6591929..d776a08323c03 100644 --- a/library/core/tests/char.rs +++ b/library/core/tests/char.rs @@ -308,6 +308,31 @@ fn test_decode_utf16() { check(&[0xD800, 0], &[Err(0xD800), Ok('\0')]); } +#[test] +fn test_decode_utf16_size_hint() { + fn check(s: &[u16]) { + let mut iter = char::decode_utf16(s.iter().cloned()); + + loop { + let count = iter.clone().count(); + let (lower, upper) = iter.size_hint(); + + assert!( + lower <= count && count <= upper.unwrap(), + "lower = {lower}, upper = {upper:?}" + ); + + if let None = iter.next() { + break; + } + } + } + + check(&[0xD800, 0x41, 0x42]); + check(&[0xD800, 0]); + check(&[0xD834, 0x006d]); +} + #[test] fn ed_iterator_specializations() { // Check counting From 2c97d1012e52c9a7ac217a15cce71473cc070e26 Mon Sep 17 00:00:00 2001 From: Maybe Waffle Date: Fri, 28 Jan 2022 12:20:58 +0300 Subject: [PATCH 3/4] Fix wrong assumption in `DecodeUtf16::size_hint` `self.buf` can contain a surrogate, but only a leading one. --- library/core/src/char/decode.rs | 13 +++++++++---- library/core/tests/char.rs | 3 ++- 2 files changed, 11 insertions(+), 5 deletions(-) diff --git a/library/core/src/char/decode.rs b/library/core/src/char/decode.rs index f3fef85ef1de5..81d49ffe06e6d 100644 --- a/library/core/src/char/decode.rs +++ b/library/core/src/char/decode.rs @@ -121,10 +121,15 @@ impl> Iterator for DecodeUtf16 { fn size_hint(&self) -> (usize, Option) { let (low, high) = self.iter.size_hint(); - // `self.buf` will never contain the first part of a surrogate, - // so the presence of `buf == Some(...)` always means +1 - // on lower and upper bound. - let addition_from_buf = self.buf.is_some() as usize; + // If + // - `self.buf` contains a non surrogate (`u < 0xD800 || 0xDFFF < u`), or + // - `high == Some(0)` (and `self.buf` contains a leading surrogate since + // it can never contain a trailing surrogate) + // + // then buf contains an additional character or error that doesn't + // need a pair from `self.iter`, so it's +1 additional element. + let addition_from_buf = + self.buf.map_or(false, |u| u < 0xD800 || 0xDFFF < u || high == Some(0)) as usize; // `self.iter` could contain entirely valid surrogates (2 elements per // char), or entirely non-surrogates (1 element per char). diff --git a/library/core/tests/char.rs b/library/core/tests/char.rs index d776a08323c03..347ac04feb31c 100644 --- a/library/core/tests/char.rs +++ b/library/core/tests/char.rs @@ -319,7 +319,7 @@ fn test_decode_utf16_size_hint() { assert!( lower <= count && count <= upper.unwrap(), - "lower = {lower}, upper = {upper:?}" + "lower = {lower}, count = {count}, upper = {upper:?}" ); if let None = iter.next() { @@ -328,6 +328,7 @@ fn test_decode_utf16_size_hint() { } } + check(&[0xD800, 0xD800, 0xDC00]); check(&[0xD800, 0x41, 0x42]); check(&[0xD800, 0]); check(&[0xD834, 0x006d]); From 17cd2cd5927cf1075efbd0f4a859182eb102b920 Mon Sep 17 00:00:00 2001 From: Maybe Waffle Date: Sun, 30 Jan 2022 15:32:21 +0300 Subject: [PATCH 4/4] Fix an edge case in `chat::DecodeUtf16::size_hint` There are cases, when data in the buf might or might not be an error. --- library/core/src/char/decode.rs | 30 +++++++++++++++++++----------- library/core/tests/char.rs | 1 + 2 files changed, 20 insertions(+), 11 deletions(-) diff --git a/library/core/src/char/decode.rs b/library/core/src/char/decode.rs index 81d49ffe06e6d..8b9f979b573f7 100644 --- a/library/core/src/char/decode.rs +++ b/library/core/src/char/decode.rs @@ -121,23 +121,31 @@ impl> Iterator for DecodeUtf16 { fn size_hint(&self) -> (usize, Option) { let (low, high) = self.iter.size_hint(); - // If - // - `self.buf` contains a non surrogate (`u < 0xD800 || 0xDFFF < u`), or - // - `high == Some(0)` (and `self.buf` contains a leading surrogate since - // it can never contain a trailing surrogate) - // - // then buf contains an additional character or error that doesn't - // need a pair from `self.iter`, so it's +1 additional element. - let addition_from_buf = - self.buf.map_or(false, |u| u < 0xD800 || 0xDFFF < u || high == Some(0)) as usize; + let (low_buf, high_buf) = match self.buf { + // buf is empty, no additional elements from it. + None => (0, 0), + // `u` is a non surrogate, so it's always an additional character. + Some(u) if u < 0xD800 || 0xDFFF < u => (1, 1), + // `u` is a leading surrogate (it can never be a trailing surrogate and + // it's a surrogate due to the previous branch) and `self.iter` is empty. + // + // `u` can't be paired, since the `self.iter` is empty, + // so it will always become an additional element (error). + Some(_u) if high == Some(0) => (1, 1), + // `u` is a leading surrogate and `iter` may be non-empty. + // + // `u` can either pair with a trailing surrogate, in which case no additional elements + // are produced, or it can become an error, in which case it's an additional character (error). + Some(_u) => (0, 1), + }; // `self.iter` could contain entirely valid surrogates (2 elements per // char), or entirely non-surrogates (1 element per char). // // On odd lower bound, at least one element must stay unpaired // (with other elements from `self.iter`), so we round up. - let low = low.div_ceil(2) + addition_from_buf; - let high = high.and_then(|h| h.checked_add(addition_from_buf)); + let low = low.div_ceil(2) + low_buf; + let high = high.and_then(|h| h.checked_add(high_buf)); (low, high) } diff --git a/library/core/tests/char.rs b/library/core/tests/char.rs index 347ac04feb31c..4c899b6eb43d0 100644 --- a/library/core/tests/char.rs +++ b/library/core/tests/char.rs @@ -329,6 +329,7 @@ fn test_decode_utf16_size_hint() { } check(&[0xD800, 0xD800, 0xDC00]); + check(&[0xD800, 0xD800, 0x0]); check(&[0xD800, 0x41, 0x42]); check(&[0xD800, 0]); check(&[0xD834, 0x006d]);