dec2flt: Clean up float parsing modules #134063
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
rustbot
added
S-waiting-on-review
tgross35
force-pushed
the
dec2flt-refactoring
branch
from
c2f95c2 to
3fa1f00
Compare
tgross35
changed the title
tgross35
force-pushed
the
dec2flt-refactoring
branch
3 times, most recently
from
058be88 to
89f2066
Compare
|
r? libs |
tgross35
changed the title
|
I'm by no means an expert in float parsing, but this change is mostly mechanical, so I do feel comfortable approving it. I looked through each commit and the changes seemed reasonable. For the float trait refactor I did take a closer look but a mistake there could have definitely slipped by (though I guess that's always true), but I assume you also ran the detailed float parsing tests (I think they don't run in CI) to ensure it still works as intended. @bors r+ |
bors
added
S-waiting-on-bors
Noratrieb
added a commit
to Noratrieb/rust
that referenced
this pull request
Dec 31, 2024
…ratrieb dec2flt: Clean up float parsing modules This is the first portion of my work adding support for parsing and printing `f16`. Changes in `float.rs` replace the magic constants with expressions and add some use of generics to better support the new float types. Everything else is related to documentation or naming; there are no functional changes in this PR. This can be reviewed by commit.
bors
added a commit
to rust-lang-ci/rust
that referenced
this pull request
Dec 31, 2024
Rollup of 9 pull requests Successful merges: - rust-lang#132474 (Add more mailmap entries) - rust-lang#133486 (borrowck diagnostics: make `add_move_error_suggestions` use the HIR rather than `SourceMap`) - rust-lang#134063 (dec2flt: Clean up float parsing modules) - rust-lang#134861 (Add GUI test for item info elements color) - rust-lang#134968 (Print how to rebless Python formatting in tidy) - rust-lang#134971 (chore: fix typos) - rust-lang#134972 (add .mailmap entry for myself) - rust-lang#134974 (Revert rust-lang#119515 single line where clause style guide) - rust-lang#134975 (Revert style guide rhs break) r? `@ghost` `@rustbot` modify labels: rollup
Just to confirm, these do still pass. We run a few minutes of edge cases in CI but the
I don't think anyone here is, I'm doing a lot of reverse engineering and the people who authored this implementation don't seem to be active anymore either 😞 but thanks for the review! |
|
Agh the exhaustive tests pass but one of the newer edge case tests failed when I tested with a rebase. Looks like I need to adjust something here. @bors r- |
bors
added
S-waiting-on-author
|
☔ The latest upstream changes (presumably #134286) made this pull request unmergeable. Please resolve the merge conflicts. |
tgross35
force-pushed
the
dec2flt-refactoring
branch
from
89f2066 to
01a519f
Compare
These constants can be useful outside of their current module. Make them `pub(crate)` to allow for this.
Fix or elaborate existing float parsing documentation. This includes introducing a convention that should make naming more consistent.
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.
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.
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`.
This is just a bit of code cleanup to make use of returning early.
tgross35
force-pushed
the
dec2flt-refactoring
branch
from
01a519f to
37e223c
Compare
|
Finally circling back to this; I had a mistake in the formula for
And test-float-parse completes successfully again 🎉. Since nothing functional changed outside of the fixed constant, @bors r=Noratrieb |
bors
added
S-waiting-on-bors
tgross35
added a commit
to tgross35/rust
that referenced
this pull request
Mar 4, 2025
…ratrieb dec2flt: Clean up float parsing modules This is the first portion of my work adding support for parsing and printing `f16`. Changes in `float.rs` replace the magic constants with expressions and add some use of generics to better support the new float types. Everything else is related to documentation or naming; there are no functional changes in this PR. This can be reviewed by commit.
bors
added a commit
to rust-lang-ci/rust
that referenced
this pull request
Mar 4, 2025
Rollup of 10 pull requests Successful merges: - rust-lang#134063 (dec2flt: Clean up float parsing modules) - rust-lang#136581 (Retire the legacy `Makefile`-based `run-make` test infra) - rust-lang#136975 (Look for `python3` first on MacOS, not `py`) - rust-lang#137147 (Add exclude to config.toml) - rust-lang#137240 (Slightly reformat `std::fs::remove_dir_all` error docs) - rust-lang#137303 (Remove `MaybeForgetReturn` suggestion) - rust-lang#137327 (Undeprecate env::home_dir) - rust-lang#137463 ([illumos] attempt to use posix_spawn to spawn processes) - rust-lang#137534 ([rustdoc] hide item that is not marked as doc(inline) and whose src is doc(hidden)) - rust-lang#137829 (Stabilize [T]::split_off... methods) r? `@ghost` `@rustbot` modify labels: rollup
jieyouxu
added a commit
to jieyouxu/rust
that referenced
this pull request
Mar 4, 2025
…ratrieb dec2flt: Clean up float parsing modules This is the first portion of my work adding support for parsing and printing `f16`. Changes in `float.rs` replace the magic constants with expressions and add some use of generics to better support the new float types. Everything else is related to documentation or naming; there are no functional changes in this PR. This can be reviewed by commit.
bors
added a commit
to rust-lang-ci/rust
that referenced
this pull request
Mar 4, 2025
Rollup of 10 pull requests Successful merges: - rust-lang#134063 (dec2flt: Clean up float parsing modules) - rust-lang#136662 (Count char width at most once in `Formatter::pad`) - rust-lang#137011 (Promote ohos targets to tier2 with host tools.) - rust-lang#137077 (Postprocess bootstrap metrics into GitHub job summary) - rust-lang#137327 (Undeprecate env::home_dir) - rust-lang#137373 (Compile run-make-support and run-make tests with the bootstrap compiler) - rust-lang#137463 ([illumos] attempt to use posix_spawn to spawn processes) - rust-lang#137477 (uefi: Add Service Binding Protocol abstraction) - rust-lang#137569 (Stablize `string_extend_from_within`) - rust-lang#137667 (Add `dist::Gcc` build step) r? `@ghost` `@rustbot` modify labels: rollup
workingjubilee
added a commit
to workingjubilee/rustc
that referenced
this pull request
Mar 5, 2025
…ratrieb dec2flt: Clean up float parsing modules This is the first portion of my work adding support for parsing and printing `f16`. Changes in `float.rs` replace the magic constants with expressions and add some use of generics to better support the new float types. Everything else is related to documentation or naming; there are no functional changes in this PR. This can be reviewed by commit.
bors
added a commit
to rust-lang-ci/rust
that referenced
this pull request
Mar 5, 2025
…kingjubilee Rollup of 25 pull requests Successful merges: - rust-lang#134063 (dec2flt: Clean up float parsing modules) - rust-lang#136581 (Retire the legacy `Makefile`-based `run-make` test infra) - rust-lang#136662 (Count char width at most once in `Formatter::pad`) - rust-lang#136798 (Added documentation for flushing per rust-lang#74348) - rust-lang#137240 (Slightly reformat `std::fs::remove_dir_all` error docs) - rust-lang#137303 (Remove `MaybeForgetReturn` suggestion) - rust-lang#137327 (Undeprecate env::home_dir) - rust-lang#137463 ([illumos] attempt to use posix_spawn to spawn processes) - rust-lang#137477 (uefi: Add Service Binding Protocol abstraction) - rust-lang#137565 (Try to point of macro expansion from resolver and method errors if it involves macro var) - rust-lang#137569 (Stabilize `string_extend_from_within`) - rust-lang#137612 (Update bootstrap to edition 2024) - rust-lang#137633 (Only use implied bounds hack if bevy, and use deeply normalize in implied bounds hack) - rust-lang#137643 (Add DWARF test case for non-C-like `repr128` enums) - rust-lang#137679 (Various coretests improvements) - rust-lang#137723 (Make `rust.description` more general-purpose and pass `CFG_VER_DESCRIPTION`) - rust-lang#137758 (fix usage of ty decl macro fragments in attributes) - rust-lang#137764 (Ensure that negative auto impls are always applicable) - rust-lang#137772 (Fix char count in `Display` for `ByteStr`) - rust-lang#137798 (ci: use ubuntu 24 on arm large runner) - rust-lang#137805 (adjust Layout debug printing to match the internal field name) - rust-lang#137808 (Do not require that unsafe fields lack drop glue) - rust-lang#137820 (Clarify why InhabitedPredicate::instantiate_opt exists) - rust-lang#137825 (Provide more context on resolve error caused from incorrect RTN) - rust-lang#138028 (compiler: add `ExternAbi::is_rustic_abi`) r? `@ghost` `@rustbot` modify labels: rollup
bors
added a commit
to rust-lang-ci/rust
that referenced
this pull request
Mar 5, 2025
Rollup of 20 pull requests Successful merges: - rust-lang#134063 (dec2flt: Clean up float parsing modules) - rust-lang#136581 (Retire the legacy `Makefile`-based `run-make` test infra) - rust-lang#136662 (Count char width at most once in `Formatter::pad`) - rust-lang#136764 (Make `ptr_cast_add_auto_to_object` lint into hard error) - rust-lang#136798 (Added documentation for flushing per rust-lang#74348) - rust-lang#136865 (Perform deeper compiletest path normalization for `$TEST_BUILD_DIR` to account for compare-mode/debugger cases, and normalize long type file filename hashes) - rust-lang#136975 (Look for `python3` first on MacOS, not `py`) - rust-lang#136977 (Upload Datadog metrics with citool) - rust-lang#137240 (Slightly reformat `std::fs::remove_dir_all` error docs) - rust-lang#137298 (Check signature WF when lowering MIR body) - rust-lang#137463 ([illumos] attempt to use posix_spawn to spawn processes) - rust-lang#137477 (uefi: Add Service Binding Protocol abstraction) - rust-lang#137569 (Stabilize `string_extend_from_within`) - rust-lang#137633 (Only use implied bounds hack if bevy, and use deeply normalize in implied bounds hack) - rust-lang#137679 (Various coretests improvements) - rust-lang#137723 (Make `rust.description` more general-purpose and pass `CFG_VER_DESCRIPTION`) - rust-lang#137728 (Remove unsizing coercions for tuples) - rust-lang#137731 (Resume one waiter at once in deadlock handler) - rust-lang#137875 (mir_build: Integrate "simplification" steps into match-pair-tree creation) - rust-lang#138028 (compiler: add `ExternAbi::is_rustic_abi`) r? `@ghost` `@rustbot` modify labels: rollup
rust-timer
added a commit
to rust-lang-ci/rust
that referenced
this pull request
Mar 5, 2025
Rollup merge of rust-lang#134063 - tgross35:dec2flt-refactoring, r=Noratrieb dec2flt: Clean up float parsing modules This is the first portion of my work adding support for parsing and printing `f16`. Changes in `float.rs` replace the magic constants with expressions and add some use of generics to better support the new float types. Everything else is related to documentation or naming; there are no functional changes in this PR. This can be reviewed by commit.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This is the first portion of my work adding support for parsing and printing
f16. Changes infloat.rsreplace the magic constants with expressions and add some use of generics to better support the new float types. Everything else is related to documentation or naming; there are no functional changes in this PR.This can be reviewed by commit.