From 1cac5fa5f9f371db4789ea928e1d2769d4cd921b Mon Sep 17 00:00:00 2001 From: jyn Date: Thu, 13 Feb 2025 10:14:06 -0500 Subject: [PATCH 01/24] Look for `python3` first on MacOS, not `py` `py` is not installed by default *and* trying to run it results in a popup asking if you want to install it. `python3` is installed by default. This hopefully should not be too disruptive to people on Windows, since they should be going through `x.ps1` instead anyway. Just in case, I've added a check for Cygwin and Msys (i'm not sure how else you'd get a bash shell on windows). --- x | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/x b/x index e656d37c1e455..551cfe6efbf33 100755 --- a/x +++ b/x @@ -25,7 +25,13 @@ xpy=$(dirname "$(realpath "$0")")/x.py # On Windows, `py -3` sometimes works. We need to try it first because `python3` # sometimes tries to launch the app store on Windows. -for SEARCH_PYTHON in py python3 python python2; do +# On MacOS, `py` tries to install "Developer command line tools". Try `python3` first. +# NOTE: running `bash -c ./x` from Windows doesn't set OSTYPE. +case ${OSTYPE:-} in + cygwin*|msys*) SEARCH="py python3 python python2";; + *) SEARCH="python3 python py python2";; +esac +for SEARCH_PYTHON in $SEARCH; do if python=$(command -v $SEARCH_PYTHON) && [ -x "$python" ]; then if [ $SEARCH_PYTHON = py ]; then extra_arg="-3" From 477a2eeb3dd5a430bb3845c8f8041ff369232967 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E8=AE=B8=E6=9D=B0=E5=8F=8B=20Jieyou=20Xu=20=28Joe=29?= <39484203+jieyouxu@users.noreply.github.com> Date: Wed, 19 Feb 2025 02:00:02 +0800 Subject: [PATCH 02/24] std::fs: slightly reformat `remove_dir_all` error docs To make the error cases easier to spot on a quick glance. --- library/std/src/fs.rs | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/library/std/src/fs.rs b/library/std/src/fs.rs index 6001a2e2f391f..2b3a84d0a12c2 100644 --- a/library/std/src/fs.rs +++ b/library/std/src/fs.rs @@ -2841,9 +2841,11 @@ pub fn remove_dir>(path: P) -> io::Result<()> { /// /// See [`fs::remove_file`] and [`fs::remove_dir`]. /// -/// `remove_dir_all` will fail if `remove_dir` or `remove_file` fail on any constituent paths, including the root `path`. -/// As a result, the directory you are deleting must exist, meaning that this function is not idempotent. -/// Additionally, `remove_dir_all` will also fail if the `path` is not a directory. +/// [`remove_dir_all`] will fail if [`remove_dir`] or [`remove_file`] fail on *any* constituent +/// paths, *including* the root `path`. Consequently, +/// +/// - The directory you are deleting *must* exist, meaning that this function is *not idempotent*. +/// - [`remove_dir_all`] will fail if the `path` is *not* a directory. /// /// Consider ignoring the error if validating the removal is not required for your use case. /// From 2c752bcf559975995eb8086a7fa6a7f9b5ba0de8 Mon Sep 17 00:00:00 2001 From: Arlo Siemsen Date: Thu, 20 Feb 2025 11:47:14 -0600 Subject: [PATCH 03/24] Undeprecate env::home_dir --- library/std/src/env.rs | 5 ----- 1 file changed, 5 deletions(-) diff --git a/library/std/src/env.rs b/library/std/src/env.rs index adbd68896241c..5a931b077cdb0 100644 --- a/library/std/src/env.rs +++ b/library/std/src/env.rs @@ -641,11 +641,6 @@ impl Error for JoinPathsError { /// None => println!("Impossible to get your home dir!"), /// } /// ``` -#[deprecated( - since = "1.29.0", - note = "This function's behavior may be unexpected on Windows. \ - Consider using a crate from crates.io instead." -)] #[must_use] #[stable(feature = "env", since = "1.0.0")] pub fn home_dir() -> Option { From 9323ba54d3b35aeaf55a9596a29682c7173cf4d2 Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Thu, 20 Feb 2025 03:04:13 +0000 Subject: [PATCH 04/24] Remove MaybeForgetReturn suggestion --- compiler/rustc_errors/src/lib.rs | 1 - .../rustc_hir_typeck/src/fn_ctxt/_impl.rs | 5 -- .../rustc_hir_typeck/src/fn_ctxt/checks.rs | 60 +------------------ .../src/error_reporting/traits/ambiguity.rs | 8 +-- ...suggest-add-return-to-coerce-ret-ty.stderr | 8 --- .../return/tail-expr-as-potential-return.rs | 1 - .../tail-expr-as-potential-return.stderr | 4 -- 7 files changed, 4 insertions(+), 83 deletions(-) diff --git a/compiler/rustc_errors/src/lib.rs b/compiler/rustc_errors/src/lib.rs index 6fce1fade2664..9df306bb63540 100644 --- a/compiler/rustc_errors/src/lib.rs +++ b/compiler/rustc_errors/src/lib.rs @@ -599,7 +599,6 @@ pub enum StashKey { MaybeFruTypo, CallAssocMethod, AssociatedTypeSuggestion, - MaybeForgetReturn, /// Query cycle detected, stashing in favor of a better error. Cycle, UndeterminedMacroResolution, diff --git a/compiler/rustc_hir_typeck/src/fn_ctxt/_impl.rs b/compiler/rustc_hir_typeck/src/fn_ctxt/_impl.rs index 2d7d80e39bc31..5bd190cda9503 100644 --- a/compiler/rustc_hir_typeck/src/fn_ctxt/_impl.rs +++ b/compiler/rustc_hir_typeck/src/fn_ctxt/_impl.rs @@ -666,12 +666,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { if !errors.is_empty() { self.adjust_fulfillment_errors_for_expr_obligation(&mut errors); - let errors_causecode = errors - .iter() - .map(|e| (e.obligation.cause.span, e.root_obligation.cause.code().clone())) - .collect::>(); self.err_ctxt().report_fulfillment_errors(errors); - self.collect_unused_stmts_for_coerce_return_ty(errors_causecode); } } diff --git a/compiler/rustc_hir_typeck/src/fn_ctxt/checks.rs b/compiler/rustc_hir_typeck/src/fn_ctxt/checks.rs index cf61659479b13..f1b60fabfefd9 100644 --- a/compiler/rustc_hir_typeck/src/fn_ctxt/checks.rs +++ b/compiler/rustc_hir_typeck/src/fn_ctxt/checks.rs @@ -3,9 +3,7 @@ use std::{fmt, iter, mem}; use itertools::Itertools; use rustc_data_structures::fx::FxIndexSet; use rustc_errors::codes::*; -use rustc_errors::{ - Applicability, Diag, ErrorGuaranteed, MultiSpan, StashKey, a_or_an, listify, pluralize, -}; +use rustc_errors::{Applicability, Diag, ErrorGuaranteed, MultiSpan, a_or_an, listify, pluralize}; use rustc_hir::def::{CtorOf, DefKind, Res}; use rustc_hir::def_id::DefId; use rustc_hir::intravisit::Visitor; @@ -2167,62 +2165,6 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { } } - pub(super) fn collect_unused_stmts_for_coerce_return_ty( - &self, - errors_causecode: Vec<(Span, ObligationCauseCode<'tcx>)>, - ) { - for (span, code) in errors_causecode { - self.dcx().try_steal_modify_and_emit_err(span, StashKey::MaybeForgetReturn, |err| { - if let Some(fn_sig) = self.body_fn_sig() - && let ObligationCauseCode::WhereClauseInExpr(_, _, binding_hir_id, ..) = code - && !fn_sig.output().is_unit() - { - let mut block_num = 0; - let mut found_semi = false; - for (hir_id, node) in self.tcx.hir_parent_iter(binding_hir_id) { - // Don't proceed into parent bodies - if hir_id.owner != binding_hir_id.owner { - break; - } - match node { - hir::Node::Stmt(stmt) => { - if let hir::StmtKind::Semi(expr) = stmt.kind { - let expr_ty = self.typeck_results.borrow().expr_ty(expr); - let return_ty = fn_sig.output(); - if !matches!(expr.kind, hir::ExprKind::Ret(..)) - && self.may_coerce(expr_ty, return_ty) - { - found_semi = true; - } - } - } - hir::Node::Block(_block) => { - if found_semi { - block_num += 1; - } - } - hir::Node::Item(item) => { - if let hir::ItemKind::Fn { .. } = item.kind { - break; - } - } - _ => {} - } - } - if block_num > 1 && found_semi { - err.span_suggestion_verbose( - // use the span of the *whole* expr - self.tcx.hir().span(binding_hir_id).shrink_to_lo(), - "you might have meant to return this to infer its type parameters", - "return ", - Applicability::MaybeIncorrect, - ); - } - } - }); - } - } - /// Given a vector of fulfillment errors, try to adjust the spans of the /// errors to more accurately point at the cause of the failure. /// diff --git a/compiler/rustc_trait_selection/src/error_reporting/traits/ambiguity.rs b/compiler/rustc_trait_selection/src/error_reporting/traits/ambiguity.rs index f15f1b78b5282..d673e5672a00b 100644 --- a/compiler/rustc_trait_selection/src/error_reporting/traits/ambiguity.rs +++ b/compiler/rustc_trait_selection/src/error_reporting/traits/ambiguity.rs @@ -1,8 +1,6 @@ use std::ops::ControlFlow; -use rustc_errors::{ - Applicability, Diag, E0283, E0284, E0790, MultiSpan, StashKey, struct_span_code_err, -}; +use rustc_errors::{Applicability, Diag, E0283, E0284, E0790, MultiSpan, struct_span_code_err}; use rustc_hir as hir; use rustc_hir::LangItem; use rustc_hir::def::{DefKind, Res}; @@ -197,7 +195,7 @@ impl<'a, 'tcx> TypeErrCtxt<'a, 'tcx> { // be ignoring the fact that we don't KNOW the type works // out. Though even that would probably be harmless, given that // we're only talking about builtin traits, which are known to be - // inhabited. We used to check for `self.tcx.sess.has_errors()` to + // inhabited. We used to check for `self.tainted_by_errors()` to // avoid inundating the user with unnecessary errors, but we now // check upstream for type errors and don't add the obligations to // begin with in those cases. @@ -211,7 +209,7 @@ impl<'a, 'tcx> TypeErrCtxt<'a, 'tcx> { TypeAnnotationNeeded::E0282, false, ); - return err.stash(span, StashKey::MaybeForgetReturn).unwrap(); + return err.emit(); } Some(e) => return e, } diff --git a/tests/ui/inference/issue-86094-suggest-add-return-to-coerce-ret-ty.stderr b/tests/ui/inference/issue-86094-suggest-add-return-to-coerce-ret-ty.stderr index 1fea73529a8a2..c61ca699b0d35 100644 --- a/tests/ui/inference/issue-86094-suggest-add-return-to-coerce-ret-ty.stderr +++ b/tests/ui/inference/issue-86094-suggest-add-return-to-coerce-ret-ty.stderr @@ -8,10 +8,6 @@ help: consider specifying the generic arguments | LL | Err::(MyError); | ++++++++++++++ -help: you might have meant to return this to infer its type parameters - | -LL | return Err(MyError); - | ++++++ error[E0282]: type annotations needed --> $DIR/issue-86094-suggest-add-return-to-coerce-ret-ty.rs:14:9 @@ -23,10 +19,6 @@ help: consider specifying the generic arguments | LL | Ok::<(), E>(()); | +++++++++ -help: you might have meant to return this to infer its type parameters - | -LL | return Ok(()); - | ++++++ error[E0308]: mismatched types --> $DIR/issue-86094-suggest-add-return-to-coerce-ret-ty.rs:21:20 diff --git a/tests/ui/return/tail-expr-as-potential-return.rs b/tests/ui/return/tail-expr-as-potential-return.rs index 11ecddb049b57..2e638f1897c22 100644 --- a/tests/ui/return/tail-expr-as-potential-return.rs +++ b/tests/ui/return/tail-expr-as-potential-return.rs @@ -60,7 +60,6 @@ fn method() -> Option { Receiver.generic(); //~^ ERROR type annotations needed //~| HELP consider specifying the generic argument - //~| HELP you might have meant to return this to infer its type parameters } None diff --git a/tests/ui/return/tail-expr-as-potential-return.stderr b/tests/ui/return/tail-expr-as-potential-return.stderr index 756de2b5a1668..8105b2df3fea6 100644 --- a/tests/ui/return/tail-expr-as-potential-return.stderr +++ b/tests/ui/return/tail-expr-as-potential-return.stderr @@ -57,10 +57,6 @@ help: consider specifying the generic argument | LL | Receiver.generic::(); | +++++ -help: you might have meant to return this to infer its type parameters - | -LL | return Receiver.generic(); - | ++++++ error: aborting due to 4 previous errors From b34054511401f7aed8e76b8d5663651b5b4ced2a Mon Sep 17 00:00:00 2001 From: Rain Date: Tue, 11 Feb 2025 22:50:26 +0000 Subject: [PATCH 05/24] [illumos] attempt to use posix_spawn to spawn processes illumos has `posix_spawn`, and the very newest versions also have `_addchdir`, so use that. This is a nice ~4x performance improvement for process creation. My go-to as usual is nextest against the clap repo, which acts as a stress test for process creation -- with [this commit]: ```console $ cargo nextest run -E 'not test(ui_tests) and not test(example_tests)' before: Summary [ 1.747s] 879 tests run: 879 passed, 2 skipped after: Summary [ 0.445s] 879 tests run: 879 passed, 2 skipped ``` [this commit]: https://github.com/clap-rs/clap/commit/fde45f9aea766fb8de46e3d46e6575f393c3b6b9 --- library/Cargo.lock | 4 ++-- library/std/Cargo.toml | 2 +- .../src/sys/pal/unix/process/process_unix.rs | 17 ++++++++++++++++- 3 files changed, 19 insertions(+), 4 deletions(-) diff --git a/library/Cargo.lock b/library/Cargo.lock index 0be2f9a154939..6bc3cf4da9f72 100644 --- a/library/Cargo.lock +++ b/library/Cargo.lock @@ -151,9 +151,9 @@ dependencies = [ [[package]] name = "libc" -version = "0.2.169" +version = "0.2.170" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "b5aba8db14291edd000dfcc4d620c7ebfb122c613afb886ca8803fa4e128a20a" +checksum = "875b3680cb2f8f71bdcf9a30f38d48282f5d3c95cbf9b3fa57269bb5d5c06828" dependencies = [ "rustc-std-workspace-core", ] diff --git a/library/std/Cargo.toml b/library/std/Cargo.toml index 228ee6eea05fa..cec5cdbb92c27 100644 --- a/library/std/Cargo.toml +++ b/library/std/Cargo.toml @@ -35,7 +35,7 @@ miniz_oxide = { version = "0.8.0", optional = true, default-features = false } addr2line = { version = "0.24.0", optional = true, default-features = false } [target.'cfg(not(all(windows, target_env = "msvc")))'.dependencies] -libc = { version = "0.2.169", default-features = false, features = [ +libc = { version = "0.2.170", default-features = false, features = [ 'rustc-dep-of-std', ], public = true } diff --git a/library/std/src/sys/pal/unix/process/process_unix.rs b/library/std/src/sys/pal/unix/process/process_unix.rs index aa7406dd54874..7c5b0fca11a10 100644 --- a/library/std/src/sys/pal/unix/process/process_unix.rs +++ b/library/std/src/sys/pal/unix/process/process_unix.rs @@ -410,6 +410,7 @@ impl Command { #[cfg(not(any( target_os = "freebsd", + target_os = "illumos", all(target_os = "linux", target_env = "gnu"), all(target_os = "linux", target_env = "musl"), target_os = "nto", @@ -427,6 +428,7 @@ impl Command { // directly. #[cfg(any( target_os = "freebsd", + target_os = "illumos", all(target_os = "linux", target_env = "gnu"), all(target_os = "linux", target_env = "musl"), target_os = "nto", @@ -584,6 +586,10 @@ impl Command { fn get_posix_spawn_addchdir() -> Option { use crate::sys::weak::weak; + // POSIX.1-2024 standardizes this function: + // https://pubs.opengroup.org/onlinepubs/9799919799/functions/posix_spawn_file_actions_addchdir.html. + // The _np version is more widely available, though, so try that first. + weak! { fn posix_spawn_file_actions_addchdir_np( *mut libc::posix_spawn_file_actions_t, @@ -591,7 +597,16 @@ impl Command { ) -> libc::c_int } - posix_spawn_file_actions_addchdir_np.get() + weak! { + fn posix_spawn_file_actions_addchdir( + *mut libc::posix_spawn_file_actions_t, + *const libc::c_char + ) -> libc::c_int + } + + posix_spawn_file_actions_addchdir_np + .get() + .or_else(|| posix_spawn_file_actions_addchdir.get()) } /// Get the function pointer for adding a chdir action to a From a8bff87cfb15396b4ddf80743652b7e234bd0100 Mon Sep 17 00:00:00 2001 From: Taylor Cramer Date: Fri, 28 Feb 2025 14:04:54 -0800 Subject: [PATCH 06/24] Stabilize [T]::split_off... methods This was previously known as the slice_take feature. --- library/core/src/slice/mod.rs | 34 +++++++--------------------------- library/coretests/tests/lib.rs | 1 - 2 files changed, 7 insertions(+), 28 deletions(-) diff --git a/library/core/src/slice/mod.rs b/library/core/src/slice/mod.rs index 7a2764206e8db..f991cc4ae2dbb 100644 --- a/library/core/src/slice/mod.rs +++ b/library/core/src/slice/mod.rs @@ -78,7 +78,7 @@ pub use raw::{from_raw_parts, from_raw_parts_mut}; /// Calculates the direction and split point of a one-sided range. /// -/// This is a helper function for `take` and `take_mut` that returns +/// This is a helper function for `split_off` and `split_off_mut` that returns /// the direction of the split (front or back) as well as the index at /// which to split. Returns `None` if the split index would overflow. #[inline] @@ -4313,8 +4313,6 @@ impl [T] { /// Splitting off the first three elements of a slice: /// /// ``` - /// #![feature(slice_take)] - /// /// let mut slice: &[_] = &['a', 'b', 'c', 'd']; /// let mut first_three = slice.split_off(..3).unwrap(); /// @@ -4325,8 +4323,6 @@ impl [T] { /// Splitting off the last two elements of a slice: /// /// ``` - /// #![feature(slice_take)] - /// /// let mut slice: &[_] = &['a', 'b', 'c', 'd']; /// let mut tail = slice.split_off(2..).unwrap(); /// @@ -4337,8 +4333,6 @@ impl [T] { /// Getting `None` when `range` is out of bounds: /// /// ``` - /// #![feature(slice_take)] - /// /// let mut slice: &[_] = &['a', 'b', 'c', 'd']; /// /// assert_eq!(None, slice.split_off(5..)); @@ -4349,7 +4343,7 @@ impl [T] { /// ``` #[inline] #[must_use = "method does not modify the slice if the range is out of bounds"] - #[unstable(feature = "slice_take", issue = "62280")] + #[stable(feature = "slice_take", since = "CURRENT_RUSTC_VERSION")] pub fn split_off<'a, R: OneSidedRange>( self: &mut &'a Self, range: R, @@ -4385,8 +4379,6 @@ impl [T] { /// Splitting off the first three elements of a slice: /// /// ``` - /// #![feature(slice_take)] - /// /// let mut slice: &mut [_] = &mut ['a', 'b', 'c', 'd']; /// let mut first_three = slice.split_off_mut(..3).unwrap(); /// @@ -4397,8 +4389,6 @@ impl [T] { /// Taking the last two elements of a slice: /// /// ``` - /// #![feature(slice_take)] - /// /// let mut slice: &mut [_] = &mut ['a', 'b', 'c', 'd']; /// let mut tail = slice.split_off_mut(2..).unwrap(); /// @@ -4409,8 +4399,6 @@ impl [T] { /// Getting `None` when `range` is out of bounds: /// /// ``` - /// #![feature(slice_take)] - /// /// let mut slice: &mut [_] = &mut ['a', 'b', 'c', 'd']; /// /// assert_eq!(None, slice.split_off_mut(5..)); @@ -4421,7 +4409,7 @@ impl [T] { /// ``` #[inline] #[must_use = "method does not modify the slice if the range is out of bounds"] - #[unstable(feature = "slice_take", issue = "62280")] + #[stable(feature = "slice_take", since = "CURRENT_RUSTC_VERSION")] pub fn split_off_mut<'a, R: OneSidedRange>( self: &mut &'a mut Self, range: R, @@ -4451,8 +4439,6 @@ impl [T] { /// # Examples /// /// ``` - /// #![feature(slice_take)] - /// /// let mut slice: &[_] = &['a', 'b', 'c']; /// let first = slice.split_off_first().unwrap(); /// @@ -4460,7 +4446,7 @@ impl [T] { /// assert_eq!(first, &'a'); /// ``` #[inline] - #[unstable(feature = "slice_take", issue = "62280")] + #[stable(feature = "slice_take", since = "CURRENT_RUSTC_VERSION")] pub fn split_off_first<'a>(self: &mut &'a Self) -> Option<&'a T> { let (first, rem) = self.split_first()?; *self = rem; @@ -4475,8 +4461,6 @@ impl [T] { /// # Examples /// /// ``` - /// #![feature(slice_take)] - /// /// let mut slice: &mut [_] = &mut ['a', 'b', 'c']; /// let first = slice.split_off_first_mut().unwrap(); /// *first = 'd'; @@ -4485,7 +4469,7 @@ impl [T] { /// assert_eq!(first, &'d'); /// ``` #[inline] - #[unstable(feature = "slice_take", issue = "62280")] + #[stable(feature = "slice_take", since = "CURRENT_RUSTC_VERSION")] pub fn split_off_first_mut<'a>(self: &mut &'a mut Self) -> Option<&'a mut T> { let (first, rem) = mem::take(self).split_first_mut()?; *self = rem; @@ -4500,8 +4484,6 @@ impl [T] { /// # Examples /// /// ``` - /// #![feature(slice_take)] - /// /// let mut slice: &[_] = &['a', 'b', 'c']; /// let last = slice.split_off_last().unwrap(); /// @@ -4509,7 +4491,7 @@ impl [T] { /// assert_eq!(last, &'c'); /// ``` #[inline] - #[unstable(feature = "slice_take", issue = "62280")] + #[stable(feature = "slice_take", since = "CURRENT_RUSTC_VERSION")] pub fn split_off_last<'a>(self: &mut &'a Self) -> Option<&'a T> { let (last, rem) = self.split_last()?; *self = rem; @@ -4524,8 +4506,6 @@ impl [T] { /// # Examples /// /// ``` - /// #![feature(slice_take)] - /// /// let mut slice: &mut [_] = &mut ['a', 'b', 'c']; /// let last = slice.split_off_last_mut().unwrap(); /// *last = 'd'; @@ -4534,7 +4514,7 @@ impl [T] { /// assert_eq!(last, &'d'); /// ``` #[inline] - #[unstable(feature = "slice_take", issue = "62280")] + #[stable(feature = "slice_take", since = "CURRENT_RUSTC_VERSION")] pub fn split_off_last_mut<'a>(self: &mut &'a mut Self) -> Option<&'a mut T> { let (last, rem) = mem::take(self).split_last_mut()?; *self = rem; diff --git a/library/coretests/tests/lib.rs b/library/coretests/tests/lib.rs index 4f21ae5013b66..b3c8e94806d48 100644 --- a/library/coretests/tests/lib.rs +++ b/library/coretests/tests/lib.rs @@ -73,7 +73,6 @@ #![feature(slice_internals)] #![feature(slice_partition_dedup)] #![feature(slice_split_once)] -#![feature(slice_take)] #![feature(split_array)] #![feature(split_as_slice)] #![feature(std_internals)] From ea13ff722db1f2382dcdfaf0cdd7c2d73da0f097 Mon Sep 17 00:00:00 2001 From: bit-aloo Date: Sun, 16 Feb 2025 23:24:29 +0530 Subject: [PATCH 07/24] add exclude to config.toml --- config.example.toml | 4 +++ src/bootstrap/src/core/config/config.rs | 38 ++++++++++++++----------- 2 files changed, 26 insertions(+), 16 deletions(-) diff --git a/config.example.toml b/config.example.toml index 2e26c024865db..d27633423a935 100644 --- a/config.example.toml +++ b/config.example.toml @@ -425,6 +425,10 @@ # a specific version. #ccache = false +# List of paths to exclude from the build and test processes. +# For example, exclude = ["tests/ui", "src/tools/tidy"]. +#exclude = [] + # ============================================================================= # General install configuration options # ============================================================================= diff --git a/src/bootstrap/src/core/config/config.rs b/src/bootstrap/src/core/config/config.rs index d0e0ed50ad89f..0791604dc034d 100644 --- a/src/bootstrap/src/core/config/config.rs +++ b/src/bootstrap/src/core/config/config.rs @@ -942,6 +942,7 @@ define_config! { jobs: Option = "jobs", compiletest_diff_tool: Option = "compiletest-diff-tool", ccache: Option = "ccache", + exclude: Option> = "exclude", } } @@ -1372,22 +1373,6 @@ impl Config { "flags.exclude" = ?flags.exclude ); - config.skip = flags - .skip - .into_iter() - .chain(flags.exclude) - .map(|p| { - // Never return top-level path here as it would break `--skip` - // logic on rustc's internal test framework which is utilized - // by compiletest. - if cfg!(windows) { - PathBuf::from(p.to_str().unwrap().replace('/', "\\")) - } else { - p - } - }) - .collect(); - #[cfg(feature = "tracing")] span!( target: "CONFIG_HANDLING", @@ -1632,8 +1617,29 @@ impl Config { jobs, compiletest_diff_tool, mut ccache, + exclude, } = toml.build.unwrap_or_default(); + let mut paths: Vec = flags.skip.into_iter().chain(flags.exclude).collect(); + + if let Some(exclude) = exclude { + paths.extend(exclude); + } + + config.skip = paths + .into_iter() + .map(|p| { + // Never return top-level path here as it would break `--skip` + // logic on rustc's internal test framework which is utilized + // by compiletest. + if cfg!(windows) { + PathBuf::from(p.to_str().unwrap().replace('/', "\\")) + } else { + p + } + }) + .collect(); + config.jobs = Some(threads_from_config(flags.jobs.unwrap_or(jobs.unwrap_or(0)))); if let Some(file_build) = build { From d9ceab9bc72e6d26330f060f7ec10de63cc56db3 Mon Sep 17 00:00:00 2001 From: bit-aloo Date: Mon, 17 Feb 2025 01:28:39 +0530 Subject: [PATCH 08/24] add test for exclude feature --- src/bootstrap/src/core/config/tests.rs | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/src/bootstrap/src/core/config/tests.rs b/src/bootstrap/src/core/config/tests.rs index eff5e0337428c..27968bea31869 100644 --- a/src/bootstrap/src/core/config/tests.rs +++ b/src/bootstrap/src/core/config/tests.rs @@ -515,3 +515,17 @@ fn test_explicit_stage() { assert!(!config.explicit_stage_from_config); assert!(!config.is_explicit_stage()); } + +#[test] +fn test_exclude() { + let config = parse("build.exclude=[\"test/codegen\"]"); + + let first_excluded = config + .skip + .first() + .expect("Expected at least one excluded path") + .to_str() + .expect("Failed to convert excluded path to string"); + + assert_eq!(first_excluded, "test/codegen"); +} From 9206960dfadbed4b35d24b1cedbe463aac45aaea Mon Sep 17 00:00:00 2001 From: bit-aloo Date: Mon, 17 Feb 2025 01:34:32 +0530 Subject: [PATCH 09/24] Add change info to change tracker --- src/bootstrap/src/utils/change_tracker.rs | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/bootstrap/src/utils/change_tracker.rs b/src/bootstrap/src/utils/change_tracker.rs index 5f49c50c5ad3a..b9f57898f4d98 100644 --- a/src/bootstrap/src/utils/change_tracker.rs +++ b/src/bootstrap/src/utils/change_tracker.rs @@ -365,4 +365,9 @@ pub const CONFIG_CHANGE_HISTORY: &[ChangeInfo] = &[ severity: ChangeSeverity::Info, summary: "`rust.channel` now supports \"auto-detect\" to load the channel from `src/ci/channel`", }, + ChangeInfo { + change_id: 137147, + severity: ChangeSeverity::Info, + summary: "New option `build.exclude` that adds support for excluding test.", + }, ]; From 3998690a68d0346bc31870595e7ca6cf8540a989 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E8=AE=B8=E6=9D=B0=E5=8F=8B=20Jieyou=20Xu=20=28Joe=29?= <39484203+jieyouxu@users.noreply.github.com> Date: Wed, 5 Feb 2025 21:15:04 +0800 Subject: [PATCH 10/24] compiletest: remove legacy `Makefile`-based `run-make` support --- src/tools/compiletest/src/header.rs | 16 +- src/tools/compiletest/src/header/tests.rs | 9 - src/tools/compiletest/src/lib.rs | 43 ++--- src/tools/compiletest/src/runtest/run_make.rs | 164 ------------------ 4 files changed, 18 insertions(+), 214 deletions(-) diff --git a/src/tools/compiletest/src/header.rs b/src/tools/compiletest/src/header.rs index 53ee901b8bc48..7675e13990d6c 100644 --- a/src/tools/compiletest/src/header.rs +++ b/src/tools/compiletest/src/header.rs @@ -709,11 +709,11 @@ impl TestProps { /// returns a struct containing various parts of the directive. fn line_directive<'line>( line_number: usize, - comment: &str, original_line: &'line str, ) -> Option> { // Ignore lines that don't start with the comment prefix. - let after_comment = original_line.trim_start().strip_prefix(comment)?.trim_start(); + let after_comment = + original_line.trim_start().strip_prefix(COMPILETEST_DIRECTIVE_PREFIX)?.trim_start(); let revision; let raw_directive; @@ -722,7 +722,7 @@ fn line_directive<'line>( // A comment like `//@[foo]` only applies to revision `foo`. let Some((line_revision, after_close_bracket)) = after_open_bracket.split_once(']') else { panic!( - "malformed condition directive: expected `{comment}[foo]`, found `{original_line}`" + "malformed condition directive: expected `{COMPILETEST_DIRECTIVE_PREFIX}[foo]`, found `{original_line}`" ) }; @@ -836,6 +836,8 @@ pub(crate) fn check_directive<'a>( CheckDirectiveResult { is_known_directive: is_known(&directive_name), trailing_directive } } +const COMPILETEST_DIRECTIVE_PREFIX: &str = "//@"; + fn iter_header( mode: Mode, _suite: &str, @@ -849,8 +851,7 @@ fn iter_header( } // Coverage tests in coverage-run mode always have these extra directives, without needing to - // specify them manually in every test file. (Some of the comments below have been copied over - // from the old `tests/run-make/coverage-reports/Makefile`, which no longer exists.) + // specify them manually in every test file. // // FIXME(jieyouxu): I feel like there's a better way to do this, leaving for later. if mode == Mode::CoverageRun { @@ -867,9 +868,6 @@ fn iter_header( } } - // NOTE(jieyouxu): once we get rid of `Makefile`s we can unconditionally check for `//@`. - let comment = if testfile.extension().is_some_and(|e| e == "rs") { "//@" } else { "#" }; - let mut rdr = BufReader::with_capacity(1024, rdr); let mut ln = String::new(); let mut line_number = 0; @@ -882,7 +880,7 @@ fn iter_header( } let ln = ln.trim(); - let Some(directive_line) = line_directive(line_number, comment, ln) else { + let Some(directive_line) = line_directive(line_number, ln) else { continue; }; diff --git a/src/tools/compiletest/src/header/tests.rs b/src/tools/compiletest/src/header/tests.rs index d7079fdeee6c3..007318be7cc39 100644 --- a/src/tools/compiletest/src/header/tests.rs +++ b/src/tools/compiletest/src/header/tests.rs @@ -239,11 +239,6 @@ fn check_ignore(config: &Config, contents: &str) -> bool { d.ignore } -fn parse_makefile(config: &Config, contents: &str) -> EarlyProps { - let bytes = contents.as_bytes(); - EarlyProps::from_reader(config, Path::new("Makefile"), bytes) -} - #[test] fn should_fail() { let config: Config = cfg().build(); @@ -261,10 +256,6 @@ fn revisions() { let config: Config = cfg().build(); assert_eq!(parse_rs(&config, "//@ revisions: a b c").revisions, vec!["a", "b", "c"],); - assert_eq!( - parse_makefile(&config, "# revisions: hello there").revisions, - vec!["hello", "there"], - ); } #[test] diff --git a/src/tools/compiletest/src/lib.rs b/src/tools/compiletest/src/lib.rs index 9dff7047bc4ab..30b8e269e6254 100644 --- a/src/tools/compiletest/src/lib.rs +++ b/src/tools/compiletest/src/lib.rs @@ -21,7 +21,7 @@ pub mod util; use core::panic; use std::collections::HashSet; -use std::ffi::{OsStr, OsString}; +use std::ffi::OsString; use std::io::{self, ErrorKind}; use std::path::{Path, PathBuf}; use std::process::{Command, Stdio}; @@ -268,12 +268,8 @@ pub fn parse_config(args: Vec) -> Config { let path = Path::new(f); let mut iter = path.iter().skip(1); - // We skip the test folder and check if the user passed `rmake.rs` or `Makefile`. - if iter - .next() - .is_some_and(|s| s == OsStr::new("rmake.rs") || s == OsStr::new("Makefile")) - && iter.next().is_none() - { + // We skip the test folder and check if the user passed `rmake.rs`. + if iter.next().is_some_and(|s| s == "rmake.rs") && iter.next().is_none() { path.parent().unwrap().to_str().unwrap().to_string() } else { f.to_string() @@ -776,16 +772,9 @@ fn collect_tests_from_dir( return Ok(()); } - // For run-make tests, a "test file" is actually a directory that contains - // an `rmake.rs` or `Makefile`" + // For run-make tests, a "test file" is actually a directory that contains an `rmake.rs`. if cx.config.mode == Mode::RunMake { - if dir.join("Makefile").exists() && dir.join("rmake.rs").exists() { - return Err(io::Error::other( - "run-make tests cannot have both `Makefile` and `rmake.rs`", - )); - } - - if dir.join("Makefile").exists() || dir.join("rmake.rs").exists() { + if dir.join("rmake.rs").exists() { let paths = TestPaths { file: dir.to_path_buf(), relative_dir: relative_dir_path.parent().unwrap().to_path_buf(), @@ -854,24 +843,14 @@ pub fn is_test(file_name: &OsString) -> bool { !invalid_prefixes.iter().any(|p| file_name.starts_with(p)) } -/// For a single test file, creates one or more test structures (one per revision) -/// that can be handed over to libtest to run, possibly in parallel. +/// For a single test file, creates one or more test structures (one per revision) that can be +/// handed over to libtest to run, possibly in parallel. fn make_test(cx: &TestCollectorCx, collector: &mut TestCollector, testpaths: &TestPaths) { - // For run-make tests, each "test file" is actually a _directory_ containing - // an `rmake.rs` or `Makefile`. But for the purposes of directive parsing, - // we want to look at that recipe file, not the directory itself. + // For run-make tests, each "test file" is actually a _directory_ containing an `rmake.rs`. But + // for the purposes of directive parsing, we want to look at that recipe file, not the directory + // itself. let test_path = if cx.config.mode == Mode::RunMake { - if testpaths.file.join("rmake.rs").exists() && testpaths.file.join("Makefile").exists() { - panic!("run-make tests cannot have both `rmake.rs` and `Makefile`"); - } - - if testpaths.file.join("rmake.rs").exists() { - // Parse directives in rmake.rs. - testpaths.file.join("rmake.rs") - } else { - // Parse directives in the Makefile. - testpaths.file.join("Makefile") - } + testpaths.file.join("rmake.rs") } else { PathBuf::from(&testpaths.file) }; diff --git a/src/tools/compiletest/src/runtest/run_make.rs b/src/tools/compiletest/src/runtest/run_make.rs index 9bb3993223e21..b237fac38f6b7 100644 --- a/src/tools/compiletest/src/runtest/run_make.rs +++ b/src/tools/compiletest/src/runtest/run_make.rs @@ -9,168 +9,6 @@ use crate::util::{copy_dir_all, dylib_env_var}; impl TestCx<'_> { pub(super) fn run_rmake_test(&self) { - let test_dir = &self.testpaths.file; - if test_dir.join("rmake.rs").exists() { - self.run_rmake_v2_test(); - } else if test_dir.join("Makefile").exists() { - self.run_rmake_legacy_test(); - } else { - self.fatal("failed to find either `rmake.rs` or `Makefile`") - } - } - - fn run_rmake_legacy_test(&self) { - let cwd = env::current_dir().unwrap(); - - // FIXME(Zalathar): This should probably be `output_base_dir` to avoid - // an unnecessary extra subdirectory, but since legacy Makefile tests - // are hopefully going away, it seems safer to leave this perilous code - // as-is until it can all be deleted. - let tmpdir = cwd.join(self.output_base_name()); - ignore_not_found(|| recursive_remove(&tmpdir)).unwrap(); - - fs::create_dir_all(&tmpdir).unwrap(); - - let host = &self.config.host; - let make = if host.contains("dragonfly") - || host.contains("freebsd") - || host.contains("netbsd") - || host.contains("openbsd") - || host.contains("aix") - { - "gmake" - } else { - "make" - }; - - let mut cmd = Command::new(make); - cmd.current_dir(&self.testpaths.file) - .stdout(Stdio::piped()) - .stderr(Stdio::piped()) - .env("TARGET", &self.config.target) - .env("PYTHON", &self.config.python) - .env("S", &self.config.src_root) - .env("RUST_BUILD_STAGE", &self.config.stage_id) - .env("RUSTC", cwd.join(&self.config.rustc_path)) - .env("TMPDIR", &tmpdir) - .env("LD_LIB_PATH_ENVVAR", dylib_env_var()) - .env("HOST_RPATH_DIR", cwd.join(&self.config.compile_lib_path)) - .env("TARGET_RPATH_DIR", cwd.join(&self.config.run_lib_path)) - .env("LLVM_COMPONENTS", &self.config.llvm_components) - // We for sure don't want these tests to run in parallel, so make - // sure they don't have access to these vars if we run via `make` - // at the top level - .env_remove("MAKEFLAGS") - .env_remove("MFLAGS") - .env_remove("CARGO_MAKEFLAGS"); - - if let Some(ref cargo) = self.config.cargo_path { - cmd.env("CARGO", cwd.join(cargo)); - } - - if let Some(ref rustdoc) = self.config.rustdoc_path { - cmd.env("RUSTDOC", cwd.join(rustdoc)); - } - - if let Some(ref node) = self.config.nodejs { - cmd.env("NODE", node); - } - - if let Some(ref linker) = self.config.target_linker { - cmd.env("RUSTC_LINKER", linker); - } - - if let Some(ref clang) = self.config.run_clang_based_tests_with { - cmd.env("CLANG", clang); - } - - if let Some(ref filecheck) = self.config.llvm_filecheck { - cmd.env("LLVM_FILECHECK", filecheck); - } - - if let Some(ref llvm_bin_dir) = self.config.llvm_bin_dir { - cmd.env("LLVM_BIN_DIR", llvm_bin_dir); - } - - if let Some(ref remote_test_client) = self.config.remote_test_client { - cmd.env("REMOTE_TEST_CLIENT", remote_test_client); - } - - // We don't want RUSTFLAGS set from the outside to interfere with - // compiler flags set in the test cases: - cmd.env_remove("RUSTFLAGS"); - - // Use dynamic musl for tests because static doesn't allow creating dylibs - if self.config.host.contains("musl") { - cmd.env("RUSTFLAGS", "-Ctarget-feature=-crt-static").env("IS_MUSL_HOST", "1"); - } - - if self.config.bless { - cmd.env("RUSTC_BLESS_TEST", "--bless"); - // Assume this option is active if the environment variable is "defined", with _any_ value. - // As an example, a `Makefile` can use this option by: - // - // ifdef RUSTC_BLESS_TEST - // cp "$(TMPDIR)"/actual_something.ext expected_something.ext - // else - // $(DIFF) expected_something.ext "$(TMPDIR)"/actual_something.ext - // endif - } - - if self.config.target.contains("msvc") && !self.config.cc.is_empty() { - // We need to pass a path to `lib.exe`, so assume that `cc` is `cl.exe` - // and that `lib.exe` lives next to it. - let lib = Path::new(&self.config.cc).parent().unwrap().join("lib.exe"); - - // MSYS doesn't like passing flags of the form `/foo` as it thinks it's - // a path and instead passes `C:\msys64\foo`, so convert all - // `/`-arguments to MSVC here to `-` arguments. - let cflags = self - .config - .cflags - .split(' ') - .map(|s| s.replace("/", "-")) - .collect::>() - .join(" "); - let cxxflags = self - .config - .cxxflags - .split(' ') - .map(|s| s.replace("/", "-")) - .collect::>() - .join(" "); - - cmd.env("IS_MSVC", "1") - .env("IS_WINDOWS", "1") - .env("MSVC_LIB", format!("'{}' -nologo", lib.display())) - .env("MSVC_LIB_PATH", format!("{}", lib.display())) - .env("CC", format!("'{}' {}", self.config.cc, cflags)) - .env("CXX", format!("'{}' {}", &self.config.cxx, cxxflags)); - } else { - cmd.env("CC", format!("{} {}", self.config.cc, self.config.cflags)) - .env("CXX", format!("{} {}", self.config.cxx, self.config.cxxflags)) - .env("AR", &self.config.ar); - - if self.config.target.contains("windows") { - cmd.env("IS_WINDOWS", "1"); - } - } - - let (output, truncated) = - self.read2_abbreviated(cmd.spawn().expect("failed to spawn `make`")); - if !output.status.success() { - let res = ProcRes { - status: output.status, - stdout: String::from_utf8_lossy(&output.stdout).into_owned(), - stderr: String::from_utf8_lossy(&output.stderr).into_owned(), - truncated, - cmdline: format!("{:?}", cmd), - }; - self.fatal_proc_rec("make failed", &res); - } - } - - fn run_rmake_v2_test(&self) { // For `run-make` V2, we need to perform 2 steps to build and run a `run-make` V2 recipe // (`rmake.rs`) to run the actual tests. The support library is already built as a tool rust // library and is available under `build/$TARGET/stageN-tools-bin/librun_make_support.rlib`. @@ -191,8 +29,6 @@ impl TestCx<'_> { // recipes to `remove_dir_all($TMPDIR)` without running into issues related trying to remove // a currently running executable because the recipe executable is not under the // `rmake_out/` directory. - // - // This setup intentionally diverges from legacy Makefile run-make tests. let base_dir = self.output_base_dir(); ignore_not_found(|| recursive_remove(&base_dir)).unwrap(); From 413b824e5b41180b1503a081daf8f23927678add Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E8=AE=B8=E6=9D=B0=E5=8F=8B=20Jieyou=20Xu=20=28Joe=29?= <39484203+jieyouxu@users.noreply.github.com> Date: Wed, 5 Feb 2025 21:43:20 +0800 Subject: [PATCH 11/24] run-make: remove `tools.mk` It has served us well, but it's time to retire the `Makefile` support file since this is no longer needed. --- tests/run-make/tools.mk | 209 ---------------------------------------- 1 file changed, 209 deletions(-) delete mode 100644 tests/run-make/tools.mk diff --git a/tests/run-make/tools.mk b/tests/run-make/tools.mk deleted file mode 100644 index b1e872a202af5..0000000000000 --- a/tests/run-make/tools.mk +++ /dev/null @@ -1,209 +0,0 @@ -# These deliberately use `=` and not `:=` so that client makefiles can -# augment HOST_RPATH_DIR / TARGET_RPATH_DIR. -HOST_RPATH_ENV = \ - $(LD_LIB_PATH_ENVVAR)="$(TMPDIR):$(HOST_RPATH_DIR):$($(LD_LIB_PATH_ENVVAR))" -TARGET_RPATH_ENV = \ - $(LD_LIB_PATH_ENVVAR)="$(TMPDIR):$(TARGET_RPATH_DIR):$($(LD_LIB_PATH_ENVVAR))" - -RUSTC_ORIGINAL := $(RUSTC) -BARE_RUSTC := $(HOST_RPATH_ENV) '$(RUSTC)' -BARE_RUSTDOC := $(HOST_RPATH_ENV) '$(RUSTDOC)' -RUSTC := $(BARE_RUSTC) --out-dir $(TMPDIR) -L $(TMPDIR) $(RUSTFLAGS) -Ainternal_features -RUSTDOC := $(BARE_RUSTDOC) -L $(TARGET_RPATH_DIR) -ifdef RUSTC_LINKER -RUSTC := $(RUSTC) -Clinker='$(RUSTC_LINKER)' -RUSTDOC := $(RUSTDOC) -Clinker='$(RUSTC_LINKER)' -endif -#CC := $(CC) -L $(TMPDIR) -HTMLDOCCK := '$(PYTHON)' '$(S)/src/etc/htmldocck.py' -CGREP := "$(S)/src/etc/cat-and-grep.sh" - -# diff with common flags for multi-platform diffs against text output -DIFF := diff -u --strip-trailing-cr - -# With RUSTC_TEST_OP you can elegantly support blessing of run-make tests. Do -# like this in a Makefile recipe: -# -# "$(TMPDIR)"/your-test > "$(TMPDIR)"/your-test.run.stdout -# $(RUSTC_TEST_OP) "$(TMPDIR)"/your-test.run.stdout your-test.run.stdout -# -# When running the test normally with -# -# ./x test tests/run-make/your-test -# -# the actual output will be diffed against the expected output. When running in -# bless-mode with -# -# ./x test --bless tests/run-make/your-test -# -# the actual output will be blessed as the expected output. -ifdef RUSTC_BLESS_TEST - RUSTC_TEST_OP = cp -else - RUSTC_TEST_OP = $(DIFF) -endif - -# Some of the Rust CI platforms use `/bin/dash` to run `shell` script in -# Makefiles. Other platforms, including many developer platforms, default to -# `/bin/bash`. (In many cases, `make` is actually using `/bin/sh`, but `sh` -# is configured to execute one or the other shell binary). `dash` features -# support only a small subset of `bash` features, so `dash` can be thought of as -# the lowest common denominator, and tests should be validated against `dash` -# whenever possible. Most developer platforms include `/bin/dash`, but to ensure -# tests still work when `/bin/dash`, if not available, this `SHELL` override is -# conditional: -ifndef IS_WINDOWS # dash interprets backslashes in executable paths incorrectly -ifneq (,$(wildcard /bin/dash)) -SHELL := /bin/dash -endif -endif - -# This is the name of the binary we will generate and run; use this -# e.g. for `$(CC) -o $(RUN_BINFILE)`. -RUN_BINFILE = $(TMPDIR)/$(1) - -# Invoke the generated binary on the remote machine if compiletest was -# configured to use a remote test device, otherwise run it on the current host. -ifdef REMOTE_TEST_CLIENT -# FIXME: if a test requires additional files, this will need to be changed to -# also push them (by changing the 0 to the number of additional files, and -# providing the path of the additional files as the last arguments). -EXECUTE = $(REMOTE_TEST_CLIENT) run 0 $(RUN_BINFILE) -else -EXECUTE = $(RUN_BINFILE) -endif - -# RUN and FAIL are basic way we will invoke the generated binary. On -# non-windows platforms, they set the LD_LIBRARY_PATH environment -# variable before running the binary. - -RLIB_GLOB = lib$(1)*.rlib -BIN = $(1) - -UNAME = $(shell uname) - -ifeq ($(UNAME),Darwin) -RUN = $(TARGET_RPATH_ENV) $(EXECUTE) -FAIL = $(TARGET_RPATH_ENV) $(EXECUTE) && exit 1 || exit 0 -DYLIB_GLOB = lib$(1)*.dylib -DYLIB = $(TMPDIR)/lib$(1).dylib -STATICLIB = $(TMPDIR)/lib$(1).a -STATICLIB_GLOB = lib$(1)*.a -else -ifdef IS_WINDOWS -RUN = PATH="$(PATH):$(TARGET_RPATH_DIR)" $(EXECUTE) -FAIL = PATH="$(PATH):$(TARGET_RPATH_DIR)" $(EXECUTE) && exit 1 || exit 0 -DYLIB_GLOB = $(1)*.dll -DYLIB = $(TMPDIR)/$(1).dll -ifdef IS_MSVC -STATICLIB = $(TMPDIR)/$(1).lib -STATICLIB_GLOB = $(1)*.lib -else -IMPLIB = $(TMPDIR)/lib$(1).dll.a -STATICLIB = $(TMPDIR)/lib$(1).a -STATICLIB_GLOB = lib$(1)*.a -endif -BIN = $(1).exe -LLVM_FILECHECK := $(shell cygpath -u "$(LLVM_FILECHECK)") -else -RUN = $(TARGET_RPATH_ENV) $(EXECUTE) -FAIL = $(TARGET_RPATH_ENV) $(EXECUTE) && exit 1 || exit 0 -DYLIB_GLOB = lib$(1)*.so -DYLIB = $(TMPDIR)/lib$(1).so -STATICLIB = $(TMPDIR)/lib$(1).a -STATICLIB_GLOB = lib$(1)*.a -endif -endif - -ifdef IS_MSVC -COMPILE_OBJ = $(CC) -c -Fo:`cygpath -w $(1)` $(2) -COMPILE_OBJ_CXX = $(CXX) -EHs -c -Fo:`cygpath -w $(1)` $(2) -NATIVE_STATICLIB_FILE = $(1).lib -NATIVE_STATICLIB = $(TMPDIR)/$(call NATIVE_STATICLIB_FILE,$(1)) -OUT_EXE=-Fe:`cygpath -w $(TMPDIR)/$(call BIN,$(1))` \ - -Fo:`cygpath -w $(TMPDIR)/$(1).obj` -else -COMPILE_OBJ = $(CC) -v -c -o $(1) $(2) -COMPILE_OBJ_CXX = $(CXX) -c -o $(1) $(2) -NATIVE_STATICLIB_FILE = lib$(1).a -NATIVE_STATICLIB = $(call STATICLIB,$(1)) -OUT_EXE=-o $(TMPDIR)/$(1) -endif - - -# Extra flags needed to compile a working executable with the standard library -ifdef IS_WINDOWS -ifdef IS_MSVC - EXTRACFLAGS := ws2_32.lib userenv.lib advapi32.lib bcrypt.lib ntdll.lib synchronization.lib -else - EXTRACFLAGS := -lws2_32 -luserenv -lbcrypt -lntdll -lsynchronization - EXTRACXXFLAGS := -lstdc++ - # So this is a bit hacky: we can't use the DLL version of libstdc++ because - # it pulls in the DLL version of libgcc, which means that we end up with 2 - # instances of the DW2 unwinding implementation. This is a problem on - # i686-pc-windows-gnu because each module (DLL/EXE) needs to register its - # unwind information with the unwinding implementation, and libstdc++'s - # __cxa_throw won't see the unwinding info we registered with our statically - # linked libgcc. - # - # Now, simply statically linking libstdc++ would fix this problem, except - # that it is compiled with the expectation that pthreads is dynamically - # linked as a DLL and will fail to link with a statically linked libpthread. - # - # So we end up with the following hack: we link use static:-bundle to only - # link the parts of libstdc++ that we actually use, which doesn't include - # the dependency on the pthreads DLL. - EXTRARSCXXFLAGS := -l static:-bundle=stdc++ -endif -else -ifeq ($(UNAME),Darwin) - EXTRACFLAGS := -lresolv - EXTRACXXFLAGS := -lc++ - EXTRARSCXXFLAGS := -lc++ -else -ifeq ($(UNAME),FreeBSD) - EXTRACFLAGS := -lm -lpthread -lgcc_s -else -ifeq ($(UNAME),SunOS) - EXTRACFLAGS := -lm -lpthread -lposix4 -lsocket -lresolv -else -ifeq ($(UNAME),OpenBSD) - EXTRACFLAGS := -lm -lpthread -lc++abi - RUSTC := $(RUSTC) -C linker="$(word 1,$(CC:ccache=))" -else - EXTRACFLAGS := -lm -lrt -ldl -lpthread - EXTRACXXFLAGS := -lstdc++ - EXTRARSCXXFLAGS := -lstdc++ -endif -endif -endif -endif -endif - -REMOVE_DYLIBS = rm $(TMPDIR)/$(call DYLIB_GLOB,$(1)) -REMOVE_RLIBS = rm $(TMPDIR)/$(call RLIB_GLOB,$(1)) - -%.a: %.o - $(AR) crus $@ $< -ifdef IS_MSVC -%.lib: lib%.o - $(MSVC_LIB) -out:`cygpath -w $@` $< -else -%.lib: lib%.o - $(AR) crus $@ $< -endif -%.dylib: %.o - $(CC) -dynamiclib -Wl,-dylib -o $@ $< -%.so: %.o - $(CC) -o $@ $< -shared - -ifdef IS_MSVC -%.dll: lib%.o - $(CC) $< -link -dll -out:`cygpath -w $@` -else -%.dll: lib%.o - $(CC) -o $@ $< -shared -Wl,--out-implib=$@.a -endif - -$(TMPDIR)/lib%.o: %.c - $(call COMPILE_OBJ,$@,$<) From 9b17c98d766f98766494230e65a5ab39c8656d6a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E8=AE=B8=E6=9D=B0=E5=8F=8B=20Jieyou=20Xu=20=28Joe=29?= <39484203+jieyouxu@users.noreply.github.com> Date: Wed, 5 Feb 2025 21:35:25 +0800 Subject: [PATCH 12/24] run-make: update test suite README --- tests/run-make/README.md | 36 +++++++----------------------------- 1 file changed, 7 insertions(+), 29 deletions(-) diff --git a/tests/run-make/README.md b/tests/run-make/README.md index 4035990347389..5cdd2fdc523cf 100644 --- a/tests/run-make/README.md +++ b/tests/run-make/README.md @@ -1,32 +1,17 @@ # The `run-make` test suite -The `run-make` test suite contains tests which are the most flexible out of all -the [rust-lang/rust](https://github.com/rust-lang/rust) test suites. `run-make` -tests can basically contain arbitrary code, and are supported by the -[`run_make_support`] library. +The `run-make` test suite contains tests which are the most flexible out of all the [rust-lang/rust](https://github.com/rust-lang/rust) test suites. `run-make` tests can basically contain arbitrary code, and are supported by the [`run_make_support`] library. ## Infrastructure -There are two kinds of run-make tests: +A `run-make` test is a test recipe source file `rmake.rs` accompanied by its parent directory (e.g. `tests/run-make/foo/rmake.rs` is the `foo` `run-make` test). -1. The new `rmake.rs` version: this allows run-make tests to be written in Rust - (with `rmake.rs` as the main test file). -2. The legacy `Makefile` version: this is what run-make tests were written with - before support for `rmake.rs` was introduced. +The implementation for collecting and building the `rmake.rs` recipes are in [`src/tools/compiletest/src/runtest.rs`](../../src/tools/compiletest/src/runtest.rs), in `run_rmake_test`. -The implementation for collecting and building the `rmake.rs` recipes (or -`Makefile`s) are in -[`src/tools/compiletest/src/runtest.rs`](../../src/tools/compiletest/src/runtest.rs), -in `run_rmake_v2_test` and `run_rmake_legacy_test`. - -### Rust-based `run-make` tests: `rmake.rs` - -The setup for the `rmake.rs` version is a 3-stage process: +The setup for the `rmake.rs` can be summarized as a 3-stage process: 1. First, we build the [`run_make_support`] library in bootstrap as a tool lib. -2. Then, we compile the `rmake.rs` "recipe" linking the support library and its - dependencies in, and provide a bunch of env vars. We setup a directory - structure within `build//test/run-make/` +2. Then, we compile the `rmake.rs` "recipe" linking the support library and its dependencies in, and provide a bunch of env vars. We setup a directory structure within `build//test/run-make/` ``` / @@ -34,15 +19,8 @@ The setup for the `rmake.rs` version is a 3-stage process: rmake_out/ # sources from test sources copied over ``` - and copy non-`rmake.rs` input support files over to `rmake_out/`. The - support library is made available as an [*extern prelude*][extern_prelude]. -3. Finally, we run the recipe binary and set `rmake_out/` as the working - directory. + and copy non-`rmake.rs` input support files over to `rmake_out/`. The support library is made available as an [*extern prelude*][extern_prelude]. +3. Finally, we run the recipe binary and set `rmake_out/` as the working directory. [`run_make_support`]: ../../src/tools/run-make-support [extern_prelude]: https://doc.rust-lang.org/reference/names/preludes.html#extern-prelude - -### Formatting - -Note that files under `tests/` are not formatted by `./x fmt`, -use `rustfmt tests/path/to/file.rs` to format a specific file if desired. From ed168e7b2bd98b7f044938ab88b791cdee73d85a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E8=AE=B8=E6=9D=B0=E5=8F=8B=20Jieyou=20Xu=20=28Joe=29?= <39484203+jieyouxu@users.noreply.github.com> Date: Sun, 2 Mar 2025 05:21:23 +0800 Subject: [PATCH 13/24] run-make-support: remove outdated comments --- .../run-make-support/src/artifact_names.rs | 17 ------ .../src/external_deps/c_cxx_compiler/cc.rs | 11 ---- .../external_deps/c_cxx_compiler/extras.rs | 55 ------------------- .../src/external_deps/rustc.rs | 25 --------- 4 files changed, 108 deletions(-) diff --git a/src/tools/run-make-support/src/artifact_names.rs b/src/tools/run-make-support/src/artifact_names.rs index 0d7b5cb98388e..8968f831542e6 100644 --- a/src/tools/run-make-support/src/artifact_names.rs +++ b/src/tools/run-make-support/src/artifact_names.rs @@ -8,23 +8,6 @@ use crate::targets::is_msvc; /// Construct the static library name based on the target. #[must_use] pub fn static_lib_name(name: &str) -> String { - // See tools.mk (irrelevant lines omitted): - // - // ```makefile - // ifeq ($(UNAME),Darwin) - // STATICLIB = $(TMPDIR)/lib$(1).a - // else - // ifdef IS_WINDOWS - // ifdef IS_MSVC - // STATICLIB = $(TMPDIR)/$(1).lib - // else - // STATICLIB = $(TMPDIR)/lib$(1).a - // endif - // else - // STATICLIB = $(TMPDIR)/lib$(1).a - // endif - // endif - // ``` assert!(!name.contains(char::is_whitespace), "static library name cannot contain whitespace"); if is_msvc() { format!("{name}.lib") } else { format!("lib{name}.a") } diff --git a/src/tools/run-make-support/src/external_deps/c_cxx_compiler/cc.rs b/src/tools/run-make-support/src/external_deps/c_cxx_compiler/cc.rs index becb91ae989cc..0e6d6ea6075d2 100644 --- a/src/tools/run-make-support/src/external_deps/c_cxx_compiler/cc.rs +++ b/src/tools/run-make-support/src/external_deps/c_cxx_compiler/cc.rs @@ -80,17 +80,6 @@ impl Cc { /// Specify `-o` or `-Fe`/`-Fo` depending on platform/compiler. pub fn out_exe(&mut self, name: &str) -> &mut Self { - // Ref: tools.mk (irrelevant lines omitted): - // - // ```makefile - // ifdef IS_MSVC - // OUT_EXE=-Fe:`cygpath -w $(TMPDIR)/$(call BIN,$(1))` \ - // -Fo:`cygpath -w $(TMPDIR)/$(1).obj` - // else - // OUT_EXE=-o $(TMPDIR)/$(1) - // endif - // ``` - let mut path = std::path::PathBuf::from(name); if is_msvc() { diff --git a/src/tools/run-make-support/src/external_deps/c_cxx_compiler/extras.rs b/src/tools/run-make-support/src/external_deps/c_cxx_compiler/extras.rs index 49210d75e063f..ca6ab3275c4d6 100644 --- a/src/tools/run-make-support/src/external_deps/c_cxx_compiler/extras.rs +++ b/src/tools/run-make-support/src/external_deps/c_cxx_compiler/extras.rs @@ -2,36 +2,6 @@ use crate::{is_msvc, is_windows, uname}; /// `EXTRACFLAGS` pub fn extra_c_flags() -> Vec<&'static str> { - // Adapted from tools.mk (trimmed): - // - // ```makefile - // ifdef IS_WINDOWS - // ifdef IS_MSVC - // EXTRACFLAGS := ws2_32.lib userenv.lib advapi32.lib bcrypt.lib ntdll.lib synchronization.lib - // else - // EXTRACFLAGS := -lws2_32 -luserenv -lbcrypt -lntdll -lsynchronization - // endif - // else - // ifeq ($(UNAME),Darwin) - // EXTRACFLAGS := -lresolv - // else - // ifeq ($(UNAME),FreeBSD) - // EXTRACFLAGS := -lm -lpthread -lgcc_s - // else - // ifeq ($(UNAME),SunOS) - // EXTRACFLAGS := -lm -lpthread -lposix4 -lsocket -lresolv - // else - // ifeq ($(UNAME),OpenBSD) - // EXTRACFLAGS := -lm -lpthread -lc++abi - // else - // EXTRACFLAGS := -lm -lrt -ldl -lpthread - // endif - // endif - // endif - // endif - // endif - // ``` - if is_windows() { if is_msvc() { vec![ @@ -60,31 +30,6 @@ pub fn extra_c_flags() -> Vec<&'static str> { /// `EXTRACXXFLAGS` pub fn extra_cxx_flags() -> Vec<&'static str> { - // Adapted from tools.mk (trimmed): - // - // ```makefile - // ifdef IS_WINDOWS - // ifdef IS_MSVC - // else - // EXTRACXXFLAGS := -lstdc++ - // endif - // else - // ifeq ($(UNAME),Darwin) - // EXTRACXXFLAGS := -lc++ - // else - // ifeq ($(UNAME),FreeBSD) - // else - // ifeq ($(UNAME),SunOS) - // else - // ifeq ($(UNAME),OpenBSD) - // else - // EXTRACXXFLAGS := -lstdc++ - // endif - // endif - // endif - // endif - // endif - // ``` if is_windows() { if is_msvc() { vec![] } else { vec!["-lstdc++"] } } else { diff --git a/src/tools/run-make-support/src/external_deps/rustc.rs b/src/tools/run-make-support/src/external_deps/rustc.rs index 710ba025830a8..07db42027f0ab 100644 --- a/src/tools/run-make-support/src/external_deps/rustc.rs +++ b/src/tools/run-make-support/src/external_deps/rustc.rs @@ -365,31 +365,6 @@ impl Rustc { /// `EXTRARSCXXFLAGS` pub fn extra_rs_cxx_flags(&mut self) -> &mut Self { - // Adapted from tools.mk (trimmed): - // - // ```makefile - // ifdef IS_WINDOWS - // ifdef IS_MSVC - // else - // EXTRARSCXXFLAGS := -lstatic:-bundle=stdc++ - // endif - // else - // ifeq ($(UNAME),Darwin) - // EXTRARSCXXFLAGS := -lc++ - // else - // ifeq ($(UNAME),FreeBSD) - // else - // ifeq ($(UNAME),SunOS) - // else - // ifeq ($(UNAME),OpenBSD) - // else - // EXTRARSCXXFLAGS := -lstdc++ - // endif - // endif - // endif - // endif - // endif - // ``` if is_windows() { // So this is a bit hacky: we can't use the DLL version of libstdc++ because // it pulls in the DLL version of libgcc, which means that we end up with 2 From efec638308362313fe8c7541cb8f10e6b9a7fe4d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E8=AE=B8=E6=9D=B0=E5=8F=8B=20Jieyou=20Xu=20=28Joe=29?= <39484203+jieyouxu@users.noreply.github.com> Date: Wed, 5 Feb 2025 21:48:48 +0800 Subject: [PATCH 14/24] tidy: remove legacy `Makefile` checks --- .../tidy/src/allowed_run_make_makefiles.txt | 0 src/tools/tidy/src/lib.rs | 1 - src/tools/tidy/src/main.rs | 2 - src/tools/tidy/src/run_make_tests.rs | 104 ------------------ 4 files changed, 107 deletions(-) delete mode 100644 src/tools/tidy/src/allowed_run_make_makefiles.txt delete mode 100644 src/tools/tidy/src/run_make_tests.rs diff --git a/src/tools/tidy/src/allowed_run_make_makefiles.txt b/src/tools/tidy/src/allowed_run_make_makefiles.txt deleted file mode 100644 index e69de29bb2d1d..0000000000000 diff --git a/src/tools/tidy/src/lib.rs b/src/tools/tidy/src/lib.rs index 1e7eb82b83e98..9f6d563166e2b 100644 --- a/src/tools/tidy/src/lib.rs +++ b/src/tools/tidy/src/lib.rs @@ -79,7 +79,6 @@ pub(crate) mod iter_header; pub mod known_bug; pub mod mir_opt_tests; pub mod pal; -pub mod run_make_tests; pub mod rustdoc_css_themes; pub mod rustdoc_gui_tests; pub mod rustdoc_templates; diff --git a/src/tools/tidy/src/main.rs b/src/tools/tidy/src/main.rs index 13a558fea48ea..1d8514ef4c9c4 100644 --- a/src/tools/tidy/src/main.rs +++ b/src/tools/tidy/src/main.rs @@ -103,8 +103,6 @@ fn main() { check!(tests_revision_unpaired_stdout_stderr, &tests_path); check!(debug_artifacts, &tests_path); check!(ui_tests, &root_path, bless); - // FIXME(jieyouxu): remove this check once all run-make tests are ported over to rmake.rs. - check!(run_make_tests, &tests_path, &src_path, bless); check!(mir_opt_tests, &tests_path, bless); check!(rustdoc_gui_tests, &tests_path); check!(rustdoc_css_themes, &librustdoc_path); diff --git a/src/tools/tidy/src/run_make_tests.rs b/src/tools/tidy/src/run_make_tests.rs deleted file mode 100644 index 8d1767878378c..0000000000000 --- a/src/tools/tidy/src/run_make_tests.rs +++ /dev/null @@ -1,104 +0,0 @@ -//! Tidy check to ensure that no new Makefiles are added under `tests/run-make/`. - -use std::collections::BTreeSet; -use std::fs::File; -use std::io::Write; -use std::path::{Path, PathBuf}; - -pub fn check(tests_path: &Path, src_path: &Path, bless: bool, bad: &mut bool) { - let mut is_sorted = true; - - let allowed_makefiles = { - let mut total_lines = 0; - let mut prev_line = ""; - let allowed_makefiles: BTreeSet<&str> = include_str!("allowed_run_make_makefiles.txt") - .lines() - .map(|line| { - total_lines += 1; - - if prev_line > line { - is_sorted = false; - } - - prev_line = line; - - line - }) - .collect(); - - if !is_sorted && !bless { - tidy_error!( - bad, - "`src/tools/tidy/src/allowed_run_make_makefiles.txt` is not in order, likely \ - because you modified it manually, please only update it with command \ - `x test tidy --bless`" - ); - } - if allowed_makefiles.len() != total_lines { - tidy_error!( - bad, - "`src/tools/tidy/src/allowed_run_make_makefiles.txt` contains duplicate entries, \ - likely because you modified it manually, please only update it with command \ - `x test tidy --bless`" - ); - } - - allowed_makefiles - }; - - let mut remaining_makefiles = allowed_makefiles.clone(); - - crate::walk::walk_no_read( - &[tests_path.join("run-make").as_ref()], - |_, _| false, - &mut |entry| { - if entry.file_type().map_or(true, |t| t.is_dir()) { - return; - } - - if entry.file_name().to_str().map_or(true, |f| f != "Makefile") { - return; - } - - let makefile_path = entry.path().strip_prefix(&tests_path).unwrap(); - let makefile_path = makefile_path.to_str().unwrap().replace('\\', "/"); - - if !remaining_makefiles.remove(makefile_path.as_str()) { - tidy_error!( - bad, - "found run-make Makefile not permitted in \ - `src/tools/tidy/src/allowed_run_make_makefiles.txt`, please write new run-make \ - tests with `rmake.rs` instead: {}", - entry.path().display() - ); - } - }, - ); - - // If there are any expected Makefiles remaining, they were moved or deleted. - // Our data must remain up to date, so they must be removed from - // `src/tools/tidy/src/allowed_run_make_makefiles.txt`. - // This can be done automatically on --bless, or else a tidy error will be issued. - if bless && (!remaining_makefiles.is_empty() || !is_sorted) { - let tidy_src = src_path.join("tools").join("tidy").join("src"); - let org_file_path = tidy_src.join("allowed_run_make_makefiles.txt"); - let temp_file_path = tidy_src.join("blessed_allowed_run_make_makefiles.txt"); - let mut temp_file = t!(File::create_new(&temp_file_path)); - for file in allowed_makefiles.difference(&remaining_makefiles) { - t!(writeln!(temp_file, "{file}")); - } - t!(std::fs::rename(&temp_file_path, &org_file_path)); - } else { - for file in remaining_makefiles { - let mut p = PathBuf::from(tests_path); - p.push(file); - tidy_error!( - bad, - "Makefile `{}` no longer exists and should be removed from the exclusions in \ - `src/tools/tidy/src/allowed_run_make_makefiles.txt`, you can run `x test tidy --bless` to update \ - the allow list", - p.display() - ); - } - } -} From b0d6a8426caf94ed0f822208a55270776149a576 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E8=AE=B8=E6=9D=B0=E5=8F=8B=20Jieyou=20Xu=20=28Joe=29?= <39484203+jieyouxu@users.noreply.github.com> Date: Wed, 5 Feb 2025 21:40:57 +0800 Subject: [PATCH 15/24] rustc-dev-guide: remove mentions of legacy `Makefile` run-make infra And remove outdated requirements to run `run-make` tests on Windows. --- .../rustc-dev-guide/src/tests/compiletest.md | 30 +------------------ .../rustc-dev-guide/src/tests/directives.md | 7 +---- src/doc/rustc-dev-guide/src/tests/running.md | 25 ---------------- 3 files changed, 2 insertions(+), 60 deletions(-) diff --git a/src/doc/rustc-dev-guide/src/tests/compiletest.md b/src/doc/rustc-dev-guide/src/tests/compiletest.md index 459c082906eba..2905e470fabd3 100644 --- a/src/doc/rustc-dev-guide/src/tests/compiletest.md +++ b/src/doc/rustc-dev-guide/src/tests/compiletest.md @@ -74,8 +74,7 @@ The following test suites are available, with links for more information: ### General purpose test suite -[`run-make`](#run-make-tests) are general purpose tests using Rust programs (or -Makefiles (legacy)). +[`run-make`](#run-make-tests) are general purpose tests using Rust programs. ### Rustdoc test suites @@ -396,14 +395,6 @@ your test, causing separate files to be generated for 32bit and 64bit systems. ### `run-make` tests -> **Note on phasing out `Makefile`s** -> -> We are planning to migrate all existing Makefile-based `run-make` tests -> to Rust programs. You should not be adding new Makefile-based `run-make` -> tests. -> -> See . - The tests in [`tests/run-make`] are general-purpose tests using Rust *recipes*, which are small programs (`rmake.rs`) allowing arbitrary Rust code such as `rustc` invocations, and is supported by a [`run_make_support`] library. Using @@ -424,11 +415,6 @@ Compiletest directives like `//@ only-` or `//@ ignore-` are supported in `rmake.rs`, like in UI tests. However, revisions or building auxiliary via directives are not currently supported. -Two `run-make` tests are ported over to Rust recipes as examples: - -- -- - #### Quickly check if `rmake.rs` tests can be compiled You can quickly check if `rmake.rs` tests can be compiled without having to @@ -481,20 +467,6 @@ Then add a corresponding entry to `"rust-analyzer.linkedProjects"` ], ``` -#### Using Makefiles (legacy) - -
-You should avoid writing new Makefile-based `run-make` tests. -
- -Each test should be in a separate directory with a `Makefile` indicating the -commands to run. - -There is a [`tools.mk`] Makefile which you can include which provides a bunch of -utilities to make it easier to run commands and compare outputs. Take a look at -some of the other tests for some examples on how to get started. - -[`tools.mk`]: https://github.com/rust-lang/rust/blob/master/tests/run-make/tools.mk [`tests/run-make`]: https://github.com/rust-lang/rust/tree/master/tests/run-make [`run_make_support`]: https://github.com/rust-lang/rust/tree/master/src/tools/run-make-support diff --git a/src/doc/rustc-dev-guide/src/tests/directives.md b/src/doc/rustc-dev-guide/src/tests/directives.md index 00bb2bc4dbb1e..c72c7d7d87eb2 100644 --- a/src/doc/rustc-dev-guide/src/tests/directives.md +++ b/src/doc/rustc-dev-guide/src/tests/directives.md @@ -6,10 +6,7 @@ FIXME(jieyouxu) completely revise this chapter. --> -Directives are special comments that tell compiletest how to build and interpret -a test. They must appear before the Rust source in the test. They may also -appear in `rmake.rs` or legacy Makefiles for [run-make -tests](compiletest.md#run-make-tests). +Directives are special comments that tell compiletest how to build and interpret a test. They must appear before the Rust source in the test. They may also appear in `rmake.rs` [run-make tests](compiletest.md#run-make-tests). They are normally put after the short comment that explains the point of this test. Compiletest test suites use `//@` to signal that a comment is a directive. @@ -221,8 +218,6 @@ The following directives will check LLVM support: [`aarch64-gnu-debug`]), which only runs a subset of `run-make` tests. Other tests with this directive will not run at all, which is usually not what you want. - - Notably, the [`aarch64-gnu-debug`] CI job *currently* only runs `run-make` - tests which additionally contain `clang` in their test name. See also [Debuginfo tests](compiletest.md#debuginfo-tests) for directives for ignoring debuggers. diff --git a/src/doc/rustc-dev-guide/src/tests/running.md b/src/doc/rustc-dev-guide/src/tests/running.md index 6ce65092389bd..9ddf0afee0c77 100644 --- a/src/doc/rustc-dev-guide/src/tests/running.md +++ b/src/doc/rustc-dev-guide/src/tests/running.md @@ -238,30 +238,6 @@ This is much faster, but doesn't always work. For example, some tests include directives that specify specific compiler flags, or which rely on other crates, and they may not run the same without those options. -## Running `run-make` tests - -### Windows - -Running the `run-make` test suite on Windows is a currently bit more involved. -There are numerous prerequisites and environmental requirements: - -- Install msys2: -- Specify `MSYS2_PATH_TYPE=inherit` in `msys2.ini` in the msys2 installation directory, run the - following with `MSYS2 MSYS`: - - `pacman -Syuu` - - `pacman -S make` - - `pacman -S diffutils` - - `pacman -S binutils` - - `./x test run-make` (`./x test tests/run-make` doesn't work) - -There is [on-going work][port-run-make] to not rely on `Makefile`s in the -run-make test suite. Once this work is completed, you can run the entire -`run-make` test suite on native Windows inside `cmd` or `PowerShell` without -needing to install and use MSYS2. As of Oct 2024, it is -already possible to run the vast majority of the `run-make` test suite outside -of MSYS2, but there will be failures for the tests that still use `Makefile`s -due to not finding `make`. - ## Running tests on a remote machine Tests may be run on a remote machine (e.g. to test builds for a different @@ -406,4 +382,3 @@ If you encounter bugs or problems, don't hesitate to open issues on the repository](https://github.com/rust-lang/rustc_codegen_gcc/). [`tests/ui`]: https://github.com/rust-lang/rust/tree/master/tests/ui -[port-run-make]: https://github.com/rust-lang/rust/issues/121876 From 95b030f671bdc9652626a7a8490f42027973dfac Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E8=AE=B8=E6=9D=B0=E5=8F=8B=20Jieyou=20Xu=20=28Joe=29?= <39484203+jieyouxu@users.noreply.github.com> Date: Wed, 5 Feb 2025 21:52:30 +0800 Subject: [PATCH 16/24] triagebot: stop backlinking to the test porting tracking issue No longer needed. --- triagebot.toml | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/triagebot.toml b/triagebot.toml index f9eb09ff34381..31683bc67e8de 100644 --- a/triagebot.toml +++ b/triagebot.toml @@ -834,13 +834,7 @@ message = "Some changes occurred in GUI tests." cc = ["@GuillaumeGomez"] [mentions."tests/run-make/"] -message = """ -This PR modifies `tests/run-make/`. If this PR is trying to port a Makefile -run-make test to use rmake.rs, please update the -[run-make port tracking issue](https://github.com/rust-lang/rust/issues/121876) -so we can track our progress. You can either modify the tracking issue -directly, or you can comment on the tracking issue and link this PR. -""" +message = "This PR modifies `run-make` tests." cc = ["@jieyouxu"] [mentions."src/rustdoc-json-types"] From a0ed304c2101b4efdaf9982189e8d6ca73690712 Mon Sep 17 00:00:00 2001 From: Trevor Gross Date: Mon, 9 Dec 2024 09:25:21 +0000 Subject: [PATCH 17/24] 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 a200fd5318669..de1557ccc9028 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 de63a462b61ac..65b5f3b9af093 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 5a2da96a44abad6be752c7ee1cac173aa1ca2f7c Mon Sep 17 00:00:00 2001 From: Trevor Gross Date: Mon, 9 Dec 2024 09:25:22 +0000 Subject: [PATCH 18/24] 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 b37724ba62d5e..a84e11ec8e710 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(super) 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(super) 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(super) const MAX_DIGITS_WITHOUT_OVERFLOW: usize = 19; pub(super) 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(super) 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(super) fn trim(&mut self) { // All of the following calls to `Decimal::trim` can't panic because: // @@ -86,7 +90,7 @@ impl Decimal { pub(super) 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 49a2d4c757e27f915d15664ba92bbe0d3f590a8e Mon Sep 17 00:00:00 2001 From: Trevor Gross Date: Mon, 9 Dec 2024 09:25:22 +0000 Subject: [PATCH 19/24] 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} | 51 ++++++++++++------- library/core/src/num/dec2flt/mod.rs | 2 +- library/core/src/num/dec2flt/slow.rs | 8 +-- .../tests/num/dec2flt/decimal_seq.rs | 30 +++++++++++ library/coretests/tests/num/dec2flt/mod.rs | 1 + 5 files changed, 70 insertions(+), 22 deletions(-) rename library/core/src/num/dec2flt/{decimal.rs => decimal_seq.rs} (94%) create mode 100644 library/coretests/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 a84e11ec8e710..de22280c001c9 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(super) 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(super) 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. @@ -55,7 +55,8 @@ impl Decimal { /// /// In Python: /// `-emin + p2 + math.floor((emin+ 1)*math.log(2, b)-math.log(1-2**(-p2), b))` - pub(super) const MAX_DIGITS: usize = 768; + pub const MAX_DIGITS: usize = 768; + /// The max decimal digits that can be exactly represented in a 64-bit integer. pub(super) const MAX_DIGITS_WITHOUT_OVERFLOW: usize = 19; pub(super) const DECIMAL_POINT_RANGE: i32 = 2047; @@ -73,12 +74,12 @@ impl Decimal { /// Trim trailing zeros from the buffer. // FIXME(tgross35): this could be `.rev().position()` if perf is okay - pub(super) fn trim(&mut self) { - // All of the following calls to `Decimal::trim` can't panic because: + pub fn trim(&mut self) { + // 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 +94,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 +129,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 +143,7 @@ impl Decimal { } n = quotient; } + while n > 0 { write_index -= 1; let quotient = n / 10; @@ -147,10 +155,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 +217,8 @@ impl Decimal { } /// Parse a big integer representation of the float as a decimal. -pub(super) 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 +236,7 @@ pub(super) 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 +248,7 @@ pub(super) 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 +262,12 @@ pub(super) 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 +289,15 @@ pub(super) 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 +362,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 +374,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/coretests/tests/num/dec2flt/decimal_seq.rs b/library/coretests/tests/num/dec2flt/decimal_seq.rs new file mode 100644 index 0000000000000..f46ba7c465a6e --- /dev/null +++ b/library/coretests/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/coretests/tests/num/dec2flt/mod.rs b/library/coretests/tests/num/dec2flt/mod.rs index 874e0ec7093c7..51f3017ddc79c 100644 --- a/library/coretests/tests/num/dec2flt/mod.rs +++ b/library/coretests/tests/num/dec2flt/mod.rs @@ -1,5 +1,6 @@ #![allow(overflowing_literals)] +mod decimal_seq; mod float; mod lemire; mod parse; From 626d2c5eed687e2c166d25f29cdfcaaba48f5f23 Mon Sep 17 00:00:00 2001 From: Trevor Gross Date: Mon, 9 Dec 2024 09:25:22 +0000 Subject: [PATCH 20/24] 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 +++---- .../coretests/tests/num/dec2flt/decimal.rs | 28 +++++++++++++++++++ library/coretests/tests/num/dec2flt/mod.rs | 1 + library/coretests/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/coretests/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/coretests/tests/num/dec2flt/decimal.rs b/library/coretests/tests/num/dec2flt/decimal.rs new file mode 100644 index 0000000000000..1fa06de692e07 --- /dev/null +++ b/library/coretests/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/coretests/tests/num/dec2flt/mod.rs b/library/coretests/tests/num/dec2flt/mod.rs index 51f3017ddc79c..a9025be5ca7f1 100644 --- a/library/coretests/tests/num/dec2flt/mod.rs +++ b/library/coretests/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/coretests/tests/num/dec2flt/parse.rs b/library/coretests/tests/num/dec2flt/parse.rs index 4a5d24ba7d5fa..ec705a61e13ca 100644 --- a/library/coretests/tests/num/dec2flt/parse.rs +++ b/library/coretests/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 6c34daff57101922dc97e1860bf5940ea88d3906 Mon Sep 17 00:00:00 2001 From: Trevor Gross Date: Mon, 9 Dec 2024 09:25:22 +0000 Subject: [PATCH 21/24] 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/coretests/tests/num/dec2flt/float.rs | 8 ++++---- library/coretests/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/coretests/tests/num/dec2flt/float.rs b/library/coretests/tests/num/dec2flt/float.rs index 7a9587a18d030..40f0776e02148 100644 --- a/library/coretests/tests/num/dec2flt/float.rs +++ b/library/coretests/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/coretests/tests/num/dec2flt/lemire.rs b/library/coretests/tests/num/dec2flt/lemire.rs index f71bbb7c7a318..9f228a25e46b1 100644 --- a/library/coretests/tests/num/dec2flt/lemire.rs +++ b/library/coretests/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 19a909ae0e9f923db2da0fc10264a75b7aa5ad99 Mon Sep 17 00:00:00 2001 From: Trevor Gross Date: Mon, 9 Dec 2024 09:25:22 +0000 Subject: [PATCH 22/24] 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 | 272 ++++++++++++------ library/core/src/num/dec2flt/lemire.rs | 18 +- library/core/src/num/dec2flt/mod.rs | 6 +- library/core/src/num/dec2flt/slow.rs | 18 +- library/coretests/tests/num/dec2flt/float.rs | 40 +++ library/coretests/tests/num/dec2flt/lemire.rs | 6 + library/coretests/tests/num/dec2flt/parse.rs | 15 + src/etc/test-float-parse/src/traits.rs | 6 +- 8 files changed, 267 insertions(+), 114 deletions(-) diff --git a/library/core/src/num/dec2flt/float.rs b/library/core/src/num/dec2flt/float.rs index da57aa9a546af..b8a28a6756917 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; +} + +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 `f32` and `f64`. +/// 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,107 @@ 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; - /// 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 MIN_EXPONENT_ROUND_TO_EVEN: i32; - const MAX_EXPONENT_ROUND_TO_EVEN: i32; + /// Bit width of the float + const BITS: u32; - // Minimum exponent that for a fast path case, or `-⌊(MANTISSA_EXPLICIT_BITS+1)/log2(5)⌋` - const MIN_EXPONENT_FAST_PATH: i64; + /// The number of bits in the significand, *including* the hidden bit. + const SIG_TOTAL_BITS: u32; - // Maximum exponent that for a fast path case, or `⌊(MANTISSA_EXPLICIT_BITS+1)/log2(5)⌋` - const MAX_EXPONENT_FAST_PATH: i64; + const EXP_MASK: Self::Int; + const SIG_MASK: Self::Int; - // 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; - - // Minimum exponent value `-(1 << (EXP_BITS - 1)) + 1`. - const MINIMUM_EXPONENT: i32; + /// The number of bits in the significand, *excluding* the hidden bit. + const SIG_BITS: u32 = Self::SIG_TOTAL_BITS - 1; + + /// Number of bits in the exponent. + const EXP_BITS: u32 = Self::BITS - Self::SIG_BITS - 1; + + /// The saturated (maximum bitpattern) value of the exponent, i.e. the infinite + /// representation. + /// + /// This shifted fully right, use `EXP_MASK` for the shifted value. + const EXP_SAT: u32 = (1 << Self::EXP_BITS) - 1; + + /// Signed version of `EXP_SAT` since we convert a lot. + const INFINITE_POWER: i32 = Self::EXP_SAT as i32; + + /// The exponent bias value. This is also the maximum value of the exponent. + const EXP_BIAS: u32 = Self::EXP_SAT >> 1; + + /// Minimum exponent value of normal values. + const EXP_MIN: i32 = -(Self::EXP_BIAS as i32 - 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; - // Largest exponent value `(1 << EXP_BITS) - 1`. - const INFINITE_POWER: i32; + /* limits related to Fast pathing */ + + /// 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 = { + let largest_pow2 = Self::EXP_BIAS + 1; + pow2_to_pow10(largest_pow2 as i64) as 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. + /// + /// The smallest power of ten is represented by `⌊log10(2^-n / (2^64 - 1))⌋`, where `n` is + /// the smallest power of two. The `2^64 - 1)` denomenator comes from the number of values + /// that are representable by the intermediate storage format. I don't actually know _why_ + /// the storage format is relevant here. + /// + /// The values may be calculated using the formula. Unfortunately we cannot calculate them at + /// compile time since intermediates exceed the range of an `f64`. + const SMALLEST_POWER_OF_TEN: i32; - // Index (in bits) of the sign. - const SIGN_INDEX: usize; + /// Maximum exponent for a fast path case, or `⌊(SIG_BITS+1)/log2(5)⌋` + // assuming FLT_EVAL_METHOD = 0 + const MAX_EXPONENT_FAST_PATH: i64 = { + let log2_5 = f64::consts::LOG2_10 - 1.0; + (Self::SIG_TOTAL_BITS as f64 / log2_5) as i64 + }; - // Smallest decimal exponent for a non-zero value. - const SMALLEST_POWER_OF_TEN: i32; + /// Minimum exponent for a fast path case, or `-⌊(SIG_BITS+1)/log2(5)⌋` + const MIN_EXPONENT_FAST_PATH: i64 = -Self::MAX_EXPONENT_FAST_PATH; - // Largest decimal exponent for a non-infinite value. - const LARGEST_POWER_OF_TEN: i32; + /// Maximum exponent that can be represented for a disguised-fast path case. + /// This is `MAX_EXPONENT_FAST_PATH + ⌊(SIG_BITS+1)/log2(10)⌋` + const MAX_EXPONENT_DISGUISED_FAST_PATH: i64 = + Self::MAX_EXPONENT_FAST_PATH + (Self::SIG_TOTAL_BITS as f64 / f64::consts::LOG2_10) as i64; - // 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::SIG_TOTAL_BITS; /// Converts integer into float through an as cast. /// This is only called in the fast-path algorithm, and therefore @@ -96,27 +184,51 @@ 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 `-(EXP_BIAS + SIG_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::EXP_MASK) >> Self::SIG_BITS).cast(); + let mantissa = if exponent == 0 { + (bits & Self::SIG_MASK) << 1 + } else { + (bits & Self::SIG_MASK) | (Self::Int::ONE << Self::SIG_BITS) + }; + // Exponent bias + mantissa shift + exponent -= (Self::EXP_BIAS + Self::SIG_BITS) as i16; + (mantissa.into(), exponent, sign) + } +} + +/// Solve for `b` in `10^b = 2^a` +const fn pow2_to_pow10(a: i64) -> i64 { + let res = (a as f64) / f64::consts::LOG2_10; + res as i64 } 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 SIG_TOTAL_BITS: u32 = Self::MANTISSA_DIGITS; + const EXP_MASK: Self::Int = Self::EXP_MASK; + const SIG_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 +248,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 +258,21 @@ 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 SIG_TOTAL_BITS: u32 = Self::MANTISSA_DIGITS; + const EXP_MASK: Self::Int = Self::EXP_MASK; + const SIG_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 +293,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..f84929a03c172 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::SIG_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 @@ -61,8 +61,8 @@ pub fn compute_float(q: i64, mut w: u64) -> BiasedFp { } } let upperbit = (hi >> 63) as i32; - let mut mantissa = hi >> (upperbit + 64 - F::MANTISSA_EXPLICIT_BITS as i32 - 3); - let mut power2 = power(q as i32) + upperbit - lz as i32 - F::MINIMUM_EXPONENT; + let mut mantissa = hi >> (upperbit + 64 - F::SIG_BITS as i32 - 3); + let mut power2 = power(q as i32) + upperbit - lz as i32 - F::EXP_MIN + 1; if power2 <= 0 { if -power2 + 1 >= 64 { // Have more than 64 bits below the minimum exponent, must be 0. @@ -72,7 +72,7 @@ pub fn compute_float(q: i64, mut w: u64) -> BiasedFp { mantissa >>= -power2 + 1; mantissa += mantissa & 1; mantissa >>= 1; - power2 = (mantissa >= (1_u64 << F::MANTISSA_EXPLICIT_BITS)) as i32; + power2 = (mantissa >= (1_u64 << F::SIG_BITS)) as i32; return BiasedFp { m: mantissa, p_biased: power2 }; } // Need to handle rounding ties. Normally, we need to round up, @@ -89,8 +89,8 @@ 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 << (upperbit + 64 - F::MANTISSA_EXPLICIT_BITS as i32 - 3)) == hi + && mantissa & 0b11 == 0b01 + && (mantissa << (upperbit + 64 - F::SIG_BITS as i32 - 3)) == hi { // Zero the lowest bit, so we don't round up. mantissa &= !1_u64; @@ -98,15 +98,15 @@ pub fn compute_float(q: i64, mut w: u64) -> BiasedFp { // Round-to-even, then shift the significant digits into place. mantissa += mantissa & 1; mantissa >>= 1; - if mantissa >= (2_u64 << F::MANTISSA_EXPLICIT_BITS) { + if mantissa >= (2_u64 << F::SIG_BITS) { // Rounding up overflowed, so the carry bit is set. Set the // mantissa to 1 (only the implicit, hidden bit is set) and // increase the exponent. - mantissa = 1_u64 << F::MANTISSA_EXPLICIT_BITS; + mantissa = 1_u64 << F::SIG_BITS; power2 += 1; } // Zero out the hidden bit. - mantissa &= !(1_u64 << F::MANTISSA_EXPLICIT_BITS); + mantissa &= !(1_u64 << F::SIG_BITS); if power2 >= F::INFINITE_POWER { // Exponent is above largest normal value, must be infinite. return fp_inf; diff --git a/library/core/src/num/dec2flt/mod.rs b/library/core/src/num/dec2flt/mod.rs index 9d1989c4b817d..d1a0e1db31314 100644 --- a/library/core/src/num/dec2flt/mod.rs +++ b/library/core/src/num/dec2flt/mod.rs @@ -232,10 +232,10 @@ pub fn pfe_invalid() -> ParseFloatError { } /// Converts a `BiasedFp` to the closest machine float type. -fn biased_fp_to_float(x: BiasedFp) -> T { +fn biased_fp_to_float(x: BiasedFp) -> F { let mut word = x.m; - word |= (x.p_biased as u64) << T::MANTISSA_EXPLICIT_BITS; - T::from_u64_bits(word) + word |= (x.p_biased as u64) << F::SIG_BITS; + F::from_u64_bits(word) } /// Converts a decimal string into a floating point number. diff --git a/library/core/src/num/dec2flt/slow.rs b/library/core/src/num/dec2flt/slow.rs index e0c4ae117da02..3baed42652393 100644 --- a/library/core/src/num/dec2flt/slow.rs +++ b/library/core/src/num/dec2flt/slow.rs @@ -74,36 +74,36 @@ pub(crate) fn parse_long_mantissa(s: &[u8]) -> BiasedFp { } // We are now in the range [1/2 ... 1] but the binary format uses [1 ... 2]. exp2 -= 1; - while (F::MINIMUM_EXPONENT + 1) > exp2 { - let mut n = ((F::MINIMUM_EXPONENT + 1) - exp2) as usize; + while F::EXP_MIN > exp2 { + let mut n = (F::EXP_MIN - exp2) as usize; if n > MAX_SHIFT { n = MAX_SHIFT; } d.right_shift(n); exp2 += n as i32; } - if (exp2 - F::MINIMUM_EXPONENT) >= F::INFINITE_POWER { + if (exp2 - F::EXP_MIN + 1) >= F::INFINITE_POWER { return fp_inf; } // 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::SIG_BITS as usize + 1); let mut mantissa = d.round(); - if mantissa >= (1_u64 << (F::MANTISSA_EXPLICIT_BITS + 1)) { + if mantissa >= (1_u64 << (F::SIG_BITS + 1)) { // Rounding up overflowed to the carry bit, need to // shift back to the hidden bit. d.right_shift(1); exp2 += 1; mantissa = d.round(); - if (exp2 - F::MINIMUM_EXPONENT) >= F::INFINITE_POWER { + if (exp2 - F::EXP_MIN + 1) >= F::INFINITE_POWER { return fp_inf; } } - let mut power2 = exp2 - F::MINIMUM_EXPONENT; - if mantissa < (1_u64 << F::MANTISSA_EXPLICIT_BITS) { + let mut power2 = exp2 - F::EXP_MIN + 1; + if mantissa < (1_u64 << F::SIG_BITS) { power2 -= 1; } // Zero out all the bits above the explicit mantissa bits. - mantissa &= (1_u64 << F::MANTISSA_EXPLICIT_BITS) - 1; + mantissa &= (1_u64 << F::SIG_BITS) - 1; BiasedFp { m: mantissa, p_biased: power2 } } diff --git a/library/coretests/tests/num/dec2flt/float.rs b/library/coretests/tests/num/dec2flt/float.rs index 40f0776e02148..b5afd3e3b2436 100644 --- a/library/coretests/tests/num/dec2flt/float.rs +++ b/library/coretests/tests/num/dec2flt/float.rs @@ -31,3 +31,43 @@ fn test_f64_integer_decode() { let (nan_m, nan_p, _nan_s) = f64::NAN.integer_decode(); assert_eq!((nan_m, nan_p), (6755399441055744, 972)); } + +/* Sanity checks of computed magic numbers */ + +#[test] +fn test_f32_consts() { + assert_eq!(::INFINITY, f32::INFINITY); + assert_eq!(::NEG_INFINITY, -f32::INFINITY); + assert_eq!(::NAN.to_bits(), f32::NAN.to_bits()); + assert_eq!(::NEG_NAN.to_bits(), (-f32::NAN).to_bits()); + assert_eq!(::SIG_BITS, 23); + assert_eq!(::MIN_EXPONENT_ROUND_TO_EVEN, -17); + assert_eq!(::MAX_EXPONENT_ROUND_TO_EVEN, 10); + assert_eq!(::MIN_EXPONENT_FAST_PATH, -10); + assert_eq!(::MAX_EXPONENT_FAST_PATH, 10); + assert_eq!(::MAX_EXPONENT_DISGUISED_FAST_PATH, 17); + assert_eq!(::EXP_MIN, -126); + assert_eq!(::EXP_SAT, 0xff); + assert_eq!(::SMALLEST_POWER_OF_TEN, -65); + assert_eq!(::LARGEST_POWER_OF_TEN, 38); + assert_eq!(::MAX_MANTISSA_FAST_PATH, 16777216); +} + +#[test] +fn test_f64_consts() { + assert_eq!(::INFINITY, f64::INFINITY); + assert_eq!(::NEG_INFINITY, -f64::INFINITY); + assert_eq!(::NAN.to_bits(), f64::NAN.to_bits()); + assert_eq!(::NEG_NAN.to_bits(), (-f64::NAN).to_bits()); + assert_eq!(::SIG_BITS, 52); + assert_eq!(::MIN_EXPONENT_ROUND_TO_EVEN, -4); + assert_eq!(::MAX_EXPONENT_ROUND_TO_EVEN, 23); + assert_eq!(::MIN_EXPONENT_FAST_PATH, -22); + assert_eq!(::MAX_EXPONENT_FAST_PATH, 22); + assert_eq!(::MAX_EXPONENT_DISGUISED_FAST_PATH, 37); + assert_eq!(::EXP_MIN, -1022); + assert_eq!(::EXP_SAT, 0x7ff); + assert_eq!(::SMALLEST_POWER_OF_TEN, -342); + assert_eq!(::LARGEST_POWER_OF_TEN, 308); + assert_eq!(::MAX_MANTISSA_FAST_PATH, 9007199254740992); +} diff --git a/library/coretests/tests/num/dec2flt/lemire.rs b/library/coretests/tests/num/dec2flt/lemire.rs index 9f228a25e46b1..0db80fbd52506 100644 --- a/library/coretests/tests/num/dec2flt/lemire.rs +++ b/library/coretests/tests/num/dec2flt/lemire.rs @@ -1,3 +1,4 @@ +use core::num::dec2flt::float::RawFloat; use core::num::dec2flt::lemire::compute_float; fn compute_float32(q: i64, w: u64) -> (i32, u64) { @@ -27,6 +28,11 @@ fn compute_float_f32_rounding() { // Let's check the lines to see if anything is different in table... assert_eq!(compute_float32(-10, 167772190000000000), (151, 2)); assert_eq!(compute_float32(-10, 167772200000000000), (151, 2)); + + // Check the rounding point between infinity and the next representable number down + assert_eq!(compute_float32(38, 3), (f32::INFINITE_POWER - 1, 6402534)); + assert_eq!(compute_float32(38, 4), (f32::INFINITE_POWER, 0)); // infinity + assert_eq!(compute_float32(20, 3402823470000000000), (f32::INFINITE_POWER - 1, 8388607)); } #[test] diff --git a/library/coretests/tests/num/dec2flt/parse.rs b/library/coretests/tests/num/dec2flt/parse.rs index ec705a61e13ca..59be3915052d8 100644 --- a/library/coretests/tests/num/dec2flt/parse.rs +++ b/library/coretests/tests/num/dec2flt/parse.rs @@ -57,6 +57,21 @@ macro_rules! assert_float_result_bits_eq { }}; } +#[test] +fn regression() { + // These showed up in fuzz tests when the minimum exponent was incorrect. + assert_float_result_bits_eq!( + 0x0, + f64, + "3313756768023998018398807867233977556112078681253148176737587500333136120852692315608454494981109839693784033457129423181787087843504060087613228932431e-475" + ); + assert_float_result_bits_eq!( + 0x0, + f64, + "5298127456259331337220.92759278003098321644501973966679724599271041396379712108366679824365568578569680024083293475291869842408884554511641179110778276695274832779269225510492006696321279587846006535230380114430977056662212751544508159333199129106162019382177820713609e-346" + ); +} + #[test] fn issue31109() { // Regression test for #31109. 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 37e223ccaa1d0036540e6e3de5d4ed4b721afd60 Mon Sep 17 00:00:00 2001 From: Trevor Gross Date: Mon, 9 Dec 2024 09:25:22 +0000 Subject: [PATCH 23/24] 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 133705c402941c0825fe658a932289847478cf9f Mon Sep 17 00:00:00 2001 From: xizheyin Date: Thu, 27 Feb 2025 20:54:26 +0800 Subject: [PATCH 24/24] [rustdoc] hide item that is not marked as doc(inline) and whose src is doc(hidden) Signed-off-by: xizheyin --- src/librustdoc/passes/strip_hidden.rs | 22 +++++++++++++++++-- tests/rustdoc/doc-hidden-reexports-109449.rs | 18 +++++++-------- tests/rustdoc/doc-hidden-source.rs | 16 ++++++++++++++ tests/rustdoc/inline_cross/inline_hidden.rs | 4 ++-- tests/rustdoc/reexport-attr-merge.rs | 4 ++-- .../reexport-doc-hidden-inside-private.rs | 4 ++-- tests/rustdoc/reexport-doc-hidden.rs | 4 ++-- tests/rustdoc/reexport-hidden-macro.rs | 2 +- tests/rustdoc/reexport-of-doc-hidden.rs | 12 +++++----- 9 files changed, 60 insertions(+), 26 deletions(-) create mode 100644 tests/rustdoc/doc-hidden-source.rs diff --git a/src/librustdoc/passes/strip_hidden.rs b/src/librustdoc/passes/strip_hidden.rs index a71bb62e56c74..bcdca862862d4 100644 --- a/src/librustdoc/passes/strip_hidden.rs +++ b/src/librustdoc/passes/strip_hidden.rs @@ -7,9 +7,8 @@ use rustc_middle::ty::TyCtxt; use rustc_span::symbol::sym; use tracing::debug; -use crate::clean; use crate::clean::utils::inherits_doc_hidden; -use crate::clean::{Item, ItemIdSet}; +use crate::clean::{self, Item, ItemIdSet, reexport_chain}; use crate::core::DocContext; use crate::fold::{DocFolder, strip_item}; use crate::passes::{ImplStripper, Pass}; @@ -89,6 +88,25 @@ impl Stripper<'_, '_> { impl DocFolder for Stripper<'_, '_> { fn fold_item(&mut self, i: Item) -> Option { let has_doc_hidden = i.is_doc_hidden(); + + if let clean::ImportItem(clean::Import { source, .. }) = &i.kind + && let Some(source_did) = source.did + && let Some(import_def_id) = i.def_id().and_then(|def_id| def_id.as_local()) + { + let reexports = reexport_chain(self.tcx, import_def_id, source_did); + + // Check if any reexport in the chain has a hidden source + let has_hidden_source = reexports + .iter() + .filter_map(|reexport| reexport.id()) + .any(|reexport_did| self.tcx.is_doc_hidden(reexport_did)) + || self.tcx.is_doc_hidden(source_did); + + if has_hidden_source { + return None; + } + } + let is_impl_or_exported_macro = match i.kind { clean::ImplItem(..) => true, // If the macro has the `#[macro_export]` attribute, it means it's accessible at the diff --git a/tests/rustdoc/doc-hidden-reexports-109449.rs b/tests/rustdoc/doc-hidden-reexports-109449.rs index 8f195544120af..78b9214300a95 100644 --- a/tests/rustdoc/doc-hidden-reexports-109449.rs +++ b/tests/rustdoc/doc-hidden-reexports-109449.rs @@ -26,21 +26,21 @@ pub mod single_reexport { //@ has 'foo/single_reexport/index.html' // First we check that we have 4 type aliases. - //@ count - '//*[@id="main-content"]/*[@class="item-table reexports"]//code' 4 + //@ count - '//*[@id="main-content"]/*[@class="item-table reexports"]//code' 0 // Then we check that we have the correct link for each re-export. //@ !has - '//*[@href="struct.Foo.html"]' 'Foo' - //@ has - '//*[@id="reexport.Foo"]/code' 'pub use crate::private_module::Public as Foo;' + //@ !has - '//*[@id="reexport.Foo"]/code' 'pub use crate::private_module::Public as Foo;' pub use crate::private_module::Public as Foo; //@ !has - '//*[@href="type.Foo2.html"]' 'Foo2' - //@ has - '//*[@id="reexport.Foo2"]/code' 'pub use crate::private_module::Bar as Foo2;' + //@ !has - '//*[@id="reexport.Foo2"]/code' 'pub use crate::private_module::Bar as Foo2;' pub use crate::private_module::Bar as Foo2; //@ !has - '//*[@href="type.Yo.html"]' 'Yo' - //@ has - '//*[@id="reexport.Yo"]/code' 'pub use crate::Bar3 as Yo;' + //@ !has - '//*[@id="reexport.Yo"]/code' 'pub use crate::Bar3 as Yo;' pub use crate::Bar3 as Yo; //@ !has - '//*[@href="struct.Yo2.html"]' 'Yo2' - //@ has - '//*[@id="reexport.Yo2"]/code' 'pub use crate::FooFoo as Yo2;' + //@ !has - '//*[@id="reexport.Yo2"]/code' 'pub use crate::FooFoo as Yo2;' pub use crate::FooFoo as Yo2; // Checking that each file is also created as expected. @@ -70,19 +70,19 @@ pub mod single_reexport_no_inline { //@ has - '//*[@id="main-content"]/*[@class="section-header"]' 'Re-exports' // Now we check that we don't have links to the items, just `pub use`. - //@ has - '//*[@id="main-content"]//*' 'pub use crate::private_module::Public as XFoo;' + //@ !has - '//*[@id="main-content"]//*' 'pub use crate::private_module::Public as XFoo;' //@ !has - '//*[@id="main-content"]//a' 'XFoo' #[doc(no_inline)] pub use crate::private_module::Public as XFoo; - //@ has - '//*[@id="main-content"]//*' 'pub use crate::private_module::Bar as Foo2;' + //@ !has - '//*[@id="main-content"]//*' 'pub use crate::private_module::Bar as Foo2;' //@ !has - '//*[@id="main-content"]//a' 'Foo2' #[doc(no_inline)] pub use crate::private_module::Bar as Foo2; - //@ has - '//*[@id="main-content"]//*' 'pub use crate::Bar3 as Yo;' + //@ !has - '//*[@id="main-content"]//*' 'pub use crate::Bar3 as Yo;' //@ !has - '//*[@id="main-content"]//a' 'Yo' #[doc(no_inline)] pub use crate::Bar3 as Yo; - //@ has - '//*[@id="main-content"]//*' 'pub use crate::FooFoo as Yo2;' + //@ !has - '//*[@id="main-content"]//*' 'pub use crate::FooFoo as Yo2;' //@ !has - '//*[@id="main-content"]//a' 'Yo2' #[doc(no_inline)] pub use crate::FooFoo as Yo2; diff --git a/tests/rustdoc/doc-hidden-source.rs b/tests/rustdoc/doc-hidden-source.rs new file mode 100644 index 0000000000000..b6bc622dd584f --- /dev/null +++ b/tests/rustdoc/doc-hidden-source.rs @@ -0,0 +1,16 @@ +// Test for . + +#![crate_name = "foo"] + +//@ has 'foo/index.html' +//@ !has - '//*[@id="main-content"]//*[@class="struct"]' 'Bar' +#[doc(hidden)] +pub struct Bar; + +//@ !has - '//*' 'pub use crate::Bar as A;' +pub use crate::Bar as A; +//@ !has - '//*' 'pub use crate::A as B;' +pub use crate::A as B; +//@ has - '//dt/a[@class="struct"]' 'C' +#[doc(inline)] +pub use crate::Bar as C; diff --git a/tests/rustdoc/inline_cross/inline_hidden.rs b/tests/rustdoc/inline_cross/inline_hidden.rs index 49ca2db6a2245..f6727de2cc67f 100644 --- a/tests/rustdoc/inline_cross/inline_hidden.rs +++ b/tests/rustdoc/inline_cross/inline_hidden.rs @@ -6,7 +6,7 @@ extern crate rustdoc_hidden; //@ has inline_hidden/index.html // Ensures this item is not inlined. -//@ has - '//*[@id="reexport.Foo"]/code' 'pub use rustdoc_hidden::Foo;' +//@ !has - '//*[@id="reexport.Foo"]/code' 'pub use rustdoc_hidden::Foo;' #[doc(no_inline)] pub use rustdoc_hidden::Foo; @@ -16,7 +16,7 @@ pub use rustdoc_hidden::Foo; pub use rustdoc_hidden::Foo as Inlined; // Even with this import, we should not see `Foo`. -//@ count - '//dt' 4 +//@ count - '//dt' 3 //@ has - '//dt/a[@class="struct"]' 'Bar' //@ has - '//dt/a[@class="fn"]' 'foo' pub use rustdoc_hidden::*; diff --git a/tests/rustdoc/reexport-attr-merge.rs b/tests/rustdoc/reexport-attr-merge.rs index e4a406c3845ad..aef302eb0b29a 100644 --- a/tests/rustdoc/reexport-attr-merge.rs +++ b/tests/rustdoc/reexport-attr-merge.rs @@ -18,7 +18,7 @@ pub use Foo1 as Foo2; // First we ensure that only the reexport `Bar2` and the inlined struct `Bar` // are inlined. -//@ count - '//a[@class="struct"]' 2 +//@ count - '//a[@class="struct"]' 1 // Then we check that `cfg` is displayed for base item, but not for intermediate re-exports. //@ has - '//*[@class="stab portability"]' 'foo' //@ !has - '//*[@class="stab portability"]' 'bar' @@ -29,5 +29,5 @@ pub use Foo2 as Bar; // This one should appear but `Bar2` won't be linked because there is no // `#[doc(inline)]`. -//@ has - '//*[@id="reexport.Bar2"]' 'pub use Foo2 as Bar2;' +//@ !has - '//*[@id="reexport.Bar2"]' 'pub use Foo2 as Bar2;' pub use Foo2 as Bar2; diff --git a/tests/rustdoc/reexport-doc-hidden-inside-private.rs b/tests/rustdoc/reexport-doc-hidden-inside-private.rs index 8e194ef74fb82..bae2aa78ec7a3 100644 --- a/tests/rustdoc/reexport-doc-hidden-inside-private.rs +++ b/tests/rustdoc/reexport-doc-hidden-inside-private.rs @@ -9,8 +9,8 @@ mod private_module { } //@ has 'foo/index.html' -//@ has - '//*[@id="reexport.Foo"]/code' 'pub use crate::private_module::Public as Foo;' +//@ !has - '//*[@id="reexport.Foo"]/code' 'pub use crate::private_module::Public as Foo;' pub use crate::private_module::Public as Foo; // Glob re-exports with no visible items should not be displayed. -//@ count - '//*[@class="item-table reexports"]/dt' 1 +//@ count - '//*[@class="item-table reexports"]/dt' 0 pub use crate::private_module::*; diff --git a/tests/rustdoc/reexport-doc-hidden.rs b/tests/rustdoc/reexport-doc-hidden.rs index b912362f2987a..1468e9ad957f7 100644 --- a/tests/rustdoc/reexport-doc-hidden.rs +++ b/tests/rustdoc/reexport-doc-hidden.rs @@ -8,7 +8,7 @@ pub type Type = u32; //@ has 'foo/index.html' -//@ has - '//*[@id="reexport.Type2"]/code' 'pub use crate::Type as Type2;' +//@ !has - '//*[@id="reexport.Type2"]/code' 'pub use crate::Type as Type2;' pub use crate::Type as Type2; //@ count - '//*[@id="reexport.Type3"]' 0 @@ -21,5 +21,5 @@ macro_rules! foo { () => {}; } -//@ has - '//*[@id="reexport.Macro"]/code' 'pub use crate::foo as Macro;' +//@ !has - '//*[@id="reexport.Macro"]/code' 'pub use crate::foo as Macro;' pub use crate::foo as Macro; diff --git a/tests/rustdoc/reexport-hidden-macro.rs b/tests/rustdoc/reexport-hidden-macro.rs index 9b83bca3906d2..7345149c64522 100644 --- a/tests/rustdoc/reexport-hidden-macro.rs +++ b/tests/rustdoc/reexport-hidden-macro.rs @@ -5,7 +5,7 @@ //@ has 'foo/index.html' //@ has - '//*[@id="main-content"]//a[@href="macro.Macro2.html"]' 'Macro2' -//@ has - '//*[@id="reexport.Macro"]/code' 'pub use crate::foo as Macro;' +//@ !has - '//*[@id="reexport.Macro"]/code' 'pub use crate::foo as Macro;' //@ has 'foo/macro.Macro2.html' //@ has - '//*[@class="docblock"]' 'Displayed' diff --git a/tests/rustdoc/reexport-of-doc-hidden.rs b/tests/rustdoc/reexport-of-doc-hidden.rs index 21511bc2aea99..e901d0ff8a2bc 100644 --- a/tests/rustdoc/reexport-of-doc-hidden.rs +++ b/tests/rustdoc/reexport-of-doc-hidden.rs @@ -12,13 +12,13 @@ macro_rules! foo { } //@ has 'foo/index.html' -//@ has - '//*[@id="reexport.Macro"]/code' 'pub use crate::foo as Macro;' +//@ !has - '//*[@id="reexport.Macro"]/code' 'pub use crate::foo as Macro;' pub use crate::foo as Macro; -//@ has - '//*[@id="reexport.Macro2"]/code' 'pub use crate::foo as Macro2;' +//@ !has - '//*[@id="reexport.Macro2"]/code' 'pub use crate::foo as Macro2;' pub use crate::foo as Macro2; -//@ has - '//*[@id="reexport.Boo"]/code' 'pub use crate::Bar as Boo;' +//@ !has - '//*[@id="reexport.Boo"]/code' 'pub use crate::Bar as Boo;' pub use crate::Bar as Boo; -//@ has - '//*[@id="reexport.Boo2"]/code' 'pub use crate::Bar as Boo2;' +//@ !has - '//*[@id="reexport.Boo2"]/code' 'pub use crate::Bar as Boo2;' pub use crate::Bar as Boo2; pub fn fofo() {} @@ -30,9 +30,9 @@ pub use crate::fofo as f2; pub mod sub { //@ has 'foo/sub/index.html' - //@ has - '//*[@id="reexport.Macro"]/code' 'pub use crate::foo as Macro;' + //@ !has - '//*[@id="reexport.Macro"]/code' 'pub use crate::foo as Macro;' pub use crate::foo as Macro; - //@ has - '//*[@id="reexport.Macro2"]/code' 'pub use crate::foo as Macro2;' + //@ !has - '//*[@id="reexport.Macro2"]/code' 'pub use crate::foo as Macro2;' pub use crate::foo as Macro2; //@ has - '//*[@id="reexport.f1"]/code' 'pub use crate::fofo as f1;'