dec2flt: Clean up float parsing modules #134063
Open
+755
−587
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
These constants can be useful outside of their current module. Make them `pub(crate)` to allow for this.
rustbot
added
S-waiting-on-review
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.
tgross35
force-pushed
the
dec2flt-refactoring
branch
from
c2f95c2 to
3fa1f00
Compare
tgross35
changed the title
tgross35
force-pushed
the
dec2flt-refactoring
branch
2 times, most recently
from
e4cb494 to
058be88
Compare
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
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
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.