Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[BUG] Inconsistent error between int and float parsing #97

Closed
dubrowgn opened this issue May 29, 2023 · 1 comment · Fixed by #156 or #161
Closed

[BUG] Inconsistent error between int and float parsing #97

dubrowgn opened this issue May 29, 2023 · 1 comment · Fixed by #156 or #161
Assignees
Labels
bug Something isn't working normal priority Normal Priority
Milestone

Comments

@dubrowgn
Copy link

Description

While I was writing unit tests to ensure leading and trailing digit separators fail to parse in my program, I noticed a leading separator for floats returns a different error code than the other 3 cases. Instead of InvalidDigit, it returns EmptyMantissa. For the given input _12.34, the mantissa isn't actually empty, but (I assume) the parser never actually gets to parsing it because of the invalid leading separator. While this is a relatively minor issue (the number fails to parse either way), I believe returning InvalidDigit is more accurate and consistent, and thus more useful to users.

Prerequisites

  • Rust version: rustc 1.68.0 (2c8cc3432 2023-03-06)
  • lexical version:
    lexical-util v0.8.5
    lexical-parse-integer v0.8.6
    lexical-parse-float v0.8.5
    lexical-core v0.8.5
    lexical v6.1.1
  • Cargo.toml
[dependencies.lexical]
default-features = false
features = [ "parse-floats", "parse-integers" ]
version = "6.x"

[dependencies.lexical-parse-integer]
default-features = false
features = [ "format", "power-of-two" ]
version = "x"

[dependencies.lexical-parse-float]
default-features = false
features = [ "format", "power-of-two" ]
version = "x"

Test case

let fopts = lexical::ParseFloatOptions::new();
let iopts = lexical::ParseIntegerOptions::new();
const FMT: u128 = 	NumberFormatBuilder::new()
	.digit_separator(num::NonZeroU8::new(b'_'))
	.build();

println!("int");
println!("{:?}", lexical::parse_with_options::<i64, _, FMT>("_1234", &iopts));
println!("{:?}", lexical::parse_with_options::<i64, _, FMT>("1234_", &iopts));

println!("float");
println!("{:?}", lexical::parse_with_options::<f64, _, FMT>("_12.34", &fopts));
println!("{:?}", lexical::parse_with_options::<f64, _, FMT>("12.34_", &fopts));

// int
// Err(InvalidDigit(0))
// Err(InvalidDigit(4))

// float
// Err(EmptyMantissa(0))
// Err(InvalidDigit(5))
@dubrowgn dubrowgn added the bug Something isn't working label May 29, 2023
@Alexhuszagh Alexhuszagh added the normal priority Normal Priority label Sep 9, 2024
@Alexhuszagh Alexhuszagh added this to the 1.0.2 milestone Sep 19, 2024
@Alexhuszagh
Copy link
Owner

This also affects when the format feature isn't enabled, luckily this should be a pretty trivial patch.

Alexhuszagh added a commit that referenced this issue Sep 22, 2024
This addressed #96 and #97, fixing the lack of processing with
consecutive digit separators by enhancing the internal logic, adds logic
for internal and first digit separators to simplify logic and improve
performance, fix unittests, and also make it so the errors are
consistent by adding checks when formatting is enabled to ensure the
correct logic is used.

Closes #96
Closes #97
Alexhuszagh added a commit that referenced this issue Sep 24, 2024
This addressed #96 and #97, fixing the lack of processing with
consecutive digit separators by enhancing the internal logic, adds logic
for internal and first digit separators to simplify logic and improve
performance, fix unittests, and also make it so the errors are
consistent by adding checks when formatting is enabled to ensure the
correct logic is used.

Closes #96
Closes #97
Alexhuszagh added a commit that referenced this issue Sep 24, 2024
This addressed #96 and #97, fixing the lack of processing with
consecutive digit separators by enhancing the internal logic, adds logic
for internal and first digit separators to simplify logic and improve
performance, fix unittests, and also make it so the errors are
consistent by adding checks when formatting is enabled to ensure the
correct logic is used.

Closes #96
Closes #97
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working normal priority Normal Priority
Projects
None yet
2 participants