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

Mixing 0x number prefix with float suffix results in unexpected behavior #12662

Closed
mattrberry opened this issue Oct 25, 2022 · 7 comments · Fixed by #12687
Closed

Mixing 0x number prefix with float suffix results in unexpected behavior #12662

mattrberry opened this issue Oct 25, 2022 · 7 comments · Fixed by #12687
Labels
kind:bug A bug in the code. Does not apply to documentation, specs, etc. topic:compiler:parser

Comments

@mattrberry
Copy link
Contributor

➜  ~ crystal eval 'puts 0x00_f32'
3890
➜  ~ crystal --version
Crystal 1.6.1 (2022-10-21)

LLVM: 14.0.6
Default target: aarch64-apple-darwin21.6.0

I'd expect this to simply result in 0, as intuitively this number is "0x00 represented as a 32-bit float".

This seems to only act incorrectly with the 0x prefix and both float sizes. Binary and octal prefixes work as expected, as do integer numbers

@mattrberry mattrberry added the kind:bug A bug in the code. Does not apply to documentation, specs, etc. label Oct 25, 2022
@mattrberry
Copy link
Contributor Author

mattrberry commented Oct 25, 2022

Ah it's effectively parsing it as 0xF32

break unless digit && digit.to_u8! < base

I guess this is a little ambiguous since the _ preceding the suffix isn't required. It could be tricky to distinguish from 0x00F32 which is a totally valid hex literal

@mattrberry mattrberry changed the title Mixing 0x number prefix with floats results in unexpected behavior Mixing 0x number prefix with float suffix results in unexpected behavior Oct 25, 2022
@straight-shoota
Copy link
Member

straight-shoota commented Oct 25, 2022

I'm curious: Why are trying to use hexadecimal notation for floating point numbers?

@mattrberry
Copy link
Contributor Author

I have no real-world use for this. I just stumbled across it while writing a test for something else in the compiler

@straight-shoota
Copy link
Member

I don't think we can do anything about that. The f32 and f64 suffixes are also valid hex digits, so there's no way to tell them apart.
Rust shares the same number literal syntax and it works exactly the same:

fn main() {
    println!("{}", 0x00_f32); // => 3890
}

Hexadecimal notation is very unusual for floating point numbers. That really doesn't make much sense because hex notation (as well as octal and binary) in integers is used to express the byte representation. But that's completely different for floats. When floats are written in hex, that's usually also for the internal representation. But then you can't just use ordinary literals for that.

I checked with Rust and it does not accept octal and binary notation for floating point numbers. Crystal does, though. I think we should remove support for that in Crystal as well. It's not even documented: https://crystal-lang.org/reference/1.6/syntax_and_semantics/literals/floats.html
As a result, it would be more consistent that floating point numbers can only be written in decimal notation. Only integers can be written in other bases (powers of 2).

@HertzDevil
Copy link
Contributor

Hexadecimal notation is very unusual for floating point numbers

Hexfloat literals exist, can be produced by using %a in sprintf, and can be parsed by String#to_f because it relies on LibC.strtof. Unfortunately they can never appear in plain code because the decimal part becomes a method call if it begins with an alphabetical character:

"%a" % 125.0 # => 0x1.f4p+6
0x1.f4p+6    # equivalent to (0x1).f4p().+(6)

To my knowledge, of the mainstream languages only C(++) and Julia support them in plain code.

@straight-shoota
Copy link
Member

@HertzDevil Yeah but this is a specific format for floating point numbers based on their internal representation. That's different from what's actually an integer literal with a _f32 suffix attached.

@mattrberry
Copy link
Contributor Author

Given that Crystal doesn't require a unique character to delineate type suffixes (i.e. 0x0'f32 in Nim), I agree with your solution of removing the binary and octal notation for floats as well to reduce ambiguity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind:bug A bug in the code. Does not apply to documentation, specs, etc. topic:compiler:parser
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants