From 04d9bb7a9a697f71abc4aac849a262f713807f29 Mon Sep 17 00:00:00 2001 From: dianne Date: Mon, 25 Nov 2024 20:25:10 -0800 Subject: [PATCH 01/16] `add_move_error_suggestions`: use a HIR visitor rather than `SourceMap` --- .../src/diagnostics/move_errors.rs | 157 +++++++++++++----- tests/ui/issues/issue-12567.stderr | 28 ++-- .../ref_pat_eat_one_layer_2024_fail2.stderr | 7 +- ...oving-wrong-ref-pattern-issue-132806.fixed | 10 ++ ...removing-wrong-ref-pattern-issue-132806.rs | 10 ++ ...ving-wrong-ref-pattern-issue-132806.stderr | 18 ++ tests/ui/nll/move-errors.stderr | 7 +- .../cant_move_out_of_pattern.stderr | 10 ++ .../ui/suggestions/dont-suggest-ref/simple.rs | 22 +-- .../dont-suggest-ref/simple.stderr | 77 +++++---- ...ption-content-move-from-tuple-match.stderr | 7 +- 11 files changed, 249 insertions(+), 104 deletions(-) create mode 100644 tests/ui/moves/do-not-suggest-removing-wrong-ref-pattern-issue-132806.fixed create mode 100644 tests/ui/moves/do-not-suggest-removing-wrong-ref-pattern-issue-132806.rs create mode 100644 tests/ui/moves/do-not-suggest-removing-wrong-ref-pattern-issue-132806.stderr diff --git a/compiler/rustc_borrowck/src/diagnostics/move_errors.rs b/compiler/rustc_borrowck/src/diagnostics/move_errors.rs index 4ba6b2e94ec56..beacbdbd3fa7b 100644 --- a/compiler/rustc_borrowck/src/diagnostics/move_errors.rs +++ b/compiler/rustc_borrowck/src/diagnostics/move_errors.rs @@ -1,9 +1,10 @@ #![allow(rustc::diagnostic_outside_of_impl)] #![allow(rustc::untranslatable_diagnostic)] +use rustc_data_structures::fx::FxHashSet; use rustc_errors::{Applicability, Diag}; use rustc_hir::intravisit::Visitor; -use rustc_hir::{CaptureBy, ExprKind, HirId, Node}; +use rustc_hir::{self as hir, CaptureBy, ExprKind, HirId, Node}; use rustc_middle::bug; use rustc_middle::mir::*; use rustc_middle::ty::{self, Ty}; @@ -683,48 +684,126 @@ impl<'infcx, 'tcx> MirBorrowckCtxt<'_, 'infcx, 'tcx> { } fn add_move_error_suggestions(&self, err: &mut Diag<'_>, binds_to: &[Local]) { - let mut suggestions: Vec<(Span, String, String)> = Vec::new(); + /// A HIR visitor to associate each binding with a `&` or `&mut` that could be removed to + /// make it bind by reference instead (if possible) + struct BindingFinder<'tcx> { + typeck_results: &'tcx ty::TypeckResults<'tcx>, + hir: rustc_middle::hir::map::Map<'tcx>, + /// Input: the span of the pattern we're finding bindings in + pat_span: Span, + /// Input: the spans of the bindings we're providing suggestions for + binding_spans: Vec, + /// Internal state: have we reached the pattern we're finding bindings in? + found_pat: bool, + /// Internal state: the innermost `&` or `&mut` "above" the visitor + ref_pat: Option<&'tcx hir::Pat<'tcx>>, + /// Internal state: could removing a `&` give bindings unexpected types? + has_adjustments: bool, + /// Output: for each input binding, the `&` or `&mut` to remove to make it by-ref + ref_pat_for_binding: Vec<(Span, Option<&'tcx hir::Pat<'tcx>>)>, + /// Output: ref patterns that can't be removed straightforwardly + cannot_remove: FxHashSet, + } + impl<'tcx> Visitor<'tcx> for BindingFinder<'tcx> { + type NestedFilter = rustc_middle::hir::nested_filter::OnlyBodies; + + fn nested_visit_map(&mut self) -> Self::Map { + self.hir + } + + fn visit_expr(&mut self, ex: &'tcx hir::Expr<'tcx>) -> Self::Result { + // Don't walk into const patterns or anything else that might confuse this + if !self.found_pat { + hir::intravisit::walk_expr(self, ex) + } + } + + fn visit_pat(&mut self, p: &'tcx hir::Pat<'tcx>) { + if p.span == self.pat_span { + self.found_pat = true; + } + + let parent_has_adjustments = self.has_adjustments; + self.has_adjustments |= + self.typeck_results.pat_adjustments().contains_key(p.hir_id); + + // Track the innermost `&` or `&mut` enclosing bindings, to suggest removing it. + let parent_ref_pat = self.ref_pat; + if let hir::PatKind::Ref(..) = p.kind { + self.ref_pat = Some(p); + // To avoid edition-dependent logic to figure out how many refs this `&` can + // peel off, simply don't remove the "parent" `&`. + self.cannot_remove.extend(parent_ref_pat.map(|r| r.hir_id)); + if self.has_adjustments { + // Removing this `&` could give child bindings unexpected types, so don't. + self.cannot_remove.insert(p.hir_id); + // As long the `&` stays, child patterns' types should be as expected. + self.has_adjustments = false; + } + } + + if let hir::PatKind::Binding(_, _, ident, _) = p.kind { + // the spans in `binding_spans` encompass both the ident and binding mode + if let Some(&bind_sp) = + self.binding_spans.iter().find(|bind_sp| bind_sp.contains(ident.span)) + { + self.ref_pat_for_binding.push((bind_sp, self.ref_pat)); + } else { + // we've encountered a binding that we're not reporting a move error for. + // we don't want to change its type, so don't remove the surrounding `&`. + if let Some(ref_pat) = self.ref_pat { + self.cannot_remove.insert(ref_pat.hir_id); + } + } + } + + hir::intravisit::walk_pat(self, p); + self.ref_pat = parent_ref_pat; + self.has_adjustments = parent_has_adjustments; + } + } + let mut pat_span = None; + let mut binding_spans = Vec::new(); for local in binds_to { let bind_to = &self.body.local_decls[*local]; - if let LocalInfo::User(BindingForm::Var(VarBindingForm { pat_span, .. })) = + if let LocalInfo::User(BindingForm::Var(VarBindingForm { pat_span: pat_sp, .. })) = *bind_to.local_info() { - let Ok(pat_snippet) = self.infcx.tcx.sess.source_map().span_to_snippet(pat_span) - else { - continue; - }; - let Some(stripped) = pat_snippet.strip_prefix('&') else { - suggestions.push(( - bind_to.source_info.span.shrink_to_lo(), - "consider borrowing the pattern binding".to_string(), - "ref ".to_string(), - )); - continue; - }; - let inner_pat_snippet = stripped.trim_start(); - let (pat_span, suggestion, to_remove) = if inner_pat_snippet.starts_with("mut") - && inner_pat_snippet["mut".len()..].starts_with(rustc_lexer::is_whitespace) - { - let inner_pat_snippet = inner_pat_snippet["mut".len()..].trim_start(); - let pat_span = pat_span.with_hi( - pat_span.lo() - + BytePos((pat_snippet.len() - inner_pat_snippet.len()) as u32), - ); - (pat_span, String::new(), "mutable borrow") - } else { - let pat_span = pat_span.with_hi( - pat_span.lo() - + BytePos( - (pat_snippet.len() - inner_pat_snippet.trim_start().len()) as u32, - ), - ); - (pat_span, String::new(), "borrow") - }; - suggestions.push(( - pat_span, - format!("consider removing the {to_remove}"), - suggestion, - )); + pat_span = Some(pat_sp); + binding_spans.push(bind_to.source_info.span); + } + } + let Some(pat_span) = pat_span else { return }; + + let hir = self.infcx.tcx.hir(); + let Some(body) = hir.maybe_body_owned_by(self.mir_def_id()) else { return }; + let typeck_results = self.infcx.tcx.typeck(self.mir_def_id()); + let mut finder = BindingFinder { + typeck_results, + hir, + pat_span, + binding_spans, + found_pat: false, + ref_pat: None, + has_adjustments: false, + ref_pat_for_binding: Vec::new(), + cannot_remove: FxHashSet::default(), + }; + finder.visit_body(body); + + let mut suggestions = Vec::new(); + for (binding_span, opt_ref_pat) in finder.ref_pat_for_binding { + if let Some(ref_pat) = opt_ref_pat + && !finder.cannot_remove.contains(&ref_pat.hir_id) + && let hir::PatKind::Ref(subpat, mutbl) = ref_pat.kind + && let Some(ref_span) = ref_pat.span.trim_end(subpat.span) + { + let mutable_str = if mutbl.is_mut() { "mutable " } else { "" }; + let msg = format!("consider removing the {mutable_str}borrow"); + suggestions.push((ref_span, msg, "".to_string())); + } else { + let msg = "consider borrowing the pattern binding".to_string(); + suggestions.push((binding_span.shrink_to_lo(), msg, "ref ".to_string())); } } suggestions.sort_unstable_by_key(|&(span, _, _)| span); diff --git a/tests/ui/issues/issue-12567.stderr b/tests/ui/issues/issue-12567.stderr index 3f95f18a96743..0b19299ece3ec 100644 --- a/tests/ui/issues/issue-12567.stderr +++ b/tests/ui/issues/issue-12567.stderr @@ -11,14 +11,16 @@ LL | (&[hd1, ..], &[hd2, ..]) | --- ...and here | = note: move occurs because these variables have types that don't implement the `Copy` trait -help: consider borrowing the pattern binding +help: consider removing the borrow | -LL | (&[], &[ref hd, ..]) | (&[hd, ..], &[]) - | +++ -help: consider borrowing the pattern binding +LL - (&[], &[hd, ..]) | (&[hd, ..], &[]) +LL + (&[], [hd, ..]) | (&[hd, ..], &[]) + | +help: consider removing the borrow + | +LL - (&[hd1, ..], &[hd2, ..]) +LL + (&[hd1, ..], [hd2, ..]) | -LL | (&[hd1, ..], &[ref hd2, ..]) - | +++ error[E0508]: cannot move out of type `[T]`, a non-copy slice --> $DIR/issue-12567.rs:2:11 @@ -33,14 +35,16 @@ LL | (&[hd1, ..], &[hd2, ..]) | --- ...and here | = note: move occurs because these variables have types that don't implement the `Copy` trait -help: consider borrowing the pattern binding +help: consider removing the borrow + | +LL - (&[], &[hd, ..]) | (&[hd, ..], &[]) +LL + (&[], [hd, ..]) | (&[hd, ..], &[]) + | +help: consider removing the borrow | -LL | (&[], &[ref hd, ..]) | (&[hd, ..], &[]) - | +++ -help: consider borrowing the pattern binding +LL - (&[hd1, ..], &[hd2, ..]) +LL + ([hd1, ..], &[hd2, ..]) | -LL | (&[ref hd1, ..], &[hd2, ..]) - | +++ error: aborting due to 2 previous errors diff --git a/tests/ui/match/ref_pat_eat_one_layer_2024/ref_pat_eat_one_layer_2024_fail2.stderr b/tests/ui/match/ref_pat_eat_one_layer_2024/ref_pat_eat_one_layer_2024_fail2.stderr index 52f4c09e5c024..a8b813941109c 100644 --- a/tests/ui/match/ref_pat_eat_one_layer_2024/ref_pat_eat_one_layer_2024_fail2.stderr +++ b/tests/ui/match/ref_pat_eat_one_layer_2024/ref_pat_eat_one_layer_2024_fail2.stderr @@ -7,10 +7,11 @@ LL | if let Some(&Some(x)) = Some(&Some(&mut 0)) { | data moved here | move occurs because `x` has type `&mut u32`, which does not implement the `Copy` trait | -help: consider borrowing the pattern binding +help: consider removing the borrow + | +LL - if let Some(&Some(x)) = Some(&Some(&mut 0)) { +LL + if let Some(Some(x)) = Some(&Some(&mut 0)) { | -LL | if let Some(&Some(ref x)) = Some(&Some(&mut 0)) { - | +++ error[E0596]: cannot borrow data in a `&` reference as mutable --> $DIR/ref_pat_eat_one_layer_2024_fail2.rs:11:10 diff --git a/tests/ui/moves/do-not-suggest-removing-wrong-ref-pattern-issue-132806.fixed b/tests/ui/moves/do-not-suggest-removing-wrong-ref-pattern-issue-132806.fixed new file mode 100644 index 0000000000000..46b05e4c0a3c2 --- /dev/null +++ b/tests/ui/moves/do-not-suggest-removing-wrong-ref-pattern-issue-132806.fixed @@ -0,0 +1,10 @@ +//@ run-rustfix +//! diagnostic test for #132806: make sure the suggestion to bind by-reference in patterns doesn't +//! erroneously remove the wrong `&` + +use std::collections::HashMap; + +fn main() { + let _ = HashMap::::new().iter().filter(|&(_k, &_v)| { true }); + //~^ ERROR cannot move out of a shared reference +} diff --git a/tests/ui/moves/do-not-suggest-removing-wrong-ref-pattern-issue-132806.rs b/tests/ui/moves/do-not-suggest-removing-wrong-ref-pattern-issue-132806.rs new file mode 100644 index 0000000000000..1312fd6425b65 --- /dev/null +++ b/tests/ui/moves/do-not-suggest-removing-wrong-ref-pattern-issue-132806.rs @@ -0,0 +1,10 @@ +//@ run-rustfix +//! diagnostic test for #132806: make sure the suggestion to bind by-reference in patterns doesn't +//! erroneously remove the wrong `&` + +use std::collections::HashMap; + +fn main() { + let _ = HashMap::::new().iter().filter(|&(&_k, &_v)| { true }); + //~^ ERROR cannot move out of a shared reference +} diff --git a/tests/ui/moves/do-not-suggest-removing-wrong-ref-pattern-issue-132806.stderr b/tests/ui/moves/do-not-suggest-removing-wrong-ref-pattern-issue-132806.stderr new file mode 100644 index 0000000000000..ff579f934137f --- /dev/null +++ b/tests/ui/moves/do-not-suggest-removing-wrong-ref-pattern-issue-132806.stderr @@ -0,0 +1,18 @@ +error[E0507]: cannot move out of a shared reference + --> $DIR/do-not-suggest-removing-wrong-ref-pattern-issue-132806.rs:8:58 + | +LL | let _ = HashMap::::new().iter().filter(|&(&_k, &_v)| { true }); + | ^^^--^^^^^^ + | | + | data moved here + | move occurs because `_k` has type `String`, which does not implement the `Copy` trait + | +help: consider removing the borrow + | +LL - let _ = HashMap::::new().iter().filter(|&(&_k, &_v)| { true }); +LL + let _ = HashMap::::new().iter().filter(|&(_k, &_v)| { true }); + | + +error: aborting due to 1 previous error + +For more information about this error, try `rustc --explain E0507`. diff --git a/tests/ui/nll/move-errors.stderr b/tests/ui/nll/move-errors.stderr index d138412137959..bcb2ab84a239b 100644 --- a/tests/ui/nll/move-errors.stderr +++ b/tests/ui/nll/move-errors.stderr @@ -209,10 +209,11 @@ LL | (D(s), &t) => (), | data moved here | move occurs because `t` has type `String`, which does not implement the `Copy` trait | -help: consider borrowing the pattern binding +help: consider removing the borrow + | +LL - (D(s), &t) => (), +LL + (D(s), t) => (), | -LL | (D(s), &ref t) => (), - | +++ error[E0509]: cannot move out of type `F`, which implements the `Drop` trait --> $DIR/move-errors.rs:102:11 diff --git a/tests/ui/pattern/deref-patterns/cant_move_out_of_pattern.stderr b/tests/ui/pattern/deref-patterns/cant_move_out_of_pattern.stderr index 108db6d9e4b53..2cf435b117999 100644 --- a/tests/ui/pattern/deref-patterns/cant_move_out_of_pattern.stderr +++ b/tests/ui/pattern/deref-patterns/cant_move_out_of_pattern.stderr @@ -9,6 +9,11 @@ LL | deref!(x) => x, | | | data moved here | move occurs because `x` has type `Struct`, which does not implement the `Copy` trait + | +help: consider borrowing the pattern binding + | +LL | deref!(ref x) => x, + | +++ error[E0507]: cannot move out of a shared reference --> $DIR/cant_move_out_of_pattern.rs:17:11 @@ -21,6 +26,11 @@ LL | deref!(x) => x, | | | data moved here | move occurs because `x` has type `Struct`, which does not implement the `Copy` trait + | +help: consider borrowing the pattern binding + | +LL | deref!(ref x) => x, + | +++ error: aborting due to 2 previous errors diff --git a/tests/ui/suggestions/dont-suggest-ref/simple.rs b/tests/ui/suggestions/dont-suggest-ref/simple.rs index 1e40e60a1ce12..4dea5319264ed 100644 --- a/tests/ui/suggestions/dont-suggest-ref/simple.rs +++ b/tests/ui/suggestions/dont-suggest-ref/simple.rs @@ -219,42 +219,42 @@ pub fn main() { let (&X(_t),) = (&x.clone(),); //~^ ERROR cannot move - //~| HELP consider borrowing the pattern binding + //~| HELP consider removing the borrow if let (&Either::One(_t),) = (&e.clone(),) { } //~^ ERROR cannot move - //~| HELP consider borrowing the pattern binding + //~| HELP consider removing the borrow while let (&Either::One(_t),) = (&e.clone(),) { } //~^ ERROR cannot move - //~| HELP consider borrowing the pattern binding + //~| HELP consider removing the borrow match (&e.clone(),) { //~^ ERROR cannot move (&Either::One(_t),) - //~^ HELP consider borrowing the pattern binding + //~^ HELP consider removing the borrow | (&Either::Two(_t),) => (), } fn f3((&X(_t),): (&X,)) { } //~^ ERROR cannot move - //~| HELP consider borrowing the pattern binding + //~| HELP consider removing the borrow let (&mut X(_t),) = (&mut xm.clone(),); //~^ ERROR cannot move - //~| HELP consider borrowing the pattern binding + //~| HELP consider removing the mutable borrow if let (&mut Either::One(_t),) = (&mut em.clone(),) { } //~^ ERROR cannot move - //~| HELP consider borrowing the pattern binding + //~| HELP consider removing the mutable borrow while let (&mut Either::One(_t),) = (&mut em.clone(),) { } //~^ ERROR cannot move - //~| HELP consider borrowing the pattern binding + //~| HELP consider removing the mutable borrow match (&mut em.clone(),) { //~^ ERROR cannot move (&mut Either::One(_t),) => (), - //~^ HELP consider borrowing the pattern binding + //~^ HELP consider removing the mutable borrow (&mut Either::Two(_t),) => (), - //~^ HELP consider borrowing the pattern binding + //~^ HELP consider removing the mutable borrow } fn f4((&mut X(_t),): (&mut X,)) { } //~^ ERROR cannot move - //~| HELP consider borrowing the pattern binding + //~| HELP consider removing the mutable borrow // move from &Either/&X value diff --git a/tests/ui/suggestions/dont-suggest-ref/simple.stderr b/tests/ui/suggestions/dont-suggest-ref/simple.stderr index 7d902dbccc4d4..41571bf9b2ca4 100644 --- a/tests/ui/suggestions/dont-suggest-ref/simple.stderr +++ b/tests/ui/suggestions/dont-suggest-ref/simple.stderr @@ -578,10 +578,11 @@ LL | let (&X(_t),) = (&x.clone(),); | data moved here | move occurs because `_t` has type `Y`, which does not implement the `Copy` trait | -help: consider borrowing the pattern binding +help: consider removing the borrow + | +LL - let (&X(_t),) = (&x.clone(),); +LL + let (X(_t),) = (&x.clone(),); | -LL | let (&X(ref _t),) = (&x.clone(),); - | +++ error[E0507]: cannot move out of a shared reference --> $DIR/simple.rs:223:34 @@ -592,10 +593,11 @@ LL | if let (&Either::One(_t),) = (&e.clone(),) { } | data moved here | move occurs because `_t` has type `X`, which does not implement the `Copy` trait | -help: consider borrowing the pattern binding +help: consider removing the borrow + | +LL - if let (&Either::One(_t),) = (&e.clone(),) { } +LL + if let (Either::One(_t),) = (&e.clone(),) { } | -LL | if let (&Either::One(ref _t),) = (&e.clone(),) { } - | +++ error[E0507]: cannot move out of a shared reference --> $DIR/simple.rs:226:37 @@ -606,10 +608,11 @@ LL | while let (&Either::One(_t),) = (&e.clone(),) { } | data moved here | move occurs because `_t` has type `X`, which does not implement the `Copy` trait | -help: consider borrowing the pattern binding +help: consider removing the borrow + | +LL - while let (&Either::One(_t),) = (&e.clone(),) { } +LL + while let (Either::One(_t),) = (&e.clone(),) { } | -LL | while let (&Either::One(ref _t),) = (&e.clone(),) { } - | +++ error[E0507]: cannot move out of a shared reference --> $DIR/simple.rs:229:11 @@ -623,10 +626,11 @@ LL | (&Either::One(_t),) | data moved here | move occurs because `_t` has type `X`, which does not implement the `Copy` trait | -help: consider borrowing the pattern binding +help: consider removing the borrow + | +LL - (&Either::One(_t),) +LL + (Either::One(_t),) | -LL | (&Either::One(ref _t),) - | +++ error[E0507]: cannot move out of a mutable reference --> $DIR/simple.rs:239:25 @@ -637,10 +641,11 @@ LL | let (&mut X(_t),) = (&mut xm.clone(),); | data moved here | move occurs because `_t` has type `Y`, which does not implement the `Copy` trait | -help: consider borrowing the pattern binding +help: consider removing the mutable borrow + | +LL - let (&mut X(_t),) = (&mut xm.clone(),); +LL + let (X(_t),) = (&mut xm.clone(),); | -LL | let (&mut X(ref _t),) = (&mut xm.clone(),); - | +++ error[E0507]: cannot move out of a mutable reference --> $DIR/simple.rs:242:38 @@ -651,10 +656,11 @@ LL | if let (&mut Either::One(_t),) = (&mut em.clone(),) { } | data moved here | move occurs because `_t` has type `X`, which does not implement the `Copy` trait | -help: consider borrowing the pattern binding +help: consider removing the mutable borrow + | +LL - if let (&mut Either::One(_t),) = (&mut em.clone(),) { } +LL + if let (Either::One(_t),) = (&mut em.clone(),) { } | -LL | if let (&mut Either::One(ref _t),) = (&mut em.clone(),) { } - | +++ error[E0507]: cannot move out of a mutable reference --> $DIR/simple.rs:245:41 @@ -665,10 +671,11 @@ LL | while let (&mut Either::One(_t),) = (&mut em.clone(),) { } | data moved here | move occurs because `_t` has type `X`, which does not implement the `Copy` trait | -help: consider borrowing the pattern binding +help: consider removing the mutable borrow + | +LL - while let (&mut Either::One(_t),) = (&mut em.clone(),) { } +LL + while let (Either::One(_t),) = (&mut em.clone(),) { } | -LL | while let (&mut Either::One(ref _t),) = (&mut em.clone(),) { } - | +++ error[E0507]: cannot move out of a mutable reference --> $DIR/simple.rs:248:11 @@ -683,14 +690,16 @@ LL | (&mut Either::Two(_t),) => (), | -- ...and here | = note: move occurs because these variables have types that don't implement the `Copy` trait -help: consider borrowing the pattern binding +help: consider removing the mutable borrow | -LL | (&mut Either::One(ref _t),) => (), - | +++ -help: consider borrowing the pattern binding +LL - (&mut Either::One(_t),) => (), +LL + (Either::One(_t),) => (), + | +help: consider removing the mutable borrow + | +LL - (&mut Either::Two(_t),) => (), +LL + (Either::Two(_t),) => (), | -LL | (&mut Either::Two(ref _t),) => (), - | +++ error[E0507]: cannot move out of a shared reference --> $DIR/simple.rs:261:18 @@ -947,10 +956,11 @@ LL | fn f3((&X(_t),): (&X,)) { } | data moved here | move occurs because `_t` has type `Y`, which does not implement the `Copy` trait | -help: consider borrowing the pattern binding +help: consider removing the borrow + | +LL - fn f3((&X(_t),): (&X,)) { } +LL + fn f3((X(_t),): (&X,)) { } | -LL | fn f3((&X(ref _t),): (&X,)) { } - | +++ error[E0507]: cannot move out of a mutable reference --> $DIR/simple.rs:255:11 @@ -961,10 +971,11 @@ LL | fn f4((&mut X(_t),): (&mut X,)) { } | data moved here | move occurs because `_t` has type `Y`, which does not implement the `Copy` trait | -help: consider borrowing the pattern binding +help: consider removing the mutable borrow + | +LL - fn f4((&mut X(_t),): (&mut X,)) { } +LL + fn f4((X(_t),): (&mut X,)) { } | -LL | fn f4((&mut X(ref _t),): (&mut X,)) { } - | +++ error[E0507]: cannot move out of `a.a` as enum variant `Some` which is behind a shared reference --> $DIR/simple.rs:331:20 diff --git a/tests/ui/suggestions/option-content-move-from-tuple-match.stderr b/tests/ui/suggestions/option-content-move-from-tuple-match.stderr index 63314acb87cbf..c93570c579ee7 100644 --- a/tests/ui/suggestions/option-content-move-from-tuple-match.stderr +++ b/tests/ui/suggestions/option-content-move-from-tuple-match.stderr @@ -10,10 +10,11 @@ LL | (None, &c) => &c.unwrap(), | data moved here | move occurs because `c` has type `Option`, which does not implement the `Copy` trait | -help: consider borrowing the pattern binding +help: consider removing the borrow + | +LL - (None, &c) => &c.unwrap(), +LL + (None, c) => &c.unwrap(), | -LL | (None, &ref c) => &c.unwrap(), - | +++ error: aborting due to 1 previous error From ce6a06802fe901bf18d07f8bd1a94973c00fa124 Mon Sep 17 00:00:00 2001 From: Trevor Gross Date: Mon, 9 Dec 2024 09:25:21 +0000 Subject: [PATCH 02/16] float: Update some constants to `pub(crate)` These constants can be useful outside of their current module. Make them `pub(crate)` to allow for this. --- library/core/src/num/f32.rs | 6 +++--- library/core/src/num/f64.rs | 6 +++--- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/library/core/src/num/f32.rs b/library/core/src/num/f32.rs index 4c0d95f95e562..8eba037e38fe2 100644 --- a/library/core/src/num/f32.rs +++ b/library/core/src/num/f32.rs @@ -493,13 +493,13 @@ impl f32 { pub const NEG_INFINITY: f32 = -1.0_f32 / 0.0_f32; /// Sign bit - const SIGN_MASK: u32 = 0x8000_0000; + pub(crate) const SIGN_MASK: u32 = 0x8000_0000; /// Exponent mask - const EXP_MASK: u32 = 0x7f80_0000; + pub(crate) const EXP_MASK: u32 = 0x7f80_0000; /// Mantissa mask - const MAN_MASK: u32 = 0x007f_ffff; + pub(crate) const MAN_MASK: u32 = 0x007f_ffff; /// Minimum representable positive value (min subnormal) const TINY_BITS: u32 = 0x1; diff --git a/library/core/src/num/f64.rs b/library/core/src/num/f64.rs index 77ca56df06705..7971e017f9175 100644 --- a/library/core/src/num/f64.rs +++ b/library/core/src/num/f64.rs @@ -492,13 +492,13 @@ impl f64 { pub const NEG_INFINITY: f64 = -1.0_f64 / 0.0_f64; /// Sign bit - const SIGN_MASK: u64 = 0x8000_0000_0000_0000; + pub(crate) const SIGN_MASK: u64 = 0x8000_0000_0000_0000; /// Exponent mask - const EXP_MASK: u64 = 0x7ff0_0000_0000_0000; + pub(crate) const EXP_MASK: u64 = 0x7ff0_0000_0000_0000; /// Mantissa mask - const MAN_MASK: u64 = 0x000f_ffff_ffff_ffff; + pub(crate) const MAN_MASK: u64 = 0x000f_ffff_ffff_ffff; /// Minimum representable positive value (min subnormal) const TINY_BITS: u64 = 0x1; From 4394b211b1450b9746006ad1a453a9ddccb60967 Mon Sep 17 00:00:00 2001 From: Trevor Gross Date: Mon, 9 Dec 2024 09:25:22 +0000 Subject: [PATCH 03/16] dec2flt: Update documentation of existing methods Fix or elaborate existing float parsing documentation. This includes introducing a convention that should make naming more consistent. --- library/core/src/num/dec2flt/common.rs | 14 ++++++++------ library/core/src/num/dec2flt/decimal.rs | 22 +++++++++++++--------- library/core/src/num/dec2flt/mod.rs | 16 ++++++++++++++-- 3 files changed, 35 insertions(+), 17 deletions(-) diff --git a/library/core/src/num/dec2flt/common.rs b/library/core/src/num/dec2flt/common.rs index 4dadf406ae8c7..1646d3a95d3b1 100644 --- a/library/core/src/num/dec2flt/common.rs +++ b/library/core/src/num/dec2flt/common.rs @@ -8,12 +8,12 @@ pub(crate) trait ByteSlice { /// Writes a 64-bit integer as 8 bytes in little-endian order. fn write_u64(&mut self, value: u64); - /// Calculate the offset of a slice from another. + /// Calculate the difference in length between two slices. fn offset_from(&self, other: &Self) -> isize; /// Iteratively parse and consume digits from bytes. - /// Returns the same bytes with consumed digits being - /// elided. + /// + /// Returns the same bytes with consumed digits being elided. Breaks on invalid digits. fn parse_digits(&self, func: impl FnMut(u8)) -> &Self; } @@ -39,11 +39,11 @@ impl ByteSlice for [u8] { fn parse_digits(&self, mut func: impl FnMut(u8)) -> &Self { let mut s = self; - while let Some((c, s_next)) = s.split_first() { + while let Some((c, rest)) = s.split_first() { let c = c.wrapping_sub(b'0'); if c < 10 { func(c); - s = s_next; + s = rest; } else { break; } @@ -53,7 +53,9 @@ impl ByteSlice for [u8] { } } -/// Determine if 8 bytes are all decimal digits. +/// Determine if all characters in an 8-byte byte string (represented as a `u64`) are all decimal +/// digits. +/// /// This does not care about the order in which the bytes were loaded. pub(crate) fn is_8digits(v: u64) -> bool { let a = v.wrapping_add(0x4646_4646_4646_4646); diff --git a/library/core/src/num/dec2flt/decimal.rs b/library/core/src/num/dec2flt/decimal.rs index be9c0eccd5eb8..a89baa78fd263 100644 --- a/library/core/src/num/dec2flt/decimal.rs +++ b/library/core/src/num/dec2flt/decimal.rs @@ -1,4 +1,4 @@ -//! Arbitrary-precision decimal class for fallback algorithms. +//! Arbitrary-precision decimal type used by fallback algorithms. //! //! This is only used if the fast-path (native floats) and //! the Eisel-Lemire algorithm are unable to unambiguously @@ -11,6 +11,7 @@ use crate::num::dec2flt::common::{ByteSlice, is_8digits}; +/// A decimal floating-point number. #[derive(Clone)] pub struct Decimal { /// The number of significant digits in the decimal. @@ -30,18 +31,17 @@ impl Default for Decimal { } impl Decimal { - /// The maximum number of digits required to unambiguously round a float. + /// The maximum number of digits required to unambiguously round up to a 64-bit float. /// - /// For a double-precision IEEE 754 float, this required 767 digits, - /// so we store the max digits + 1. + /// For an IEEE 754 binary64 float, this required 767 digits. So we store the max digits + 1. /// /// We can exactly represent a float in radix `b` from radix 2 if /// `b` is divisible by 2. This function calculates the exact number of /// digits required to exactly represent that float. /// /// According to the "Handbook of Floating Point Arithmetic", - /// for IEEE754, with emin being the min exponent, p2 being the - /// precision, and b being the radix, the number of digits follows as: + /// for IEEE754, with `emin` being the min exponent, `p2` being the + /// precision, and `b` being the radix, the number of digits follows as: /// /// `−emin + p2 + ⌊(emin + 1) log(2, b) − log(1 − 2^(−p2), b)⌋` /// @@ -56,11 +56,14 @@ impl Decimal { /// In Python: /// `-emin + p2 + math.floor((emin+ 1)*math.log(2, b)-math.log(1-2**(-p2), b))` pub const MAX_DIGITS: usize = 768; - /// The max digits that can be exactly represented in a 64-bit integer. + /// The max decimal digits that can be exactly represented in a 64-bit integer. pub const MAX_DIGITS_WITHOUT_OVERFLOW: usize = 19; pub const DECIMAL_POINT_RANGE: i32 = 2047; - /// Append a digit to the buffer. + /// Append a digit to the buffer if it fits. + // FIXME(tgross35): it may be better for this to return an option + // FIXME(tgross35): incrementing the digit counter even if we don't push anything + // seems incorrect. pub fn try_add_digit(&mut self, digit: u8) { if self.num_digits < Self::MAX_DIGITS { self.digits[self.num_digits] = digit; @@ -69,6 +72,7 @@ impl Decimal { } /// Trim trailing zeros from the buffer. + // FIXME(tgross35): this could be `.rev().position()` if perf is okay pub fn trim(&mut self) { // All of the following calls to `Decimal::trim` can't panic because: // @@ -86,7 +90,7 @@ impl Decimal { pub fn round(&self) -> u64 { if self.num_digits == 0 || self.decimal_point < 0 { return 0; - } else if self.decimal_point > 18 { + } else if self.decimal_point >= Self::MAX_DIGITS_WITHOUT_OVERFLOW as i32 { return 0xFFFF_FFFF_FFFF_FFFF_u64; } let dp = self.decimal_point as usize; diff --git a/library/core/src/num/dec2flt/mod.rs b/library/core/src/num/dec2flt/mod.rs index 6dca740684537..91bfe1bef2e77 100644 --- a/library/core/src/num/dec2flt/mod.rs +++ b/library/core/src/num/dec2flt/mod.rs @@ -3,8 +3,8 @@ //! # Problem statement //! //! We are given a decimal string such as `12.34e56`. This string consists of integral (`12`), -//! fractional (`34`), and exponent (`56`) parts. All parts are optional and interpreted as zero -//! when missing. +//! fractional (`34`), and exponent (`56`) parts. All parts are optional and interpreted as a +//! default value (1 or 0) when missing. //! //! We seek the IEEE 754 floating point number that is closest to the exact value of the decimal //! string. It is well-known that many decimal strings do not have terminating representations in @@ -67,6 +67,18 @@ //! "such that the exponent +/- the number of decimal digits fits into a 64 bit integer". //! Larger exponents are accepted, but we don't do arithmetic with them, they are immediately //! turned into {positive,negative} {zero,infinity}. +//! +//! # Notation +//! +//! This module uses the same notation as the Lemire paper: +//! +//! - `m`: binary mantissa; always nonnegative +//! - `p`: binary exponent; a signed integer +//! - `w`: decimal significand; always nonnegative +//! - `q`: decimal exponent; a signed integer +//! +//! This gives `m * 2^p` for the binary floating-point number, with `w * 10^q` as the decimal +//! equivalent. #![doc(hidden)] #![unstable( From f75b04593c89706b987b6219e8a7188828995c2c Mon Sep 17 00:00:00 2001 From: Trevor Gross Date: Mon, 9 Dec 2024 09:25:22 +0000 Subject: [PATCH 04/16] dec2flt: Rename `Decimal` to `DecimalSeq` This module currently contains two decimal types, `Decimal` and `Number`. These names don't provide a whole lot of insight into what exactly they are, and `Number` is actually the one that is more like an expected `Decimal` type. In accordance with this, rename the existing `Decimal` to `DecimalSeq`. This highlights that it contains a sequence of decimal digits, rather than representing a base-10 floating point (decimal) number. Additionally, add some tests to validate internal behavior. --- .../dec2flt/{decimal.rs => decimal_seq.rs} | 46 +++++++++++++------ library/core/src/num/dec2flt/mod.rs | 2 +- library/core/src/num/dec2flt/slow.rs | 8 ++-- library/core/tests/num/dec2flt/decimal_seq.rs | 30 ++++++++++++ library/core/tests/num/dec2flt/mod.rs | 1 + 5 files changed, 67 insertions(+), 20 deletions(-) rename library/core/src/num/dec2flt/{decimal.rs => decimal_seq.rs} (94%) create mode 100644 library/core/tests/num/dec2flt/decimal_seq.rs diff --git a/library/core/src/num/dec2flt/decimal.rs b/library/core/src/num/dec2flt/decimal_seq.rs similarity index 94% rename from library/core/src/num/dec2flt/decimal.rs rename to library/core/src/num/dec2flt/decimal_seq.rs index a89baa78fd263..75ffb963930f5 100644 --- a/library/core/src/num/dec2flt/decimal.rs +++ b/library/core/src/num/dec2flt/decimal_seq.rs @@ -11,9 +11,9 @@ use crate::num::dec2flt::common::{ByteSlice, is_8digits}; -/// A decimal floating-point number. -#[derive(Clone)] -pub struct Decimal { +/// A decimal floating-point number, represented as a sequence of decimal digits. +#[derive(Clone, Debug, PartialEq)] +pub struct DecimalSeq { /// The number of significant digits in the decimal. pub num_digits: usize, /// The offset of the decimal point in the significant digits. @@ -24,13 +24,13 @@ pub struct Decimal { pub digits: [u8; Self::MAX_DIGITS], } -impl Default for Decimal { +impl Default for DecimalSeq { fn default() -> Self { Self { num_digits: 0, decimal_point: 0, truncated: false, digits: [0; Self::MAX_DIGITS] } } } -impl Decimal { +impl DecimalSeq { /// The maximum number of digits required to unambiguously round up to a 64-bit float. /// /// For an IEEE 754 binary64 float, this required 767 digits. So we store the max digits + 1. @@ -74,11 +74,11 @@ impl Decimal { /// Trim trailing zeros from the buffer. // FIXME(tgross35): this could be `.rev().position()` if perf is okay pub fn trim(&mut self) { - // All of the following calls to `Decimal::trim` can't panic because: + // All of the following calls to `DecimalSeq::trim` can't panic because: // - // 1. `parse_decimal` sets `num_digits` to a max of `Decimal::MAX_DIGITS`. + // 1. `parse_decimal` sets `num_digits` to a max of `DecimalSeq::MAX_DIGITS`. // 2. `right_shift` sets `num_digits` to `write_index`, which is bounded by `num_digits`. - // 3. `left_shift` `num_digits` to a max of `Decimal::MAX_DIGITS`. + // 3. `left_shift` `num_digits` to a max of `DecimalSeq::MAX_DIGITS`. // // Trim is only called in `right_shift` and `left_shift`. debug_assert!(self.num_digits <= Self::MAX_DIGITS); @@ -93,21 +93,26 @@ impl Decimal { } else if self.decimal_point >= Self::MAX_DIGITS_WITHOUT_OVERFLOW as i32 { return 0xFFFF_FFFF_FFFF_FFFF_u64; } + let dp = self.decimal_point as usize; let mut n = 0_u64; + for i in 0..dp { n *= 10; if i < self.num_digits { n += self.digits[i] as u64; } } + let mut round_up = false; + if dp < self.num_digits { round_up = self.digits[dp] >= 5; if self.digits[dp] == 5 && dp + 1 == self.num_digits { round_up = self.truncated || ((dp != 0) && (1 & self.digits[dp - 1] != 0)) } } + if round_up { n += 1; } @@ -123,6 +128,7 @@ impl Decimal { let mut read_index = self.num_digits; let mut write_index = self.num_digits + num_new_digits; let mut n = 0_u64; + while read_index != 0 { read_index -= 1; write_index -= 1; @@ -136,6 +142,7 @@ impl Decimal { } n = quotient; } + while n > 0 { write_index -= 1; let quotient = n / 10; @@ -147,10 +154,13 @@ impl Decimal { } n = quotient; } + self.num_digits += num_new_digits; + if self.num_digits > Self::MAX_DIGITS { self.num_digits = Self::MAX_DIGITS; } + self.decimal_point += num_new_digits as i32; self.trim(); } @@ -206,8 +216,8 @@ impl Decimal { } /// Parse a big integer representation of the float as a decimal. -pub fn parse_decimal(mut s: &[u8]) -> Decimal { - let mut d = Decimal::default(); +pub fn parse_decimal_seq(mut s: &[u8]) -> DecimalSeq { + let mut d = DecimalSeq::default(); let start = s; while let Some((&b'0', s_next)) = s.split_first() { @@ -225,7 +235,7 @@ pub fn parse_decimal(mut s: &[u8]) -> Decimal { s = s_next; } } - while s.len() >= 8 && d.num_digits + 8 < Decimal::MAX_DIGITS { + while s.len() >= 8 && d.num_digits + 8 < DecimalSeq::MAX_DIGITS { let v = s.read_u64(); if !is_8digits(v) { break; @@ -237,6 +247,7 @@ pub fn parse_decimal(mut s: &[u8]) -> Decimal { s = s.parse_digits(|digit| d.try_add_digit(digit)); d.decimal_point = s.len() as i32 - first.len() as i32; } + if d.num_digits != 0 { // Ignore the trailing zeros if there are any let mut n_trailing_zeros = 0; @@ -250,11 +261,12 @@ pub fn parse_decimal(mut s: &[u8]) -> Decimal { d.decimal_point += n_trailing_zeros as i32; d.num_digits -= n_trailing_zeros; d.decimal_point += d.num_digits as i32; - if d.num_digits > Decimal::MAX_DIGITS { + if d.num_digits > DecimalSeq::MAX_DIGITS { d.truncated = true; - d.num_digits = Decimal::MAX_DIGITS; + d.num_digits = DecimalSeq::MAX_DIGITS; } } + if let Some((&ch, s_next)) = s.split_first() { if ch == b'e' || ch == b'E' { s = s_next; @@ -276,13 +288,15 @@ pub fn parse_decimal(mut s: &[u8]) -> Decimal { d.decimal_point += if neg_exp { -exp_num } else { exp_num }; } } - for i in d.num_digits..Decimal::MAX_DIGITS_WITHOUT_OVERFLOW { + + for i in d.num_digits..DecimalSeq::MAX_DIGITS_WITHOUT_OVERFLOW { d.digits[i] = 0; } + d } -fn number_of_digits_decimal_left_shift(d: &Decimal, mut shift: usize) -> usize { +fn number_of_digits_decimal_left_shift(d: &DecimalSeq, mut shift: usize) -> usize { #[rustfmt::skip] const TABLE: [u16; 65] = [ 0x0000, 0x0800, 0x0801, 0x0803, 0x1006, 0x1009, 0x100D, 0x1812, 0x1817, 0x181D, 0x2024, @@ -347,6 +361,7 @@ fn number_of_digits_decimal_left_shift(d: &Decimal, mut shift: usize) -> usize { let pow5_a = (0x7FF & x_a) as usize; let pow5_b = (0x7FF & x_b) as usize; let pow5 = &TABLE_POW5[pow5_a..]; + for (i, &p5) in pow5.iter().enumerate().take(pow5_b - pow5_a) { if i >= d.num_digits { return num_new_digits - 1; @@ -358,5 +373,6 @@ fn number_of_digits_decimal_left_shift(d: &Decimal, mut shift: usize) -> usize { return num_new_digits; } } + num_new_digits } diff --git a/library/core/src/num/dec2flt/mod.rs b/library/core/src/num/dec2flt/mod.rs index 91bfe1bef2e77..471505d249403 100644 --- a/library/core/src/num/dec2flt/mod.rs +++ b/library/core/src/num/dec2flt/mod.rs @@ -97,7 +97,7 @@ use crate::fmt; use crate::str::FromStr; mod common; -mod decimal; +pub mod decimal_seq; mod fpu; mod slow; mod table; diff --git a/library/core/src/num/dec2flt/slow.rs b/library/core/src/num/dec2flt/slow.rs index 85d4b13284b7d..e274e1aa0aa1b 100644 --- a/library/core/src/num/dec2flt/slow.rs +++ b/library/core/src/num/dec2flt/slow.rs @@ -1,7 +1,7 @@ //! Slow, fallback algorithm for cases the Eisel-Lemire algorithm cannot round. use crate::num::dec2flt::common::BiasedFp; -use crate::num::dec2flt::decimal::{Decimal, parse_decimal}; +use crate::num::dec2flt::decimal_seq::{DecimalSeq, parse_decimal_seq}; use crate::num::dec2flt::float::RawFloat; /// Parse the significant digits and biased, binary exponent of a float. @@ -36,7 +36,7 @@ pub(crate) fn parse_long_mantissa(s: &[u8]) -> BiasedFp { let fp_zero = BiasedFp::zero_pow2(0); let fp_inf = BiasedFp::zero_pow2(F::INFINITE_POWER); - let mut d = parse_decimal(s); + let mut d = parse_decimal_seq(s); // Short-circuit if the value can only be a literal 0 or infinity. if d.num_digits == 0 || d.decimal_point < -324 { @@ -50,7 +50,7 @@ pub(crate) fn parse_long_mantissa(s: &[u8]) -> BiasedFp { let n = d.decimal_point as usize; let shift = get_shift(n); d.right_shift(shift); - if d.decimal_point < -Decimal::DECIMAL_POINT_RANGE { + if d.decimal_point < -DecimalSeq::DECIMAL_POINT_RANGE { return fp_zero; } exp2 += shift as i32; @@ -67,7 +67,7 @@ pub(crate) fn parse_long_mantissa(s: &[u8]) -> BiasedFp { get_shift((-d.decimal_point) as _) }; d.left_shift(shift); - if d.decimal_point > Decimal::DECIMAL_POINT_RANGE { + if d.decimal_point > DecimalSeq::DECIMAL_POINT_RANGE { return fp_inf; } exp2 -= shift as i32; diff --git a/library/core/tests/num/dec2flt/decimal_seq.rs b/library/core/tests/num/dec2flt/decimal_seq.rs new file mode 100644 index 0000000000000..f46ba7c465a6e --- /dev/null +++ b/library/core/tests/num/dec2flt/decimal_seq.rs @@ -0,0 +1,30 @@ +use core::num::dec2flt::decimal_seq::{DecimalSeq, parse_decimal_seq}; + +#[test] +fn test_trim() { + let mut dec = DecimalSeq::default(); + let digits = [1, 2, 3, 4]; + + dec.digits[0..4].copy_from_slice(&digits); + dec.num_digits = 8; + dec.trim(); + + assert_eq!(dec.digits[0..4], digits); + assert_eq!(dec.num_digits, 4); +} + +#[test] +fn test_parse() { + let tests = [("1.234", [1, 2, 3, 4], 1)]; + + for (s, exp_digits, decimal_point) in tests { + let actual = parse_decimal_seq(s.as_bytes()); + let mut digits = [0; DecimalSeq::MAX_DIGITS]; + digits[..exp_digits.len()].copy_from_slice(&exp_digits); + + let expected = + DecimalSeq { num_digits: exp_digits.len(), decimal_point, truncated: false, digits }; + + assert_eq!(actual, expected); + } +} diff --git a/library/core/tests/num/dec2flt/mod.rs b/library/core/tests/num/dec2flt/mod.rs index 874e0ec7093c7..51f3017ddc79c 100644 --- a/library/core/tests/num/dec2flt/mod.rs +++ b/library/core/tests/num/dec2flt/mod.rs @@ -1,5 +1,6 @@ #![allow(overflowing_literals)] +mod decimal_seq; mod float; mod lemire; mod parse; From 2bbf55025b4fc7e920d1ca068bb795ec997c70dc Mon Sep 17 00:00:00 2001 From: Trevor Gross Date: Mon, 9 Dec 2024 09:25:22 +0000 Subject: [PATCH 05/16] dec2flt: Rename `Number` to `Decimal` The previous commit renamed `Decimal` to `DecimalSeq`. Now, rename the type that represents a decimal floating point number to be `Decimal`. Additionally, add some tests for internal behavior. --- .../src/num/dec2flt/{number.rs => decimal.rs} | 6 ++-- library/core/src/num/dec2flt/mod.rs | 2 +- library/core/src/num/dec2flt/parse.rs | 10 +++---- library/core/tests/num/dec2flt/decimal.rs | 28 +++++++++++++++++++ library/core/tests/num/dec2flt/mod.rs | 1 + library/core/tests/num/dec2flt/parse.rs | 26 ++++++++--------- 6 files changed, 51 insertions(+), 22 deletions(-) rename library/core/src/num/dec2flt/{number.rs => decimal.rs} (96%) create mode 100644 library/core/tests/num/dec2flt/decimal.rs diff --git a/library/core/src/num/dec2flt/number.rs b/library/core/src/num/dec2flt/decimal.rs similarity index 96% rename from library/core/src/num/dec2flt/number.rs rename to library/core/src/num/dec2flt/decimal.rs index 2538991564ae4..1ba9150a0119d 100644 --- a/library/core/src/num/dec2flt/number.rs +++ b/library/core/src/num/dec2flt/decimal.rs @@ -3,7 +3,6 @@ use crate::num::dec2flt::float::RawFloat; use crate::num::dec2flt::fpu::set_precision; -#[rustfmt::skip] const INT_POW10: [u64; 16] = [ 1, 10, @@ -23,15 +22,16 @@ const INT_POW10: [u64; 16] = [ 1000000000000000, ]; +/// A floating point number with up to 64 bits of mantissa and an `i64` exponent. #[derive(Clone, Copy, Debug, Default, PartialEq, Eq)] -pub struct Number { +pub struct Decimal { pub exponent: i64, pub mantissa: u64, pub negative: bool, pub many_digits: bool, } -impl Number { +impl Decimal { /// Detect if the float can be accurately reconstructed from native floats. #[inline] fn is_fast_path(&self) -> bool { diff --git a/library/core/src/num/dec2flt/mod.rs b/library/core/src/num/dec2flt/mod.rs index 471505d249403..a82c858df4998 100644 --- a/library/core/src/num/dec2flt/mod.rs +++ b/library/core/src/num/dec2flt/mod.rs @@ -97,6 +97,7 @@ use crate::fmt; use crate::str::FromStr; mod common; +pub mod decimal; pub mod decimal_seq; mod fpu; mod slow; @@ -104,7 +105,6 @@ mod table; // float is used in flt2dec, and all are used in unit tests. pub mod float; pub mod lemire; -pub mod number; pub mod parse; macro_rules! from_str_float_impl { diff --git a/library/core/src/num/dec2flt/parse.rs b/library/core/src/num/dec2flt/parse.rs index 06ee8e95fbc47..e38fedc58bec0 100644 --- a/library/core/src/num/dec2flt/parse.rs +++ b/library/core/src/num/dec2flt/parse.rs @@ -1,8 +1,8 @@ //! Functions to parse floating-point numbers. use crate::num::dec2flt::common::{ByteSlice, is_8digits}; +use crate::num::dec2flt::decimal::Decimal; use crate::num::dec2flt::float::RawFloat; -use crate::num::dec2flt::number::Number; const MIN_19DIGIT_INT: u64 = 100_0000_0000_0000_0000; @@ -100,7 +100,7 @@ fn parse_scientific(s_ref: &mut &[u8]) -> Option { /// /// This creates a representation of the float as the /// significant digits and the decimal exponent. -fn parse_partial_number(mut s: &[u8]) -> Option<(Number, usize)> { +fn parse_partial_number(mut s: &[u8]) -> Option<(Decimal, usize)> { debug_assert!(!s.is_empty()); // parse initial digits before dot @@ -146,7 +146,7 @@ fn parse_partial_number(mut s: &[u8]) -> Option<(Number, usize)> { // handle uncommon case with many digits if n_digits <= 19 { - return Some((Number { exponent, mantissa, negative: false, many_digits: false }, len)); + return Some((Decimal { exponent, mantissa, negative: false, many_digits: false }, len)); } n_digits -= 19; @@ -179,13 +179,13 @@ fn parse_partial_number(mut s: &[u8]) -> Option<(Number, usize)> { exponent += exp_number; } - Some((Number { exponent, mantissa, negative: false, many_digits }, len)) + Some((Decimal { exponent, mantissa, negative: false, many_digits }, len)) } /// Try to parse a non-special floating point number, /// as well as two slices with integer and fractional parts /// and the parsed exponent. -pub fn parse_number(s: &[u8]) -> Option { +pub fn parse_number(s: &[u8]) -> Option { if let Some((float, rest)) = parse_partial_number(s) { if rest == s.len() { return Some(float); diff --git a/library/core/tests/num/dec2flt/decimal.rs b/library/core/tests/num/dec2flt/decimal.rs new file mode 100644 index 0000000000000..1fa06de692e07 --- /dev/null +++ b/library/core/tests/num/dec2flt/decimal.rs @@ -0,0 +1,28 @@ +use core::num::dec2flt::decimal::Decimal; + +type FPath = ((i64, u64, bool, bool), Option); + +const FPATHS_F32: &[FPath] = + &[((0, 0, false, false), Some(0.0)), ((0, 0, false, false), Some(0.0))]; +const FPATHS_F64: &[FPath] = + &[((0, 0, false, false), Some(0.0)), ((0, 0, false, false), Some(0.0))]; + +#[test] +fn check_fast_path_f32() { + for ((exponent, mantissa, negative, many_digits), expected) in FPATHS_F32.iter().copied() { + let dec = Decimal { exponent, mantissa, negative, many_digits }; + let actual = dec.try_fast_path::(); + + assert_eq!(actual, expected); + } +} + +#[test] +fn check_fast_path_f64() { + for ((exponent, mantissa, negative, many_digits), expected) in FPATHS_F64.iter().copied() { + let dec = Decimal { exponent, mantissa, negative, many_digits }; + let actual = dec.try_fast_path::(); + + assert_eq!(actual, expected); + } +} diff --git a/library/core/tests/num/dec2flt/mod.rs b/library/core/tests/num/dec2flt/mod.rs index 51f3017ddc79c..a9025be5ca7f1 100644 --- a/library/core/tests/num/dec2flt/mod.rs +++ b/library/core/tests/num/dec2flt/mod.rs @@ -1,5 +1,6 @@ #![allow(overflowing_literals)] +mod decimal; mod decimal_seq; mod float; mod lemire; diff --git a/library/core/tests/num/dec2flt/parse.rs b/library/core/tests/num/dec2flt/parse.rs index 4a5d24ba7d5fa..ec705a61e13ca 100644 --- a/library/core/tests/num/dec2flt/parse.rs +++ b/library/core/tests/num/dec2flt/parse.rs @@ -1,9 +1,9 @@ -use core::num::dec2flt::number::Number; +use core::num::dec2flt::decimal::Decimal; use core::num::dec2flt::parse::parse_number; use core::num::dec2flt::{dec2flt, pfe_invalid}; -fn new_number(e: i64, m: u64) -> Number { - Number { exponent: e, mantissa: m, negative: false, many_digits: false } +fn new_dec(e: i64, m: u64) -> Decimal { + Decimal { exponent: e, mantissa: m, negative: false, many_digits: false } } #[test] @@ -31,23 +31,23 @@ fn invalid_chars() { } } -fn parse_positive(s: &[u8]) -> Option { +fn parse_positive(s: &[u8]) -> Option { parse_number(s) } #[test] fn valid() { - assert_eq!(parse_positive(b"123.456e789"), Some(new_number(786, 123456))); - assert_eq!(parse_positive(b"123.456e+789"), Some(new_number(786, 123456))); - assert_eq!(parse_positive(b"123.456e-789"), Some(new_number(-792, 123456))); - assert_eq!(parse_positive(b".050"), Some(new_number(-3, 50))); - assert_eq!(parse_positive(b"999"), Some(new_number(0, 999))); - assert_eq!(parse_positive(b"1.e300"), Some(new_number(300, 1))); - assert_eq!(parse_positive(b".1e300"), Some(new_number(299, 1))); - assert_eq!(parse_positive(b"101e-33"), Some(new_number(-33, 101))); + assert_eq!(parse_positive(b"123.456e789"), Some(new_dec(786, 123456))); + assert_eq!(parse_positive(b"123.456e+789"), Some(new_dec(786, 123456))); + assert_eq!(parse_positive(b"123.456e-789"), Some(new_dec(-792, 123456))); + assert_eq!(parse_positive(b".050"), Some(new_dec(-3, 50))); + assert_eq!(parse_positive(b"999"), Some(new_dec(0, 999))); + assert_eq!(parse_positive(b"1.e300"), Some(new_dec(300, 1))); + assert_eq!(parse_positive(b".1e300"), Some(new_dec(299, 1))); + assert_eq!(parse_positive(b"101e-33"), Some(new_dec(-33, 101))); let zeros = "0".repeat(25); let s = format!("1.5e{zeros}"); - assert_eq!(parse_positive(s.as_bytes()), Some(new_number(-1, 15))); + assert_eq!(parse_positive(s.as_bytes()), Some(new_dec(-1, 15))); } macro_rules! assert_float_result_bits_eq { From f0f8c8924444c884a8db5b4c415cd0362ab12157 Mon Sep 17 00:00:00 2001 From: Trevor Gross Date: Mon, 9 Dec 2024 09:25:22 +0000 Subject: [PATCH 06/16] dec2flt: Rename fields to be consistent with documented notation --- library/core/src/num/dec2flt/common.rs | 13 +++++++------ library/core/src/num/dec2flt/lemire.rs | 4 ++-- library/core/src/num/dec2flt/mod.rs | 13 ++++++++----- library/core/src/num/dec2flt/slow.rs | 2 +- library/core/tests/num/dec2flt/float.rs | 8 ++++---- library/core/tests/num/dec2flt/lemire.rs | 4 ++-- 6 files changed, 24 insertions(+), 20 deletions(-) diff --git a/library/core/src/num/dec2flt/common.rs b/library/core/src/num/dec2flt/common.rs index 1646d3a95d3b1..a140a311c452f 100644 --- a/library/core/src/num/dec2flt/common.rs +++ b/library/core/src/num/dec2flt/common.rs @@ -63,19 +63,20 @@ pub(crate) fn is_8digits(v: u64) -> bool { (a | b) & 0x8080_8080_8080_8080 == 0 } -/// A custom 64-bit floating point type, representing `f * 2^e`. -/// e is biased, so it be directly shifted into the exponent bits. +/// A custom 64-bit floating point type, representing `m * 2^p`. +/// p is biased, so it be directly shifted into the exponent bits. #[derive(Debug, Copy, Clone, PartialEq, Eq, Default)] pub struct BiasedFp { /// The significant digits. - pub f: u64, + pub m: u64, /// The biased, binary exponent. - pub e: i32, + pub p_biased: i32, } impl BiasedFp { + /// Represent `0 ^ p` #[inline] - pub const fn zero_pow2(e: i32) -> Self { - Self { f: 0, e } + pub const fn zero_pow2(p_biased: i32) -> Self { + Self { m: 0, p_biased } } } diff --git a/library/core/src/num/dec2flt/lemire.rs b/library/core/src/num/dec2flt/lemire.rs index 01642e1b1112a..0cc39cb9b6231 100644 --- a/library/core/src/num/dec2flt/lemire.rs +++ b/library/core/src/num/dec2flt/lemire.rs @@ -73,7 +73,7 @@ pub fn compute_float(q: i64, mut w: u64) -> BiasedFp { mantissa += mantissa & 1; mantissa >>= 1; power2 = (mantissa >= (1_u64 << F::MANTISSA_EXPLICIT_BITS)) as i32; - return BiasedFp { f: mantissa, e: power2 }; + return BiasedFp { m: mantissa, p_biased: power2 }; } // Need to handle rounding ties. Normally, we need to round up, // but if we fall right in between and we have an even basis, we @@ -111,7 +111,7 @@ pub fn compute_float(q: i64, mut w: u64) -> BiasedFp { // Exponent is above largest normal value, must be infinite. return fp_inf; } - BiasedFp { f: mantissa, e: power2 } + BiasedFp { m: mantissa, p_biased: power2 } } /// Calculate a base 2 exponent from a decimal exponent. diff --git a/library/core/src/num/dec2flt/mod.rs b/library/core/src/num/dec2flt/mod.rs index a82c858df4998..9d1989c4b817d 100644 --- a/library/core/src/num/dec2flt/mod.rs +++ b/library/core/src/num/dec2flt/mod.rs @@ -233,8 +233,8 @@ pub fn pfe_invalid() -> ParseFloatError { /// Converts a `BiasedFp` to the closest machine float type. fn biased_fp_to_float(x: BiasedFp) -> T { - let mut word = x.f; - word |= (x.e as u64) << T::MANTISSA_EXPLICIT_BITS; + let mut word = x.m; + word |= (x.p_biased as u64) << T::MANTISSA_EXPLICIT_BITS; T::from_u64_bits(word) } @@ -272,12 +272,15 @@ pub fn dec2flt(s: &str) -> Result { // redundantly using the Eisel-Lemire algorithm if it was unable to // correctly round on the first pass. let mut fp = compute_float::(num.exponent, num.mantissa); - if num.many_digits && fp.e >= 0 && fp != compute_float::(num.exponent, num.mantissa + 1) { - fp.e = -1; + if num.many_digits + && fp.p_biased >= 0 + && fp != compute_float::(num.exponent, num.mantissa + 1) + { + fp.p_biased = -1; } // Unable to correctly round the float using the Eisel-Lemire algorithm. // Fallback to a slower, but always correct algorithm. - if fp.e < 0 { + if fp.p_biased < 0 { fp = parse_long_mantissa::(s); } diff --git a/library/core/src/num/dec2flt/slow.rs b/library/core/src/num/dec2flt/slow.rs index e274e1aa0aa1b..e0c4ae117da02 100644 --- a/library/core/src/num/dec2flt/slow.rs +++ b/library/core/src/num/dec2flt/slow.rs @@ -105,5 +105,5 @@ pub(crate) fn parse_long_mantissa(s: &[u8]) -> BiasedFp { } // Zero out all the bits above the explicit mantissa bits. mantissa &= (1_u64 << F::MANTISSA_EXPLICIT_BITS) - 1; - BiasedFp { f: mantissa, e: power2 } + BiasedFp { m: mantissa, p_biased: power2 } } diff --git a/library/core/tests/num/dec2flt/float.rs b/library/core/tests/num/dec2flt/float.rs index 7a9587a18d030..40f0776e02148 100644 --- a/library/core/tests/num/dec2flt/float.rs +++ b/library/core/tests/num/dec2flt/float.rs @@ -12,8 +12,8 @@ fn test_f32_integer_decode() { // Ignore the "sign" (quiet / signalling flag) of NAN. // It can vary between runtime operations and LLVM folding. - let (nan_m, nan_e, _nan_s) = f32::NAN.integer_decode(); - assert_eq!((nan_m, nan_e), (12582912, 105)); + let (nan_m, nan_p, _nan_s) = f32::NAN.integer_decode(); + assert_eq!((nan_m, nan_p), (12582912, 105)); } #[test] @@ -28,6 +28,6 @@ fn test_f64_integer_decode() { // Ignore the "sign" (quiet / signalling flag) of NAN. // It can vary between runtime operations and LLVM folding. - let (nan_m, nan_e, _nan_s) = f64::NAN.integer_decode(); - assert_eq!((nan_m, nan_e), (6755399441055744, 972)); + let (nan_m, nan_p, _nan_s) = f64::NAN.integer_decode(); + assert_eq!((nan_m, nan_p), (6755399441055744, 972)); } diff --git a/library/core/tests/num/dec2flt/lemire.rs b/library/core/tests/num/dec2flt/lemire.rs index f71bbb7c7a318..9f228a25e46b1 100644 --- a/library/core/tests/num/dec2flt/lemire.rs +++ b/library/core/tests/num/dec2flt/lemire.rs @@ -2,12 +2,12 @@ use core::num::dec2flt::lemire::compute_float; fn compute_float32(q: i64, w: u64) -> (i32, u64) { let fp = compute_float::(q, w); - (fp.e, fp.f) + (fp.p_biased, fp.m) } fn compute_float64(q: i64, w: u64) -> (i32, u64) { let fp = compute_float::(q, w); - (fp.e, fp.f) + (fp.p_biased, fp.m) } #[test] From 6dce46c4a24faa16901b1c05dd716c4fb70351ba Mon Sep 17 00:00:00 2001 From: Trevor Gross Date: Mon, 9 Dec 2024 09:25:22 +0000 Subject: [PATCH 07/16] dec2flt: Refactor float traits A lot of the magic constants can be turned into expressions. This reduces some code duplication. Additionally, add traits to make these operations fully generic. This will make it easier to support `f16` and `f128`. --- library/core/src/num/dec2flt/float.rs | 251 ++++++++++++++++--------- library/core/src/num/dec2flt/lemire.rs | 4 +- library/core/src/num/dec2flt/slow.rs | 2 +- src/etc/test-float-parse/src/traits.rs | 6 +- 4 files changed, 167 insertions(+), 96 deletions(-) diff --git a/library/core/src/num/dec2flt/float.rs b/library/core/src/num/dec2flt/float.rs index da57aa9a546af..0e07ecf0c8d78 100644 --- a/library/core/src/num/dec2flt/float.rs +++ b/library/core/src/num/dec2flt/float.rs @@ -1,14 +1,57 @@ //! Helper trait for generic float types. +use core::f64; + use crate::fmt::{Debug, LowerExp}; use crate::num::FpCategory; -use crate::ops::{Add, Div, Mul, Neg}; +use crate::ops::{self, Add, Div, Mul, Neg}; + +/// Lossy `as` casting between two types. +pub trait CastInto: Copy { + fn cast(self) -> T; +} + +/// Collection of traits that allow us to be generic over integer size. +pub trait Integer: + Sized + + Clone + + Copy + + Debug + + ops::Shr + + ops::Shl + + ops::BitAnd + + ops::BitOr + + PartialEq + + CastInto +{ + const ZERO: Self; + const ONE: Self; +} -/// A helper trait to avoid duplicating basically all the conversion code for `f32` and `f64`. +macro_rules! int { + ($($ty:ty),+) => { + $( + impl CastInto for $ty { + fn cast(self) -> i16 { + self as i16 + } + } + + impl Integer for $ty { + const ZERO: Self = 0; + const ONE: Self = 1; + } + )+ + } +} + +int!(u32, u64); + +/// A helper trait to avoid duplicating basically all the conversion code for IEEE floats. /// /// See the parent module's doc comment for why this is necessary. /// -/// Should **never ever** be implemented for other types or be used outside the dec2flt module. +/// Should **never ever** be implemented for other types or be used outside the `dec2flt` module. #[doc(hidden)] pub trait RawFloat: Sized @@ -24,62 +67,93 @@ pub trait RawFloat: + Copy + Debug { + /// The unsigned integer with the same size as the float + type Int: Integer + Into; + + /* general constants */ + const INFINITY: Self; const NEG_INFINITY: Self; const NAN: Self; const NEG_NAN: Self; + /// Bit width of the float + const BITS: u32; + + /// Mantissa digits including the hidden bit (provided by core) + const MANTISSA_BITS: u32; + + const EXPONENT_MASK: Self::Int; + const MANTISSA_MASK: Self::Int; + /// The number of bits in the significand, *excluding* the hidden bit. - const MANTISSA_EXPLICIT_BITS: usize; - - // Round-to-even only happens for negative values of q - // when q ≥ −4 in the 64-bit case and when q ≥ −17 in - // the 32-bitcase. - // - // When q ≥ 0,we have that 5^q ≤ 2m+1. In the 64-bit case,we - // have 5^q ≤ 2m+1 ≤ 2^54 or q ≤ 23. In the 32-bit case,we have - // 5^q ≤ 2m+1 ≤ 2^25 or q ≤ 10. - // - // When q < 0, we have w ≥ (2m+1)×5^−q. We must have that w < 2^64 - // so (2m+1)×5^−q < 2^64. We have that 2m+1 > 2^53 (64-bit case) - // or 2m+1 > 2^24 (32-bit case). Hence,we must have 2^53×5^−q < 2^64 - // (64-bit) and 2^24×5^−q < 2^64 (32-bit). Hence we have 5^−q < 2^11 - // or q ≥ −4 (64-bit case) and 5^−q < 2^40 or q ≥ −17 (32-bitcase). - // - // Thus we have that we only need to round ties to even when - // we have that q ∈ [−4,23](in the 64-bit case) or q∈[−17,10] - // (in the 32-bit case). In both cases,the power of five(5^|q|) - // fits in a 64-bit word. + const MANTISSA_EXPLICIT_BITS: u32 = Self::MANTISSA_BITS - 1; + + /// Bits for the exponent + const EXPONENT_BITS: u32 = Self::BITS - Self::MANTISSA_EXPLICIT_BITS - 1; + + /// Minimum exponent value `-(1 << (EXP_BITS - 1)) + 1`. + const MINIMUM_EXPONENT: i32 = -(1 << (Self::EXPONENT_BITS - 1)) + 1; + + /// Maximum exponent without overflowing to infinity + const MAXIMUM_EXPONENT: u32 = (1 << Self::EXPONENT_BITS) - 1; + + /// The exponent bias value + const EXPONENT_BIAS: u32 = Self::MAXIMUM_EXPONENT >> 1; + + /// Largest exponent value `(1 << EXP_BITS) - 1`. + const INFINITE_POWER: i32 = (1 << Self::EXPONENT_BITS) - 1; + + /// Round-to-even only happens for negative values of q + /// when q ≥ −4 in the 64-bit case and when q ≥ −17 in + /// the 32-bitcase. + /// + /// When q ≥ 0,we have that 5^q ≤ 2m+1. In the 64-bit case,we + /// have 5^q ≤ 2m+1 ≤ 2^54 or q ≤ 23. In the 32-bit case,we have + /// 5^q ≤ 2m+1 ≤ 2^25 or q ≤ 10. + /// + /// When q < 0, we have w ≥ (2m+1)×5^−q. We must have that w < 2^64 + /// so (2m+1)×5^−q < 2^64. We have that 2m+1 > 2^53 (64-bit case) + /// or 2m+1 > 2^24 (32-bit case). Hence,we must have 2^53×5^−q < 2^64 + /// (64-bit) and 2^24×5^−q < 2^64 (32-bit). Hence we have 5^−q < 2^11 + /// or q ≥ −4 (64-bit case) and 5^−q < 2^40 or q ≥ −17 (32-bitcase). + /// + /// Thus we have that we only need to round ties to even when + /// we have that q ∈ [−4,23](in the 64-bit case) or q∈[−17,10] + /// (in the 32-bit case). In both cases,the power of five(5^|q|) + /// fits in a 64-bit word. const MIN_EXPONENT_ROUND_TO_EVEN: i32; const MAX_EXPONENT_ROUND_TO_EVEN: i32; - // Minimum exponent that for a fast path case, or `-⌊(MANTISSA_EXPLICIT_BITS+1)/log2(5)⌋` - const MIN_EXPONENT_FAST_PATH: i64; - - // Maximum exponent that for a fast path case, or `⌊(MANTISSA_EXPLICIT_BITS+1)/log2(5)⌋` - const MAX_EXPONENT_FAST_PATH: i64; + /* limits related to Fast pathing */ - // Maximum exponent that can be represented for a disguised-fast path case. - // This is `MAX_EXPONENT_FAST_PATH + ⌊(MANTISSA_EXPLICIT_BITS+1)/log2(10)⌋` - const MAX_EXPONENT_DISGUISED_FAST_PATH: i64; + /// Largest decimal exponent for a non-infinite value. + /// + /// This is the max exponent in binary converted to the max exponent in decimal. Allows fast + /// pathing anything larger than `10^LARGEST_POWER_OF_TEN`, which will round to infinity. + const LARGEST_POWER_OF_TEN: i32 = + ((Self::EXPONENT_BIAS as f64 + 1.0) / f64::consts::LOG2_10) as i32; - // Minimum exponent value `-(1 << (EXP_BITS - 1)) + 1`. - const MINIMUM_EXPONENT: i32; + /// Smallest decimal exponent for a non-zero value. This allows for fast pathing anything + /// smaller than `10^SMALLEST_POWER_OF_TEN`, which will round to zero. + const SMALLEST_POWER_OF_TEN: i32 = + -(((Self::EXPONENT_BIAS + Self::MANTISSA_BITS + 64) as f64) / f64::consts::LOG2_10) as i32; - // Largest exponent value `(1 << EXP_BITS) - 1`. - const INFINITE_POWER: i32; + /// Maximum exponent for a fast path case, or `⌊(MANTISSA_EXPLICIT_BITS+1)/log2(5)⌋` + // assuming FLT_EVAL_METHOD = 0 + const MAX_EXPONENT_FAST_PATH: i64 = + ((Self::MANTISSA_BITS as f64) / (f64::consts::LOG2_10 - 1.0)) as i64; - // Index (in bits) of the sign. - const SIGN_INDEX: usize; + /// Minimum exponent for a fast path case, or `-⌊(MANTISSA_EXPLICIT_BITS+1)/log2(5)⌋` + const MIN_EXPONENT_FAST_PATH: i64 = -Self::MAX_EXPONENT_FAST_PATH; - // Smallest decimal exponent for a non-zero value. - const SMALLEST_POWER_OF_TEN: i32; + /// Maximum exponent that can be represented for a disguised-fast path case. + /// This is `MAX_EXPONENT_FAST_PATH + ⌊(MANTISSA_EXPLICIT_BITS+1)/log2(10)⌋` + const MAX_EXPONENT_DISGUISED_FAST_PATH: i64 = + Self::MAX_EXPONENT_FAST_PATH + (Self::MANTISSA_BITS as f64 / f64::consts::LOG2_10) as i64; - // Largest decimal exponent for a non-infinite value. - const LARGEST_POWER_OF_TEN: i32; - - // Maximum mantissa for the fast-path (`1 << 53` for f64). - const MAX_MANTISSA_FAST_PATH: u64 = 2_u64 << Self::MANTISSA_EXPLICIT_BITS; + /// Maximum mantissa for the fast-path (`1 << 53` for f64). + const MAX_MANTISSA_FAST_PATH: u64 = 1 << Self::MANTISSA_BITS; /// Converts integer into float through an as cast. /// This is only called in the fast-path algorithm, and therefore @@ -96,27 +170,45 @@ pub trait RawFloat: /// Returns the category that this number falls into. fn classify(self) -> FpCategory; + /// Transmute to the integer representation + fn to_bits(self) -> Self::Int; + /// Returns the mantissa, exponent and sign as integers. - fn integer_decode(self) -> (u64, i16, i8); + /// + /// That is, this returns `(m, p, s)` such that `s * m * 2^p` represents the original float. + /// For 0, the exponent will be `-(EXPONENT_BIAS + MANTISSA_EXPLICIT_BITS`, which is the + /// minimum subnormal power. + fn integer_decode(self) -> (u64, i16, i8) { + let bits = self.to_bits(); + let sign: i8 = if bits >> (Self::BITS - 1) == Self::Int::ZERO { 1 } else { -1 }; + let mut exponent: i16 = + ((bits & Self::EXPONENT_MASK) >> Self::MANTISSA_EXPLICIT_BITS).cast(); + let mantissa = if exponent == 0 { + (bits & Self::MANTISSA_MASK) << 1 + } else { + (bits & Self::MANTISSA_MASK) | (Self::Int::ONE << Self::MANTISSA_EXPLICIT_BITS) + }; + // Exponent bias + mantissa shift + exponent -= (Self::EXPONENT_BIAS + Self::MANTISSA_EXPLICIT_BITS) as i16; + (mantissa.into(), exponent, sign) + } } impl RawFloat for f32 { + type Int = u32; + const INFINITY: Self = f32::INFINITY; const NEG_INFINITY: Self = f32::NEG_INFINITY; const NAN: Self = f32::NAN; const NEG_NAN: Self = -f32::NAN; - const MANTISSA_EXPLICIT_BITS: usize = 23; + const BITS: u32 = 32; + const MANTISSA_BITS: u32 = Self::MANTISSA_DIGITS; + const EXPONENT_MASK: Self::Int = Self::EXP_MASK; + const MANTISSA_MASK: Self::Int = Self::MAN_MASK; + const MIN_EXPONENT_ROUND_TO_EVEN: i32 = -17; const MAX_EXPONENT_ROUND_TO_EVEN: i32 = 10; - const MIN_EXPONENT_FAST_PATH: i64 = -10; // assuming FLT_EVAL_METHOD = 0 - const MAX_EXPONENT_FAST_PATH: i64 = 10; - const MAX_EXPONENT_DISGUISED_FAST_PATH: i64 = 17; - const MINIMUM_EXPONENT: i32 = -127; - const INFINITE_POWER: i32 = 0xFF; - const SIGN_INDEX: usize = 31; - const SMALLEST_POWER_OF_TEN: i32 = -65; - const LARGEST_POWER_OF_TEN: i32 = 38; #[inline] fn from_u64(v: u64) -> Self { @@ -136,16 +228,8 @@ impl RawFloat for f32 { TABLE[exponent & 15] } - /// Returns the mantissa, exponent and sign as integers. - fn integer_decode(self) -> (u64, i16, i8) { - let bits = self.to_bits(); - let sign: i8 = if bits >> 31 == 0 { 1 } else { -1 }; - let mut exponent: i16 = ((bits >> 23) & 0xff) as i16; - let mantissa = - if exponent == 0 { (bits & 0x7fffff) << 1 } else { (bits & 0x7fffff) | 0x800000 }; - // Exponent bias + mantissa shift - exponent -= 127 + 23; - (mantissa as u64, exponent, sign) + fn to_bits(self) -> Self::Int { + self.to_bits() } fn classify(self) -> FpCategory { @@ -154,22 +238,20 @@ impl RawFloat for f32 { } impl RawFloat for f64 { - const INFINITY: Self = f64::INFINITY; - const NEG_INFINITY: Self = f64::NEG_INFINITY; - const NAN: Self = f64::NAN; - const NEG_NAN: Self = -f64::NAN; + type Int = u64; + + const INFINITY: Self = Self::INFINITY; + const NEG_INFINITY: Self = Self::NEG_INFINITY; + const NAN: Self = Self::NAN; + const NEG_NAN: Self = -Self::NAN; + + const BITS: u32 = 64; + const MANTISSA_BITS: u32 = Self::MANTISSA_DIGITS; + const EXPONENT_MASK: Self::Int = Self::EXP_MASK; + const MANTISSA_MASK: Self::Int = Self::MAN_MASK; - const MANTISSA_EXPLICIT_BITS: usize = 52; const MIN_EXPONENT_ROUND_TO_EVEN: i32 = -4; const MAX_EXPONENT_ROUND_TO_EVEN: i32 = 23; - const MIN_EXPONENT_FAST_PATH: i64 = -22; // assuming FLT_EVAL_METHOD = 0 - const MAX_EXPONENT_FAST_PATH: i64 = 22; - const MAX_EXPONENT_DISGUISED_FAST_PATH: i64 = 37; - const MINIMUM_EXPONENT: i32 = -1023; - const INFINITE_POWER: i32 = 0x7FF; - const SIGN_INDEX: usize = 63; - const SMALLEST_POWER_OF_TEN: i32 = -342; - const LARGEST_POWER_OF_TEN: i32 = 308; #[inline] fn from_u64(v: u64) -> Self { @@ -190,19 +272,8 @@ impl RawFloat for f64 { TABLE[exponent & 31] } - /// Returns the mantissa, exponent and sign as integers. - fn integer_decode(self) -> (u64, i16, i8) { - let bits = self.to_bits(); - let sign: i8 = if bits >> 63 == 0 { 1 } else { -1 }; - let mut exponent: i16 = ((bits >> 52) & 0x7ff) as i16; - let mantissa = if exponent == 0 { - (bits & 0xfffffffffffff) << 1 - } else { - (bits & 0xfffffffffffff) | 0x10000000000000 - }; - // Exponent bias + mantissa shift - exponent -= 1023 + 52; - (mantissa, exponent, sign) + fn to_bits(self) -> Self::Int { + self.to_bits() } fn classify(self) -> FpCategory { diff --git a/library/core/src/num/dec2flt/lemire.rs b/library/core/src/num/dec2flt/lemire.rs index 0cc39cb9b6231..badb2ad3471f2 100644 --- a/library/core/src/num/dec2flt/lemire.rs +++ b/library/core/src/num/dec2flt/lemire.rs @@ -38,7 +38,7 @@ pub fn compute_float(q: i64, mut w: u64) -> BiasedFp { // Normalize our significant digits, so the most-significant bit is set. let lz = w.leading_zeros(); w <<= lz; - let (lo, hi) = compute_product_approx(q, w, F::MANTISSA_EXPLICIT_BITS + 3); + let (lo, hi) = compute_product_approx(q, w, F::MANTISSA_EXPLICIT_BITS as usize + 3); if lo == 0xFFFF_FFFF_FFFF_FFFF { // If we have failed to approximate w x 5^-q with our 128-bit value. // Since the addition of 1 could lead to an overflow which could then @@ -89,7 +89,7 @@ pub fn compute_float(q: i64, mut w: u64) -> BiasedFp { if lo <= 1 && q >= F::MIN_EXPONENT_ROUND_TO_EVEN as i64 && q <= F::MAX_EXPONENT_ROUND_TO_EVEN as i64 - && mantissa & 3 == 1 + && mantissa & 0b11 == 0b01 && (mantissa << (upperbit + 64 - F::MANTISSA_EXPLICIT_BITS as i32 - 3)) == hi { // Zero the lowest bit, so we don't round up. diff --git a/library/core/src/num/dec2flt/slow.rs b/library/core/src/num/dec2flt/slow.rs index e0c4ae117da02..ed7263c3f99d6 100644 --- a/library/core/src/num/dec2flt/slow.rs +++ b/library/core/src/num/dec2flt/slow.rs @@ -87,7 +87,7 @@ pub(crate) fn parse_long_mantissa(s: &[u8]) -> BiasedFp { } // Shift the decimal to the hidden bit, and then round the value // to get the high mantissa+1 bits. - d.left_shift(F::MANTISSA_EXPLICIT_BITS + 1); + d.left_shift(F::MANTISSA_EXPLICIT_BITS as usize + 1); let mut mantissa = d.round(); if mantissa >= (1_u64 << (F::MANTISSA_EXPLICIT_BITS + 1)) { // Rounding up overflowed to the carry bit, need to diff --git a/src/etc/test-float-parse/src/traits.rs b/src/etc/test-float-parse/src/traits.rs index f5333d63b3693..f7f5f5dd35b70 100644 --- a/src/etc/test-float-parse/src/traits.rs +++ b/src/etc/test-float-parse/src/traits.rs @@ -147,12 +147,12 @@ pub trait Float: } macro_rules! impl_float { - ($($fty:ty, $ity:ty, $bits:literal);+) => { + ($($fty:ty, $ity:ty);+) => { $( impl Float for $fty { type Int = $ity; type SInt = ::Signed; - const BITS: u32 = $bits; + const BITS: u32 = <$ity>::BITS; const MAN_BITS: u32 = Self::MANTISSA_DIGITS - 1; const MAN_MASK: Self::Int = (Self::Int::ONE << Self::MAN_BITS) - Self::Int::ONE; const SIGN_MASK: Self::Int = Self::Int::ONE << (Self::BITS-1); @@ -168,7 +168,7 @@ macro_rules! impl_float { } } -impl_float!(f32, u32, 32; f64, u64, 64); +impl_float!(f32, u32; f64, u64); /// A test generator. Should provide an iterator that produces unique patterns to parse. /// From 89f2066203acdcabf690c9d2397c94c9feed3cb9 Mon Sep 17 00:00:00 2001 From: Trevor Gross Date: Mon, 9 Dec 2024 09:25:22 +0000 Subject: [PATCH 08/16] dec2flt: Refactor the fast path This is just a bit of code cleanup to make use of returning early. --- library/core/src/num/dec2flt/decimal.rs | 47 ++++++++++++------------- 1 file changed, 23 insertions(+), 24 deletions(-) diff --git a/library/core/src/num/dec2flt/decimal.rs b/library/core/src/num/dec2flt/decimal.rs index 1ba9150a0119d..db7176c124318 100644 --- a/library/core/src/num/dec2flt/decimal.rs +++ b/library/core/src/num/dec2flt/decimal.rs @@ -34,14 +34,15 @@ pub struct Decimal { impl Decimal { /// Detect if the float can be accurately reconstructed from native floats. #[inline] - fn is_fast_path(&self) -> bool { + fn can_use_fast_path(&self) -> bool { F::MIN_EXPONENT_FAST_PATH <= self.exponent && self.exponent <= F::MAX_EXPONENT_DISGUISED_FAST_PATH && self.mantissa <= F::MAX_MANTISSA_FAST_PATH && !self.many_digits } - /// The fast path algorithm using machine-sized integers and floats. + /// Try turning the decimal into an exact float representation, using machine-sized integers + /// and floats. /// /// This is extracted into a separate function so that it can be attempted before constructing /// a Decimal. This only works if both the mantissa and the exponent @@ -59,30 +60,28 @@ impl Decimal { // require setting it by changing the global state (like the control word of the x87 FPU). let _cw = set_precision::(); - if self.is_fast_path::() { - let mut value = if self.exponent <= F::MAX_EXPONENT_FAST_PATH { - // normal fast path - let value = F::from_u64(self.mantissa); - if self.exponent < 0 { - value / F::pow10_fast_path((-self.exponent) as _) - } else { - value * F::pow10_fast_path(self.exponent as _) - } + if !self.can_use_fast_path::() { + return None; + } + + let value = if self.exponent <= F::MAX_EXPONENT_FAST_PATH { + // normal fast path + let value = F::from_u64(self.mantissa); + if self.exponent < 0 { + value / F::pow10_fast_path((-self.exponent) as _) } else { - // disguised fast path - let shift = self.exponent - F::MAX_EXPONENT_FAST_PATH; - let mantissa = self.mantissa.checked_mul(INT_POW10[shift as usize])?; - if mantissa > F::MAX_MANTISSA_FAST_PATH { - return None; - } - F::from_u64(mantissa) * F::pow10_fast_path(F::MAX_EXPONENT_FAST_PATH as _) - }; - if self.negative { - value = -value; + value * F::pow10_fast_path(self.exponent as _) } - Some(value) } else { - None - } + // disguised fast path + let shift = self.exponent - F::MAX_EXPONENT_FAST_PATH; + let mantissa = self.mantissa.checked_mul(INT_POW10[shift as usize])?; + if mantissa > F::MAX_MANTISSA_FAST_PATH { + return None; + } + F::from_u64(mantissa) * F::pow10_fast_path(F::MAX_EXPONENT_FAST_PATH as _) + }; + + if self.negative { Some(-value) } else { Some(value) } } } From 00bf8717e3ab5273b3ec587e3a241408cca23935 Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Sat, 28 Dec 2024 20:54:00 +0100 Subject: [PATCH 09/16] Add GUI test for item info elements color --- tests/rustdoc-gui/item-info.goml | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/tests/rustdoc-gui/item-info.goml b/tests/rustdoc-gui/item-info.goml index b5b0052fe6139..647a2fd290de0 100644 --- a/tests/rustdoc-gui/item-info.goml +++ b/tests/rustdoc-gui/item-info.goml @@ -45,3 +45,26 @@ compare-elements-css: ( "#main-content > .item-info .stab:nth-of-type(2)", ["height"], ) + +// Now checking the text color and the links color. +show-text: true +include: "utils.goml" +go-to: "file://" + |DOC_PATH| + "/lib2/trait.Trait.html" + +call-function: ("switch-theme", {"theme": "ayu"}) +assert-css: (".item-info .stab", {"color": "rgb(197, 197, 197)"}, ALL) +assert-css: (".item-info .stab strong", {"color": "rgb(197, 197, 197)"}, ALL) +assert-css: (".item-info .stab span", {"color": "rgb(197, 197, 197)"}, ALL) +assert-css: (".item-info .stab a", {"color": "rgb(57, 175, 215)"}, ALL) + +call-function: ("switch-theme", {"theme": "dark"}) +assert-css: (".item-info .stab", {"color": "rgb(221, 221, 221)"}, ALL) +assert-css: (".item-info .stab strong", {"color": "rgb(221, 221, 221)"}, ALL) +assert-css: (".item-info .stab span", {"color": "rgb(221, 221, 221)"}, ALL) +assert-css: (".item-info .stab a", {"color": "rgb(210, 153, 29)"}, ALL) + +call-function: ("switch-theme", {"theme": "light"}) +assert-css: (".item-info .stab", {"color": "rgb(0, 0, 0)"}, ALL) +assert-css: (".item-info .stab strong", {"color": "rgb(0, 0, 0)"}, ALL) +assert-css: (".item-info .stab span", {"color": "rgb(0, 0, 0)"}, ALL) +assert-css: (".item-info .stab a", {"color": "rgb(56, 115, 173)"}, ALL) From a985205e08e7c0e1e76fde5647f3815b65bc8601 Mon Sep 17 00:00:00 2001 From: Noratrieb <48135649+Noratrieb@users.noreply.github.com> Date: Fri, 1 Nov 2024 20:47:18 +0100 Subject: [PATCH 10/16] Add more mailmap entries The people updated in this commit have contributed under different email addresses than the ones they have used in rust-lang/team. A new change will use team data for thanks reviewers, which requires this to be in sync. Therefore, I have updated many of the people that I've noticed being duplicated after the change. --- .mailmap | 45 ++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 44 insertions(+), 1 deletion(-) diff --git a/.mailmap b/.mailmap index 874a42656c512..138adb5cc5ba4 100644 --- a/.mailmap +++ b/.mailmap @@ -48,6 +48,7 @@ Andrew Poelstra Anhad Singh Antoine Plaskowski Anton Löfgren +apiraino Araam Borhanian Araam Borhanian Areski Belaid areski @@ -62,7 +63,10 @@ Austin Seipp Ayaz Hafiz Aydin Kim aydin.kim Ayush Mishra +Ashley Mannix asrar +b-naber +b-naber BaoshanPang Barosl Lee Barosl LEE Bastian Kersting @@ -98,6 +102,8 @@ Caleb Cartwright Caleb Jones Noah Lev Noah Lev <37223377+camelid@users.noreply.github.com> +Catherine +Catherine cameron1024 Camille Gillot Carl-Anton Ingmarsson @@ -133,11 +139,13 @@ Clement Miao Clément Renault Cliff Dyer Clinton Ryan +Taylor Cramer ember arlynx Crazycolorz5 csmoe <35686186+csmoe@users.noreply.github.com> Cyryl Płotnicki Damien Schoof +Dan Gohman Dan Robertson Daniel Campoverde Daniel J Rollins @@ -179,10 +187,14 @@ Eduardo Bautista <=> Eduardo Bautista Eduardo Broto Edward Shen +Jacob Finkelman +Jacob Finkelman Elliott Slaughter Elly Fong-Jones Eric Holk Eric Holk +Eric Holk +Eric Holk Eric Holmes Eric Reed Erick Tryzelaar @@ -206,6 +218,7 @@ Felix S. Klock II Félix Saparelli Flaper Fesp Florian Berger +Florian Gilcher Florian Wilkens Florian Wilkens François Mockers Frank Steffahn @@ -240,6 +253,8 @@ Herman J. Radtke III Herman J. Radtke III Hrvoje Nikšić Hsiang-Cheng Yang +Huon Wilson +Huon Wilson Ian Jackson Ian Jackson Ian Jackson @@ -252,9 +267,13 @@ ivan tkachenko J. J. Weber Jack Huey Jacob +Jacob Hoffman-Andrews Jacob Greenfield Jacob Pratt Jacob Pratt +Jake Goulding +Jake Goulding +Jake Goulding Jake Vossen Jakob Degen Jakob Lautrup Nysom @@ -287,6 +306,7 @@ Jerry Hardee Jesús Rubio Jethro Beekman Jian Zeng +Jieyou Xu Jieyou Xu <39484203+jieyouxu@users.noreply.github.com> Jihyun Yu Jihyun Yu jihyun @@ -322,9 +342,12 @@ Josh Holmer Josh Stone Josh Stone Julia Ryan +Jubilee Young <46493976+workingjubilee@users.noreply.github.com> +Jubilee Young Julian Knodt jumbatm <30644300+jumbatm@users.noreply.github.com> Junyoung Cho +Jynn Nelson Jynn Nelson Jynn Nelson Jynn Nelson @@ -385,12 +408,14 @@ Marcell Pardavi Marcus Klaas de Vries Margaret Meyerhofer Mark Mansi +Mark Mansi Mark Rousskov Mark Sinclair Mark Sinclair =Mark Sinclair <=125axel125@gmail.com> Markus Legner Markus Westerlind Markus Martin Carton +Martin Carton Martin Habovštiak Martin Hafskjold Thoresen Martin Nordholts @@ -415,6 +440,7 @@ Melody Horn Mendes mental mibac138 <5672750+mibac138@users.noreply.github.com> +Michael Howell Michael Williams Michael Woerister Michael Woerister @@ -431,6 +457,7 @@ Ms2ger msizanoen1 Mukilan Thiagarajan Nadrieril Feneanar +Nadrieril Feneanar Nadrieril Feneanar NAKASHIMA, Makoto NAKASHIMA, Makoto @@ -447,15 +474,23 @@ Nicholas Bishop Nicholas Bishop Nicholas Nethercote Nicholas Nethercote +Nick Cameron +Nick Fitzgerald Nick Platt Niclas Schwarzlose <15schnic@gmail.com> Nicolas Abram Nicole Mazzuca +Niko Matsakis +Niko Matsakis +Noratrieb <48135649+Noratrieb@users.noreply.github.com> Noratrieb <48135649+Noratrieb@users.noreply.github.com> <48135649+Nilstrieb@users.noreply.github.com> Noratrieb <48135649+Noratrieb@users.noreply.github.com> +Noratrieb <48135649+Noratrieb@users.noreply.github.com> Noratrieb <48135649+Noratrieb@users.noreply.github.com> Nif Ward Nika Layzell +Nikita Popov +Nikita Popov NODA Kai Oğuz Ağcayazı Oğuz Ağcayazı @@ -516,6 +551,7 @@ Ricky Hosfelt Ritiek Malhotra Rob Arnold Rob Arnold Rob Arnold +Robert Collins Robert Foss robertfoss Robert Gawdzik Robert Gawdzik ☢ Robert Habermeier @@ -554,6 +590,11 @@ Simonas Kazlauskas Simonas Kazlauskas Simonas Kazlauskas Siva Prasad Smittyvb +Sophia June Turner <547158+sophiajt@users.noreply.github.com> +Sophia June Turner <547158+sophiajt@users.noreply.github.com> <547158+jntrnr@users.noreply.github.com> +Sophia June Turner <547158+sophiajt@users.noreply.github.com> +Sophia June Turner <547158+sophiajt@users.noreply.github.com> +Sophia June Turner <547158+sophiajt@users.noreply.github.com> Srinivas Reddy Thatiparthy Stanislav Tkach startling @@ -586,8 +627,10 @@ Tim Diekmann Tim Hutt Tim JIANG Tim Joseph Dumol +Tim Neumann Timothy Maloney Tomas Koutsky +Tomasz Miąsko Torsten Weber Torsten Weber Trevor Gross @@ -607,7 +650,7 @@ Valerii Lashmanov Vitali Haravy Vitali Haravy Vitaly Shukela Waffle Lapkin -Waffle Lapkin +Waffle Lapkin Wesley Wiser whitequark William Ting From ca8b12eb5459a98d5c93728bab4ac9d480731d60 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jakub=20Ber=C3=A1nek?= Date: Tue, 31 Dec 2024 15:38:43 +0100 Subject: [PATCH 11/16] Print how to rebless Python formatting in tidy --- src/tools/tidy/src/ext_tool_checks.rs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/tools/tidy/src/ext_tool_checks.rs b/src/tools/tidy/src/ext_tool_checks.rs index 9792650d37d30..e8370a0af027b 100644 --- a/src/tools/tidy/src/ext_tool_checks.rs +++ b/src/tools/tidy/src/ext_tool_checks.rs @@ -154,6 +154,9 @@ fn check_impl( args.insert(0, "--diff".as_ref()); let _ = py_runner(py_path.as_ref().unwrap(), true, None, "ruff", &args); } + if res.is_err() && !bless { + eprintln!("rerun tidy with `--extra-checks=py:fmt --bless` to reformat Python code"); + } // Rethrow error let _ = res?; } From bb16267a65f00defdc5d484375c59f9d93dfa50e Mon Sep 17 00:00:00 2001 From: dxsullivan <193140725+dxsullivan@users.noreply.github.com> Date: Tue, 31 Dec 2024 23:46:39 +0800 Subject: [PATCH 12/16] chore: fix typos Signed-off-by: dxsullivan <193140725+dxsullivan@users.noreply.github.com> --- tests/rustdoc/type-alias/deeply-nested-112515.rs | 2 +- tests/ui/duplicate/multiple-types-with-same-name-and-derive.rs | 2 +- tests/ui/incoherent-inherent-impls/no-other-unrelated-errors.rs | 2 +- tests/ui/indexing/indexing-spans-caller-location.rs | 2 +- .../issue-107745-avoid-expr-from-macro-expansion.rs | 2 +- 5 files changed, 5 insertions(+), 5 deletions(-) diff --git a/tests/rustdoc/type-alias/deeply-nested-112515.rs b/tests/rustdoc/type-alias/deeply-nested-112515.rs index 161188ee5762f..9530feb78de66 100644 --- a/tests/rustdoc/type-alias/deeply-nested-112515.rs +++ b/tests/rustdoc/type-alias/deeply-nested-112515.rs @@ -1,6 +1,6 @@ // Regression test for . // It's to ensure that this code doesn't have infinite loop in rustdoc when -// trying to retrive type alias implementations. +// trying to retrieve type alias implementations. // ignore-tidy-linelength diff --git a/tests/ui/duplicate/multiple-types-with-same-name-and-derive.rs b/tests/ui/duplicate/multiple-types-with-same-name-and-derive.rs index 8d36981b41b2f..72375eb0b3e83 100644 --- a/tests/ui/duplicate/multiple-types-with-same-name-and-derive.rs +++ b/tests/ui/duplicate/multiple-types-with-same-name-and-derive.rs @@ -1,6 +1,6 @@ // Here, there are two types with the same name. One of these has a `derive` annotation, but in the // expansion these `impl`s are associated to the the *other* type. There is a suggestion to remove -// unneded type parameters, but because we're now point at a type with no type parameters, the +// unneeded type parameters, but because we're now point at a type with no type parameters, the // suggestion would suggest removing code from an empty span, which would ICE in nightly. // // issue: rust-lang/rust#108748 diff --git a/tests/ui/incoherent-inherent-impls/no-other-unrelated-errors.rs b/tests/ui/incoherent-inherent-impls/no-other-unrelated-errors.rs index 8eaa0c9194a80..cef017e79a48f 100644 --- a/tests/ui/incoherent-inherent-impls/no-other-unrelated-errors.rs +++ b/tests/ui/incoherent-inherent-impls/no-other-unrelated-errors.rs @@ -1,4 +1,4 @@ -// E0116 caused other unrelated errors, so check no unrelated errors are emmitted. +// E0116 caused other unrelated errors, so check no unrelated errors are emitted. fn main() { let x = "hello"; diff --git a/tests/ui/indexing/indexing-spans-caller-location.rs b/tests/ui/indexing/indexing-spans-caller-location.rs index 02d8b853734e5..b01e3894ac1bd 100644 --- a/tests/ui/indexing/indexing-spans-caller-location.rs +++ b/tests/ui/indexing/indexing-spans-caller-location.rs @@ -20,7 +20,7 @@ impl std::ops::Index for A { type Output = (); fn index(&self, _idx: usize) -> &() { - // Use the relative number to make it resistent to header changes. + // Use the relative number to make it resistant to header changes. assert_eq!(caller_line(), self.prev_line + 2); &() } diff --git a/tests/ui/inference/need_type_info/issue-107745-avoid-expr-from-macro-expansion.rs b/tests/ui/inference/need_type_info/issue-107745-avoid-expr-from-macro-expansion.rs index 7f6758f47f8fe..3cdb488e7a545 100644 --- a/tests/ui/inference/need_type_info/issue-107745-avoid-expr-from-macro-expansion.rs +++ b/tests/ui/inference/need_type_info/issue-107745-avoid-expr-from-macro-expansion.rs @@ -2,7 +2,7 @@ // Regression test for #107745. // Previously need_type_info::update_infer_source will consider expressions originating from -// macro expressions as candiate "previous sources". This unfortunately can mean that +// macro expressions as candidate "previous sources". This unfortunately can mean that // for macros expansions such as `format!()` internal implementation details can leak, such as: // // ``` From b4a092662cd656f4a28ae720bc600684d54b55c8 Mon Sep 17 00:00:00 2001 From: Eric Huss Date: Tue, 31 Dec 2024 07:57:06 -0800 Subject: [PATCH 13/16] Revert "Rollup merge of #119515 - joshtriplett:style-guide-gat-where-clause-same-line, r=compiler-errors" This reverts commit 4d1cce9de5af0e74169c6508d44c99b546d7abde, reversing changes made to 030a12ce2b1ee42f2d8837b1b500fd9cf12ea191. --- src/doc/style-guide/src/editions.md | 1 - src/doc/style-guide/src/items.md | 30 ++++------------------------- 2 files changed, 4 insertions(+), 27 deletions(-) diff --git a/src/doc/style-guide/src/editions.md b/src/doc/style-guide/src/editions.md index d9dba641495ab..17e8a48fd73b8 100644 --- a/src/doc/style-guide/src/editions.md +++ b/src/doc/style-guide/src/editions.md @@ -46,7 +46,6 @@ include: - Miscellaneous `rustfmt` bugfixes. - Use version-sort (sort `x8`, `x16`, `x32`, `x64`, `x128` in that order). - Change "ASCIIbetical" sort to Unicode-aware "non-lowercase before lowercase". -- Format single associated type `where` clauses on the same line if they fit. ## Rust 2015/2018/2021 style edition diff --git a/src/doc/style-guide/src/items.md b/src/doc/style-guide/src/items.md index 5ea8b6cd54294..be361eee330ac 100644 --- a/src/doc/style-guide/src/items.md +++ b/src/doc/style-guide/src/items.md @@ -295,18 +295,8 @@ Prefer to use single-letter names for generic parameters. These rules apply for `where` clauses on any item. -If a where clause is short, and appears on a short one-line function -declaration with no body or on a short type with no `=`, format it on -the same line as the declaration: - -```rust -fn new(&self) -> Self where Self: Sized; - -type Item<'a>: SomeTrait where Self: 'a; -``` - -Otherwise, if immediately following a closing bracket of any kind, write the -keyword `where` on the same line, with a space before it. +If immediately following a closing bracket of any kind, write the keyword +`where` on the same line, with a space before it. Otherwise, put `where` on a new line at the same indentation level. Put each component of a `where` clause on its own line, block-indented. Use a trailing @@ -357,7 +347,7 @@ where ``` If a `where` clause is very short, prefer using an inline bound on the type -parameter if possible. +parameter. If a component of a `where` clause does not fit and contains `+`, break it before each `+` and block-indent the continuation lines. Put each bound on its @@ -431,21 +421,9 @@ Format associated types like type aliases. Where an associated type has a bound, put a space after the colon but not before: ```rust -type Foo: Bar; +pub type Foo: Bar; ``` -If an associated type is short, has no `=`, and has a `where` clause with only -one entry, format the entire type declaration including the `where` clause on -the same line if it fits: - -```rust -type Item<'a> where Self: 'a; -type Item<'a>: PartialEq + Send where Self: 'a; -``` - -If the associated type has a `=`, or if the `where` clause contains multiple -entries, format it across multiple lines as with a type alias. - ## extern items When writing extern items (such as `extern "C" fn`), always specify the ABI. From 19387e65188dce96200b7ac5b9df99a5f75cf824 Mon Sep 17 00:00:00 2001 From: Skgland <3877590+Skgland@users.noreply.github.com> Date: Tue, 31 Dec 2024 17:02:29 +0100 Subject: [PATCH 14/16] add .mailmap entry for myself --- .mailmap | 3 +++ 1 file changed, 3 insertions(+) diff --git a/.mailmap b/.mailmap index 874a42656c512..295eb644d9fb7 100644 --- a/.mailmap +++ b/.mailmap @@ -553,6 +553,9 @@ Simon Sapin Simonas Kazlauskas Simonas Kazlauskas Simonas Kazlauskas Siva Prasad +Skgland <3877590+Skgland@users.noreply.github.com> +Skgland <3877590+Skgland@users.noreply.github.com> +Skgland <3877590+Skgland@users.noreply.github.com> Smittyvb Srinivas Reddy Thatiparthy Stanislav Tkach From a6ba04ae6a9f76a0513a3084dfd2a4ebab4b381f Mon Sep 17 00:00:00 2001 From: Eric Huss Date: Tue, 31 Dec 2024 08:50:04 -0800 Subject: [PATCH 15/16] Revert "Rollup merge of #132369 - joshtriplett:style-guide-binop-heuristic-assignment-only, r=calebcartwright" This reverts commit 348d28052b1717f152b04725492c256c3409a361, reversing changes made to 526c67f37be44688345aec14f7b1c5926f4a59a7. --- src/doc/style-guide/src/editions.md | 5 ++--- src/doc/style-guide/src/expressions.md | 12 +++++++++--- 2 files changed, 11 insertions(+), 6 deletions(-) diff --git a/src/doc/style-guide/src/editions.md b/src/doc/style-guide/src/editions.md index d9dba641495ab..74e873e35ff38 100644 --- a/src/doc/style-guide/src/editions.md +++ b/src/doc/style-guide/src/editions.md @@ -40,9 +40,8 @@ include: of a delimited expression, delimited expressions are generally combinable, regardless of the number of members. Previously only applied with exactly one member (except for closures with explicit blocks). -- When line-breaking an assignment operator, if the left-hand side spans - multiple lines, use the base indentation of the last line of the left-hand - side to indent the right-hand side. +- When line-breaking a binary operator, if the first operand spans multiple + lines, use the base indentation of the last line. - Miscellaneous `rustfmt` bugfixes. - Use version-sort (sort `x8`, `x16`, `x32`, `x64`, `x128` in that order). - Change "ASCIIbetical" sort to Unicode-aware "non-lowercase before lowercase". diff --git a/src/doc/style-guide/src/expressions.md b/src/doc/style-guide/src/expressions.md index 4f63a632030b4..3bb0ee6d5ff6c 100644 --- a/src/doc/style-guide/src/expressions.md +++ b/src/doc/style-guide/src/expressions.md @@ -328,9 +328,9 @@ foo_bar Prefer line-breaking at an assignment operator (either `=` or `+=`, etc.) rather than at other binary operators. -If line-breaking an assignment operator where the left-hand side spans multiple -lines, use the base indentation of the *last* line of the left-hand side, and -indent the right-hand side relative to that: +If line-breaking at a binary operator (including assignment operators) where the +first operand spans multiple lines, use the base indentation of the *last* +line of the first operand, and indent relative to that: ```rust impl SomeType { @@ -341,6 +341,12 @@ impl SomeType { .extra_info = long_long_long_long_long_long_long_long_long_long_long_long_long_long_long; + self.array[array_index as usize] + .as_mut() + .expect("thing must exist") + .extra_info + + long_long_long_long_long_long_long_long_long_long_long_long_long_long_long; + self.array[array_index as usize] .as_mut() .expect("thing must exist") From 7a46c7b1123fa3643e9bda0c9ffbfc5dee7e4dfb Mon Sep 17 00:00:00 2001 From: Eric Huss Date: Tue, 31 Dec 2024 08:50:28 -0800 Subject: [PATCH 16/16] Revert "Rollup merge of #119838 - joshtriplett:style-guide-binop-indent, r=compiler-errors" This reverts commit 36287830a22897fc05f33f615291df7efe8cad20, reversing changes made to 31026b7fe3e510a646eddeda838d1f0859f892e7. --- src/doc/style-guide/src/editions.md | 2 -- src/doc/style-guide/src/expressions.md | 31 -------------------------- 2 files changed, 33 deletions(-) diff --git a/src/doc/style-guide/src/editions.md b/src/doc/style-guide/src/editions.md index 74e873e35ff38..9d593f8081025 100644 --- a/src/doc/style-guide/src/editions.md +++ b/src/doc/style-guide/src/editions.md @@ -40,8 +40,6 @@ include: of a delimited expression, delimited expressions are generally combinable, regardless of the number of members. Previously only applied with exactly one member (except for closures with explicit blocks). -- When line-breaking a binary operator, if the first operand spans multiple - lines, use the base indentation of the last line. - Miscellaneous `rustfmt` bugfixes. - Use version-sort (sort `x8`, `x16`, `x32`, `x64`, `x128` in that order). - Change "ASCIIbetical" sort to Unicode-aware "non-lowercase before lowercase". diff --git a/src/doc/style-guide/src/expressions.md b/src/doc/style-guide/src/expressions.md index 3bb0ee6d5ff6c..171a24cd89d73 100644 --- a/src/doc/style-guide/src/expressions.md +++ b/src/doc/style-guide/src/expressions.md @@ -328,37 +328,6 @@ foo_bar Prefer line-breaking at an assignment operator (either `=` or `+=`, etc.) rather than at other binary operators. -If line-breaking at a binary operator (including assignment operators) where the -first operand spans multiple lines, use the base indentation of the *last* -line of the first operand, and indent relative to that: - -```rust -impl SomeType { - fn method(&mut self) { - self.array[array_index as usize] - .as_mut() - .expect("thing must exist") - .extra_info = - long_long_long_long_long_long_long_long_long_long_long_long_long_long_long; - - self.array[array_index as usize] - .as_mut() - .expect("thing must exist") - .extra_info - + long_long_long_long_long_long_long_long_long_long_long_long_long_long_long; - - self.array[array_index as usize] - .as_mut() - .expect("thing must exist") - .extra_info = Some(ExtraInfo { - parent, - count: count as u16, - children: children.into_boxed_slice(), - }); - } -} -``` - ### Casts (`as`) Format `as` casts like a binary operator. In particular, always include spaces