From 65c75b2db7827f8faa63c332139a3264b4ab9427 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=D0=90=D1=80=D1=82=D1=91=D0=BC=20=D0=9F=D0=B0=D0=B2=D0=BB?= =?UTF-8?q?=D0=BE=D0=B2?= Date: Thu, 14 Apr 2022 20:44:09 +0300 Subject: [PATCH 01/10] Use rounding in float to Duration conversion methods --- library/core/src/time.rs | 61 +++++++++++++++++++++++++--------------- 1 file changed, 38 insertions(+), 23 deletions(-) diff --git a/library/core/src/time.rs b/library/core/src/time.rs index 243c044b5d9..fc2484f0cdd 100644 --- a/library/core/src/time.rs +++ b/library/core/src/time.rs @@ -730,9 +730,9 @@ impl Duration { /// // subnormal float /// let res = Duration::from_secs_f64(f64::from_bits(1)); /// assert_eq!(res, Duration::new(0, 0)); - /// // conversion uses truncation, not rounding + /// // conversion uses rounding /// let res = Duration::from_secs_f64(0.999e-9); - /// assert_eq!(res, Duration::new(0, 0)); + /// assert_eq!(res, Duration::new(0, 1)); /// ``` #[stable(feature = "duration_float", since = "1.38.0")] #[must_use] @@ -760,17 +760,17 @@ impl Duration { /// let res = Duration::from_secs_f32(1e-20); /// assert_eq!(res, Duration::new(0, 0)); /// let res = Duration::from_secs_f32(4.2e-7); - /// assert_eq!(res, Duration::new(0, 419)); + /// assert_eq!(res, Duration::new(0, 420)); /// let res = Duration::from_secs_f32(2.7); - /// assert_eq!(res, Duration::new(2, 700_000_047)); + /// assert_eq!(res, Duration::new(2, 700_000_048)); /// let res = Duration::from_secs_f32(3e10); /// assert_eq!(res, Duration::new(30_000_001_024, 0)); /// // subnormal float /// let res = Duration::from_secs_f32(f32::from_bits(1)); /// assert_eq!(res, Duration::new(0, 0)); - /// // conversion uses truncation, not rounding + /// // conversion uses rounding /// let res = Duration::from_secs_f32(0.999e-9); - /// assert_eq!(res, Duration::new(0, 0)); + /// assert_eq!(res, Duration::new(0, 1)); /// ``` #[stable(feature = "duration_float", since = "1.38.0")] #[must_use] @@ -815,7 +815,7 @@ impl Duration { /// use std::time::Duration; /// /// let dur = Duration::new(2, 700_000_000); - /// assert_eq!(dur.mul_f32(3.14), Duration::new(8, 478_000_640)); + /// assert_eq!(dur.mul_f32(3.14), Duration::new(8, 478_000_641)); /// assert_eq!(dur.mul_f32(3.14e5), Duration::new(847800, 0)); /// ``` #[stable(feature = "duration_float", since = "1.38.0")] @@ -838,8 +838,7 @@ impl Duration { /// /// let dur = Duration::new(2, 700_000_000); /// assert_eq!(dur.div_f64(3.14), Duration::new(0, 859_872_611)); - /// // note that truncation is used, not rounding - /// assert_eq!(dur.div_f64(3.14e5), Duration::new(0, 8_598)); + /// assert_eq!(dur.div_f64(3.14e5), Duration::new(0, 8_599)); /// ``` #[stable(feature = "duration_float", since = "1.38.0")] #[must_use = "this returns the result of the operation, \ @@ -862,9 +861,8 @@ impl Duration { /// let dur = Duration::new(2, 700_000_000); /// // note that due to rounding errors result is slightly /// // different from 0.859_872_611 - /// assert_eq!(dur.div_f32(3.14), Duration::new(0, 859_872_579)); - /// // note that truncation is used, not rounding - /// assert_eq!(dur.div_f32(3.14e5), Duration::new(0, 8_598)); + /// assert_eq!(dur.div_f32(3.14), Duration::new(0, 859_872_580)); + /// assert_eq!(dur.div_f32(3.14e5), Duration::new(0, 8_599)); /// ``` #[stable(feature = "duration_float", since = "1.38.0")] #[must_use = "this returns the result of the operation, \ @@ -1278,13 +1276,30 @@ macro_rules! try_from_secs { } else if exp < 0 { // the input is less than 1 second let t = <$double_ty>::from(mant) << ($offset + exp); - let nanos = (u128::from(NANOS_PER_SEC) * u128::from(t)) >> ($mant_bits + $offset); - (0, nanos as u32) + let nanos_offset = $mant_bits + $offset; + let nanos_tmp = u128::from(NANOS_PER_SEC) * u128::from(t); + let nanos = (nanos_tmp >> nanos_offset) as u32; + if nanos_tmp & (1 << (nanos_offset - 1)) == 0 { + (0, nanos) + } else if nanos + 1 != NANOS_PER_SEC { + (0, nanos + 1) + } else { + (1, 0) + } } else if exp < $mant_bits { - let secs = mant >> ($mant_bits - exp); + let secs = u64::from(mant >> ($mant_bits - exp)); let t = <$double_ty>::from((mant << exp) & MANT_MASK); - let nanos = (<$double_ty>::from(NANOS_PER_SEC) * t) >> $mant_bits; - (u64::from(secs), nanos as u32) + let nanos_tmp = <$double_ty>::from(NANOS_PER_SEC) * t; + let nanos = (nanos_tmp >> $mant_bits) as u32; + if nanos_tmp & (1 << ($mant_bits - 1)) == 0 { + (secs, nanos) + } else if nanos + 1 != NANOS_PER_SEC { + (secs, nanos + 1) + } else { + // `secs + 1` can not overflow since `exp` is less than `$mant_bits` + // and the latter is less than 64 bits for both `f32` and `f64` + (secs + 1, 0) + } } else if exp < 64 { // the input has no fractional part let secs = u64::from(mant) << (exp - $mant_bits); @@ -1315,17 +1330,17 @@ impl Duration { /// let res = Duration::try_from_secs_f32(1e-20); /// assert_eq!(res, Ok(Duration::new(0, 0))); /// let res = Duration::try_from_secs_f32(4.2e-7); - /// assert_eq!(res, Ok(Duration::new(0, 419))); + /// assert_eq!(res, Ok(Duration::new(0, 420))); /// let res = Duration::try_from_secs_f32(2.7); - /// assert_eq!(res, Ok(Duration::new(2, 700_000_047))); + /// assert_eq!(res, Ok(Duration::new(2, 700_000_048))); /// let res = Duration::try_from_secs_f32(3e10); /// assert_eq!(res, Ok(Duration::new(30_000_001_024, 0))); /// // subnormal float: /// let res = Duration::try_from_secs_f32(f32::from_bits(1)); /// assert_eq!(res, Ok(Duration::new(0, 0))); - /// // conversion uses truncation, not rounding + /// // conversion uses rounding /// let res = Duration::try_from_secs_f32(0.999e-9); - /// assert_eq!(res, Ok(Duration::new(0, 0))); + /// assert_eq!(res, Ok(Duration::new(0, 1))); /// /// let res = Duration::try_from_secs_f32(-5.0); /// assert!(res.is_err()); @@ -1372,9 +1387,9 @@ impl Duration { /// // subnormal float /// let res = Duration::try_from_secs_f64(f64::from_bits(1)); /// assert_eq!(res, Ok(Duration::new(0, 0))); - /// // conversion uses truncation, not rounding + /// // conversion uses rounding /// let res = Duration::try_from_secs_f32(0.999e-9); - /// assert_eq!(res, Ok(Duration::new(0, 0))); + /// assert_eq!(res, Ok(Duration::new(0, 1))); /// /// let res = Duration::try_from_secs_f64(-5.0); /// assert!(res.is_err()); From f7521db243a90d77d8f8cc263c97abb1e598e03d Mon Sep 17 00:00:00 2001 From: Vadim Petrochenkov Date: Sat, 21 May 2022 15:06:47 +0300 Subject: [PATCH 02/10] rustdoc: Remove `ItemFragment(Kind)` --- .../passes/collect_intra_doc_links.rs | 104 +++++------------- 1 file changed, 30 insertions(+), 74 deletions(-) diff --git a/src/librustdoc/passes/collect_intra_doc_links.rs b/src/librustdoc/passes/collect_intra_doc_links.rs index ea0f5102539..976d89bec3b 100644 --- a/src/librustdoc/passes/collect_intra_doc_links.rs +++ b/src/librustdoc/passes/collect_intra_doc_links.rs @@ -220,80 +220,40 @@ enum MalformedGenerics { #[derive(Clone, Debug, Hash, PartialEq, Eq)] pub(crate) enum UrlFragment { - Item(ItemFragment), + Item(DefId), UserWritten(String), } impl UrlFragment { /// Render the fragment, including the leading `#`. pub(crate) fn render(&self, s: &mut String, tcx: TyCtxt<'_>) -> std::fmt::Result { + s.push('#'); match self { - UrlFragment::Item(frag) => frag.render(s, tcx), - UrlFragment::UserWritten(raw) => write!(s, "#{}", raw), - } - } -} - -#[derive(Copy, Clone, Debug, Hash, PartialEq, Eq)] -pub(crate) struct ItemFragment(FragmentKind, DefId); - -#[derive(Copy, Clone, Debug, Hash, PartialEq, Eq)] -pub(crate) enum FragmentKind { - Method, - TyMethod, - AssociatedConstant, - AssociatedType, - - StructField, - Variant, - VariantField, -} - -impl FragmentKind { - fn from_def_id(tcx: TyCtxt<'_>, def_id: DefId) -> FragmentKind { - match tcx.def_kind(def_id) { - DefKind::AssocFn => { - if tcx.associated_item(def_id).defaultness.has_value() { - FragmentKind::Method - } else { - FragmentKind::TyMethod - } - } - DefKind::AssocConst => FragmentKind::AssociatedConstant, - DefKind::AssocTy => FragmentKind::AssociatedType, - DefKind::Variant => FragmentKind::Variant, - DefKind::Field => { - if tcx.def_kind(tcx.parent(def_id)) == DefKind::Variant { - FragmentKind::VariantField - } else { - FragmentKind::StructField - } - } - kind => bug!("unexpected associated item kind: {:?}", kind), - } - } -} - -impl ItemFragment { - /// Render the fragment, including the leading `#`. - pub(crate) fn render(&self, s: &mut String, tcx: TyCtxt<'_>) -> std::fmt::Result { - write!(s, "#")?; - match *self { - ItemFragment(kind, def_id) => { + &UrlFragment::Item(def_id) => { let name = tcx.item_name(def_id); - match kind { - FragmentKind::Method => write!(s, "method.{}", name), - FragmentKind::TyMethod => write!(s, "tymethod.{}", name), - FragmentKind::AssociatedConstant => write!(s, "associatedconstant.{}", name), - FragmentKind::AssociatedType => write!(s, "associatedtype.{}", name), - FragmentKind::StructField => write!(s, "structfield.{}", name), - FragmentKind::Variant => write!(s, "variant.{}", name), - FragmentKind::VariantField => { - let variant = tcx.item_name(tcx.parent(def_id)); - write!(s, "variant.{}.field.{}", variant, name) + match tcx.def_kind(def_id) { + DefKind::AssocFn => { + if tcx.associated_item(def_id).defaultness.has_value() { + write!(s, "method.{}", name) + } else { + write!(s, "tymethod.{}", name) + } } + DefKind::AssocConst => write!(s, "associatedconstant.{}", name), + DefKind::AssocTy => write!(s, "associatedtype.{}", name), + DefKind::Variant => write!(s, "variant.{}", name), + DefKind::Field => { + let parent_id = tcx.parent(def_id); + if tcx.def_kind(parent_id) == DefKind::Variant { + write!(s, "variant.{}.field.{}", tcx.item_name(parent_id), name) + } else { + write!(s, "structfield.{}", name) + } + } + kind => bug!("unexpected associated item kind: {:?}", kind), } } + UrlFragment::UserWritten(raw) => Ok(s.push_str(&raw)), } } } @@ -1124,7 +1084,7 @@ impl LinkCollector<'_, '_> { match res { Res::Primitive(prim) => { - if let Some(UrlFragment::Item(ItemFragment(_, id))) = fragment { + if let Some(UrlFragment::Item(id)) = fragment { // We're actually resolving an associated item of a primitive, so we need to // verify the disambiguator (if any) matches the type of the associated item. // This case should really follow the same flow as the `Res::Def` branch below, @@ -1172,12 +1132,11 @@ impl LinkCollector<'_, '_> { }) } Res::Def(kind, id) => { - let (kind_for_dis, id_for_dis) = - if let Some(UrlFragment::Item(ItemFragment(_, id))) = fragment { - (self.cx.tcx.def_kind(id), id) - } else { - (kind, id) - }; + let (kind_for_dis, id_for_dis) = if let Some(UrlFragment::Item(id)) = fragment { + (self.cx.tcx.def_kind(id), id) + } else { + (kind, id) + }; self.verify_disambiguator( path_str, &ori_link, @@ -1318,10 +1277,7 @@ impl LinkCollector<'_, '_> { return None; } (Some(u_frag), None) => Some(UrlFragment::UserWritten(u_frag.clone())), - (None, Some(def_id)) => Some(UrlFragment::Item(ItemFragment( - FragmentKind::from_def_id(self.cx.tcx, def_id), - def_id, - ))), + (None, Some(def_id)) => Some(UrlFragment::Item(def_id)), (None, None) => None, }; Some((res, fragment)) From c5449f57c190c2bbe29d6a4c7f0ced7049a46b6f Mon Sep 17 00:00:00 2001 From: Vadim Petrochenkov Date: Sat, 21 May 2022 15:19:53 +0300 Subject: [PATCH 03/10] rustdoc: Stop using `write!` in `UrlFragment::render` --- src/librustdoc/clean/types.rs | 2 +- .../passes/collect_intra_doc_links.rs | 28 ++++++++++--------- 2 files changed, 16 insertions(+), 14 deletions(-) diff --git a/src/librustdoc/clean/types.rs b/src/librustdoc/clean/types.rs index 3123ece3773..6ee80a3476c 100644 --- a/src/librustdoc/clean/types.rs +++ b/src/librustdoc/clean/types.rs @@ -525,7 +525,7 @@ impl Item { if let Ok((mut href, ..)) = href(*did, cx) { debug!(?href); if let Some(ref fragment) = *fragment { - fragment.render(&mut href, cx.tcx()).unwrap() + fragment.render(&mut href, cx.tcx()) } Some(RenderedLink { original_text: s.clone(), diff --git a/src/librustdoc/passes/collect_intra_doc_links.rs b/src/librustdoc/passes/collect_intra_doc_links.rs index 976d89bec3b..b20402e532a 100644 --- a/src/librustdoc/passes/collect_intra_doc_links.rs +++ b/src/librustdoc/passes/collect_intra_doc_links.rs @@ -20,7 +20,6 @@ use rustc_span::BytePos; use smallvec::{smallvec, SmallVec}; use std::borrow::Cow; -use std::fmt::Write; use std::mem; use std::ops::Range; @@ -226,34 +225,37 @@ pub(crate) enum UrlFragment { impl UrlFragment { /// Render the fragment, including the leading `#`. - pub(crate) fn render(&self, s: &mut String, tcx: TyCtxt<'_>) -> std::fmt::Result { + pub(crate) fn render(&self, s: &mut String, tcx: TyCtxt<'_>) { s.push('#'); match self { &UrlFragment::Item(def_id) => { - let name = tcx.item_name(def_id); - match tcx.def_kind(def_id) { + let kind = match tcx.def_kind(def_id) { DefKind::AssocFn => { if tcx.associated_item(def_id).defaultness.has_value() { - write!(s, "method.{}", name) + "method." } else { - write!(s, "tymethod.{}", name) + "tymethod." } } - DefKind::AssocConst => write!(s, "associatedconstant.{}", name), - DefKind::AssocTy => write!(s, "associatedtype.{}", name), - DefKind::Variant => write!(s, "variant.{}", name), + DefKind::AssocConst => "associatedconstant.", + DefKind::AssocTy => "associatedtype.", + DefKind::Variant => "variant.", DefKind::Field => { let parent_id = tcx.parent(def_id); if tcx.def_kind(parent_id) == DefKind::Variant { - write!(s, "variant.{}.field.{}", tcx.item_name(parent_id), name) + s.push_str("variant."); + s.push_str(tcx.item_name(parent_id).as_str()); + ".field." } else { - write!(s, "structfield.{}", name) + "structfield." } } kind => bug!("unexpected associated item kind: {:?}", kind), - } + }; + s.push_str(kind); + s.push_str(tcx.item_name(def_id).as_str()); } - UrlFragment::UserWritten(raw) => Ok(s.push_str(&raw)), + UrlFragment::UserWritten(raw) => s.push_str(&raw), } } } From 3f21c31bf4f3ebab5e982a2cff8befbf1763294b Mon Sep 17 00:00:00 2001 From: Vadim Petrochenkov Date: Sat, 21 May 2022 15:35:07 +0300 Subject: [PATCH 04/10] rustdoc: Some link resolution caching cleanup --- .../passes/collect_intra_doc_links.rs | 51 ++++++------------- 1 file changed, 16 insertions(+), 35 deletions(-) diff --git a/src/librustdoc/passes/collect_intra_doc_links.rs b/src/librustdoc/passes/collect_intra_doc_links.rs index b20402e532a..ffc765f4040 100644 --- a/src/librustdoc/passes/collect_intra_doc_links.rs +++ b/src/librustdoc/passes/collect_intra_doc_links.rs @@ -277,11 +277,6 @@ struct DiagnosticInfo<'a> { link_range: Range, } -#[derive(Clone, Debug, Hash)] -struct CachedLink { - res: (Res, Option), -} - struct LinkCollector<'a, 'tcx> { cx: &'a mut DocContext<'tcx>, /// A stack of modules used to decide what scope to resolve in. @@ -291,7 +286,7 @@ struct LinkCollector<'a, 'tcx> { mod_ids: Vec, /// Cache the resolved links so we can avoid resolving (and emitting errors for) the same link. /// The link will be `None` if it could not be resolved (i.e. the error was cached). - visited_links: FxHashMap>, + visited_links: FxHashMap)>>, } impl<'a, 'tcx> LinkCollector<'a, 'tcx> { @@ -1060,6 +1055,9 @@ impl LinkCollector<'_, '_> { extra_fragment: extra_fragment.clone(), }, diag_info.clone(), // this struct should really be Copy, but Range is not :( + // For reference-style links we want to report only one error so unsuccessful + // resolutions are cached, for other links we want to report an error every + // time so they are not cached. matches!(ori_link.kind, LinkType::Reference | LinkType::Shortcut), )?; @@ -1256,26 +1254,20 @@ impl LinkCollector<'_, '_> { &mut self, key: ResolutionInfo, diag: DiagnosticInfo<'_>, - cache_resolution_failure: bool, + // If errors are cached then they are only reported on first ocurrence + // which we want in some cases but not in others. + cache_errors: bool, ) -> Option<(Res, Option)> { - if let Some(ref cached) = self.visited_links.get(&key) { - match cached { - Some(cached) => { - return Some(cached.res.clone()); - } - None if cache_resolution_failure => return None, - None => { - // Although we hit the cache and found a resolution error, this link isn't - // supposed to cache those. Run link resolution again to emit the expected - // resolution error. - } + if let Some(res) = self.visited_links.get(&key) { + if res.is_some() || cache_errors { + return res.clone(); } } let res = self.resolve_with_disambiguator(&key, diag.clone()).and_then(|(res, def_id)| { let fragment = match (&key.extra_fragment, def_id) { (Some(_), Some(def_id)) => { - report_anchor_conflict(self.cx, diag, Res::from_def_id(self.cx.tcx, def_id)); + report_anchor_conflict(self.cx, diag, def_id); return None; } (Some(u_frag), None) => Some(UrlFragment::UserWritten(u_frag.clone())), @@ -1285,21 +1277,10 @@ impl LinkCollector<'_, '_> { Some((res, fragment)) }); - // Cache only if resolved successfully - don't silence duplicate errors - if let Some(res) = res { - // Store result for the actual namespace - self.visited_links.insert(key, Some(CachedLink { res: res.clone() })); - - Some(res) - } else { - if cache_resolution_failure { - // For reference-style links we only want to report one resolution error - // so let's cache them as well. - self.visited_links.insert(key, None); - } - - None + if res.is_some() || cache_errors { + self.visited_links.insert(key, res.clone()); } + res } /// After parsing the disambiguator, resolve the main part of the link. @@ -1875,8 +1856,8 @@ fn report_multiple_anchors(cx: &DocContext<'_>, diag_info: DiagnosticInfo<'_>) { anchor_failure(cx, diag_info, &msg, 1) } -fn report_anchor_conflict(cx: &DocContext<'_>, diag_info: DiagnosticInfo<'_>, res: Res) { - let (link, kind) = (diag_info.ori_link, res.descr()); +fn report_anchor_conflict(cx: &DocContext<'_>, diag_info: DiagnosticInfo<'_>, def_id: DefId) { + let (link, kind) = (diag_info.ori_link, Res::from_def_id(cx.tcx, def_id).descr()); let msg = format!("`{link}` contains an anchor, but links to {kind}s are already anchored"); anchor_failure(cx, diag_info, &msg, 0) } From c2d8445cde2b515d54891964efcdc43722c5d5ca Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=D0=90=D1=80=D1=82=D1=91=D0=BC=20=D0=9F=D0=B0=D0=B2=D0=BB?= =?UTF-8?q?=D0=BE=D0=B2=20=5BArtyom=20Pavlov=5D?= Date: Wed, 25 May 2022 05:01:11 +0300 Subject: [PATCH 05/10] implement tie to even --- library/core/src/time.rs | 92 +++++++++++++++++++++++++++++++--------- 1 file changed, 73 insertions(+), 19 deletions(-) diff --git a/library/core/src/time.rs b/library/core/src/time.rs index fc2484f0cdd..7f23cb83e09 100644 --- a/library/core/src/time.rs +++ b/library/core/src/time.rs @@ -1270,8 +1270,8 @@ macro_rules! try_from_secs { let mant = (bits & MANT_MASK) | (MANT_MASK + 1); let exp = ((bits >> $mant_bits) & EXP_MASK) as i16 + MIN_EXP; - let (secs, nanos) = if exp < -30 { - // the input represents less than 1ns. + let (secs, nanos) = if exp < -31 { + // the input represents less than 1ns and can not be rounded to it (0u64, 0u32) } else if exp < 0 { // the input is less than 1 second @@ -1279,27 +1279,37 @@ macro_rules! try_from_secs { let nanos_offset = $mant_bits + $offset; let nanos_tmp = u128::from(NANOS_PER_SEC) * u128::from(t); let nanos = (nanos_tmp >> nanos_offset) as u32; - if nanos_tmp & (1 << (nanos_offset - 1)) == 0 { - (0, nanos) - } else if nanos + 1 != NANOS_PER_SEC { - (0, nanos + 1) - } else { - (1, 0) - } + + let rem_mask = (1 << nanos_offset) - 1; + let rem_msb_mask = 1 << (nanos_offset - 1); + let rem = nanos_tmp & rem_mask; + let is_tie = rem == rem_msb_mask; + let is_even = (nanos & 1) == 0; + let rem_msb = nanos_tmp & rem_msb_mask == 0; + let add_ns = !(rem_msb || (is_even && is_tie)); + + // note that neither `f32`, nor `f64` can represent + // 0.999_999_999_5 exactly, so the nanos part + // never will be equal to 10^9. + (0, nanos + add_ns as u32) } else if exp < $mant_bits { let secs = u64::from(mant >> ($mant_bits - exp)); let t = <$double_ty>::from((mant << exp) & MANT_MASK); + let nanos_offset = $mant_bits; let nanos_tmp = <$double_ty>::from(NANOS_PER_SEC) * t; - let nanos = (nanos_tmp >> $mant_bits) as u32; - if nanos_tmp & (1 << ($mant_bits - 1)) == 0 { - (secs, nanos) - } else if nanos + 1 != NANOS_PER_SEC { - (secs, nanos + 1) - } else { - // `secs + 1` can not overflow since `exp` is less than `$mant_bits` - // and the latter is less than 64 bits for both `f32` and `f64` - (secs + 1, 0) - } + let nanos = (nanos_tmp >> nanos_offset) as u32; + + let rem_mask = (1 << nanos_offset) - 1; + let rem_msb_mask = 1 << (nanos_offset - 1); + let rem = nanos_tmp & rem_mask; + let is_tie = rem == rem_msb_mask; + let is_even = (nanos & 1) == 0; + let rem_msb = nanos_tmp & rem_msb_mask == 0; + let add_ns = !(rem_msb || (is_even && is_tie)); + + // neither `f32`, nor `f64` can represent x.999_999_999_5 exactly, + // so the nanos part never will be equal to 10^9 + (secs, nanos + add_ns as u32) } else if exp < 64 { // the input has no fractional part let secs = u64::from(mant) << (exp - $mant_bits); @@ -1348,6 +1358,28 @@ impl Duration { /// assert!(res.is_err()); /// let res = Duration::try_from_secs_f32(2e19); /// assert!(res.is_err()); + /// + /// // this method uses round to nearest, ties to even + /// + /// // this float represents exactly 976562.5e-9 + /// let val = f32::from_bits(0x3A80_0000); + /// let res = Duration::try_from_secs_f32(val); + /// assert_eq!(res, Ok(Duration::new(0, 976_562))); + /// + /// // this float represents exactly 2929687.5e-9 + /// let val = f32::from_bits(0x3B40_0000); + /// let res = Duration::try_from_secs_f32(val); + /// assert_eq!(res, Ok(Duration::new(0, 2_929_688))); + /// + /// // this float represents exactly 1.000_976_562_5 + /// let val = f32::from_bits(0x3F802000); + /// let res = Duration::try_from_secs_f32(val); + /// assert_eq!(res, Ok(Duration::new(1, 976_562))); + /// + /// // this float represents exactly 1.002_929_687_5 + /// let val = f32::from_bits(0x3F806000); + /// let res = Duration::try_from_secs_f32(val); + /// assert_eq!(res, Ok(Duration::new(1, 2_929_688))); /// ``` #[unstable(feature = "duration_checked_float", issue = "83400")] #[inline] @@ -1397,6 +1429,28 @@ impl Duration { /// assert!(res.is_err()); /// let res = Duration::try_from_secs_f64(2e19); /// assert!(res.is_err()); + /// + /// // this method uses round to nearest, ties to even + /// + /// // this float represents exactly 976562.5e-9 + /// let val = f64::from_bits(0x3F50_0000_0000_0000); + /// let res = Duration::try_from_secs_f64(val); + /// assert_eq!(res, Ok(Duration::new(0, 976_562))); + /// + /// // this float represents exactly 2929687.5e-9 + /// let val = f64::from_bits(0x3F68_0000_0000_0000); + /// let res = Duration::try_from_secs_f64(val); + /// assert_eq!(res, Ok(Duration::new(0, 2_929_688))); + /// + /// // this float represents exactly 1.000_976_562_5 + /// let val = f64::from_bits(0x3FF0_0400_0000_0000); + /// let res = Duration::try_from_secs_f64(val); + /// assert_eq!(res, Ok(Duration::new(1, 976_562))); + /// + /// // this float represents exactly 1.002_929_687_5 + /// let val = f64::from_bits(0x3_FF00_C000_0000_000); + /// let res = Duration::try_from_secs_f64(val); + /// assert_eq!(res, Ok(Duration::new(1, 2_929_688))); /// ``` #[unstable(feature = "duration_checked_float", issue = "83400")] #[inline] From 26f859463eb756e7363cc32f5b5ae795790803b5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=D0=90=D1=80=D1=82=D1=91=D0=BC=20=D0=9F=D0=B0=D0=B2=D0=BB?= =?UTF-8?q?=D0=BE=D0=B2=20=5BArtyom=20Pavlov=5D?= Date: Wed, 25 May 2022 05:14:30 +0300 Subject: [PATCH 06/10] tweak doctests --- library/core/src/time.rs | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/library/core/src/time.rs b/library/core/src/time.rs index 7f23cb83e09..6d098acb5ea 100644 --- a/library/core/src/time.rs +++ b/library/core/src/time.rs @@ -1348,9 +1348,6 @@ impl Duration { /// // subnormal float: /// let res = Duration::try_from_secs_f32(f32::from_bits(1)); /// assert_eq!(res, Ok(Duration::new(0, 0))); - /// // conversion uses rounding - /// let res = Duration::try_from_secs_f32(0.999e-9); - /// assert_eq!(res, Ok(Duration::new(0, 1))); /// /// let res = Duration::try_from_secs_f32(-5.0); /// assert!(res.is_err()); @@ -1359,7 +1356,9 @@ impl Duration { /// let res = Duration::try_from_secs_f32(2e19); /// assert!(res.is_err()); /// - /// // this method uses round to nearest, ties to even + /// // the conversion uses rounding with tie resolution to even + /// let res = Duration::try_from_secs_f32(0.999e-9); + /// assert_eq!(res, Ok(Duration::new(0, 1))); /// /// // this float represents exactly 976562.5e-9 /// let val = f32::from_bits(0x3A80_0000); @@ -1419,9 +1418,6 @@ impl Duration { /// // subnormal float /// let res = Duration::try_from_secs_f64(f64::from_bits(1)); /// assert_eq!(res, Ok(Duration::new(0, 0))); - /// // conversion uses rounding - /// let res = Duration::try_from_secs_f32(0.999e-9); - /// assert_eq!(res, Ok(Duration::new(0, 1))); /// /// let res = Duration::try_from_secs_f64(-5.0); /// assert!(res.is_err()); @@ -1430,7 +1426,9 @@ impl Duration { /// let res = Duration::try_from_secs_f64(2e19); /// assert!(res.is_err()); /// - /// // this method uses round to nearest, ties to even + /// // the conversion uses rounding with tie resolution to even + /// let res = Duration::try_from_secs_f64(0.999e-9); + /// assert_eq!(res, Ok(Duration::new(0, 1))); /// /// // this float represents exactly 976562.5e-9 /// let val = f64::from_bits(0x3F50_0000_0000_0000); From c838129205fa062059bf9de2158395649d9fde5c Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Thu, 26 May 2022 12:42:14 -0700 Subject: [PATCH 07/10] Update `triagebot.toml` for macos ping group --- triagebot.toml | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/triagebot.toml b/triagebot.toml index f6f1b918f06..994936c6e76 100644 --- a/triagebot.toml +++ b/triagebot.toml @@ -80,6 +80,13 @@ resolved/implemented on Fuchsia. Could one of you weigh in? """ label = "O-fuchsia" +[ping.macos] +message = """\ +Hey MacOS Group! This issue or PR could use some MacOS-specific guidance. Could one +of you weigh in? Thanks <3 +""" +label = "O-macos" + [prioritize] label = "I-prioritize" From 06af3a63a597a40604976666dea19a503557855a Mon Sep 17 00:00:00 2001 From: Artyom Pavlov Date: Fri, 27 May 2022 00:22:56 +0000 Subject: [PATCH 08/10] add debug asserts --- library/core/src/time.rs | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/library/core/src/time.rs b/library/core/src/time.rs index 6d098acb5ea..46fab01b1bf 100644 --- a/library/core/src/time.rs +++ b/library/core/src/time.rs @@ -1290,8 +1290,10 @@ macro_rules! try_from_secs { // note that neither `f32`, nor `f64` can represent // 0.999_999_999_5 exactly, so the nanos part - // never will be equal to 10^9. - (0, nanos + add_ns as u32) + // never will be equal to NANOS_PER_SEC + let nanos = nanos + add_ns as u32; + debug_assert!(nanos < NANOS_PER_SEC); + (0, nanos) } else if exp < $mant_bits { let secs = u64::from(mant >> ($mant_bits - exp)); let t = <$double_ty>::from((mant << exp) & MANT_MASK); @@ -1308,8 +1310,10 @@ macro_rules! try_from_secs { let add_ns = !(rem_msb || (is_even && is_tie)); // neither `f32`, nor `f64` can represent x.999_999_999_5 exactly, - // so the nanos part never will be equal to 10^9 - (secs, nanos + add_ns as u32) + // so the nanos part never will be equal to NANOS_PER_SEC + let nanos = nanos + add_ns as u32; + debug_assert!(nanos < NANOS_PER_SEC); + (secs, nanos) } else if exp < 64 { // the input has no fractional part let secs = u64::from(mant) << (exp - $mant_bits); From 38609cd8a9113969746e6a0598fca7d13788e8ec Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=D0=90=D1=80=D1=82=D1=91=D0=BC=20=D0=9F=D0=B0=D0=B2=D0=BB?= =?UTF-8?q?=D0=BE=D0=B2=20=5BArtyom=20Pavlov=5D?= Date: Fri, 27 May 2022 04:59:01 +0300 Subject: [PATCH 09/10] fix nanos overflow for f64 --- library/core/src/time.rs | 33 ++++++++++++++++++++++++--------- 1 file changed, 24 insertions(+), 9 deletions(-) diff --git a/library/core/src/time.rs b/library/core/src/time.rs index 46fab01b1bf..6983f10df3f 100644 --- a/library/core/src/time.rs +++ b/library/core/src/time.rs @@ -1288,12 +1288,14 @@ macro_rules! try_from_secs { let rem_msb = nanos_tmp & rem_msb_mask == 0; let add_ns = !(rem_msb || (is_even && is_tie)); - // note that neither `f32`, nor `f64` can represent - // 0.999_999_999_5 exactly, so the nanos part - // never will be equal to NANOS_PER_SEC + // f32 does not have enough presicion to trigger the second branch + // since it can not represent numbers between 0.999_999_940_395 and 1.0. let nanos = nanos + add_ns as u32; - debug_assert!(nanos < NANOS_PER_SEC); - (0, nanos) + if ($mant_bits == 23) || (nanos != NANOS_PER_SEC) { + (0, nanos) + } else { + (1, 0) + } } else if exp < $mant_bits { let secs = u64::from(mant >> ($mant_bits - exp)); let t = <$double_ty>::from((mant << exp) & MANT_MASK); @@ -1309,11 +1311,16 @@ macro_rules! try_from_secs { let rem_msb = nanos_tmp & rem_msb_mask == 0; let add_ns = !(rem_msb || (is_even && is_tie)); - // neither `f32`, nor `f64` can represent x.999_999_999_5 exactly, - // so the nanos part never will be equal to NANOS_PER_SEC + // f32 does not have enough presicion to trigger the second branch. + // For example, it can not represent numbers between 1.999_999_880... + // and 2.0. Bigger values result in even smaller presicion of the + // fractional part. let nanos = nanos + add_ns as u32; - debug_assert!(nanos < NANOS_PER_SEC); - (secs, nanos) + if ($mant_bits == 23) || (nanos != NANOS_PER_SEC) { + (secs, nanos) + } else { + (secs + 1, 0) + } } else if exp < 64 { // the input has no fractional part let secs = u64::from(mant) << (exp - $mant_bits); @@ -1433,6 +1440,14 @@ impl Duration { /// // the conversion uses rounding with tie resolution to even /// let res = Duration::try_from_secs_f64(0.999e-9); /// assert_eq!(res, Ok(Duration::new(0, 1))); + /// let res = Duration::try_from_secs_f64(0.999_999_999_499); + /// assert_eq!(res, Ok(Duration::new(0, 999_999_999))); + /// let res = Duration::try_from_secs_f64(0.999_999_999_501); + /// assert_eq!(res, Ok(Duration::new(1, 0))); + /// let res = Duration::try_from_secs_f64(42.999_999_999_499); + /// assert_eq!(res, Ok(Duration::new(42, 999_999_999))); + /// let res = Duration::try_from_secs_f64(42.999_999_999_501); + /// assert_eq!(res, Ok(Duration::new(43, 0))); /// /// // this float represents exactly 976562.5e-9 /// let val = f64::from_bits(0x3F50_0000_0000_0000); From 6495963d5a2d3e36ce799cfb932aa1a4b080f262 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=D0=90=D1=80=D1=82=D1=91=D0=BC=20=D0=9F=D0=B0=D0=B2=D0=BB?= =?UTF-8?q?=D0=BE=D0=B2=20=5BArtyom=20Pavlov=5D?= Date: Fri, 27 May 2022 05:15:22 +0300 Subject: [PATCH 10/10] fmt --- library/core/src/time.rs | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/library/core/src/time.rs b/library/core/src/time.rs index 6983f10df3f..d653f715f6d 100644 --- a/library/core/src/time.rs +++ b/library/core/src/time.rs @@ -1291,11 +1291,7 @@ macro_rules! try_from_secs { // f32 does not have enough presicion to trigger the second branch // since it can not represent numbers between 0.999_999_940_395 and 1.0. let nanos = nanos + add_ns as u32; - if ($mant_bits == 23) || (nanos != NANOS_PER_SEC) { - (0, nanos) - } else { - (1, 0) - } + if ($mant_bits == 23) || (nanos != NANOS_PER_SEC) { (0, nanos) } else { (1, 0) } } else if exp < $mant_bits { let secs = u64::from(mant >> ($mant_bits - exp)); let t = <$double_ty>::from((mant << exp) & MANT_MASK);