f64 for octal binary hexadecimal #128
Conversation
2 similar comments
| @@ -331,7 +331,7 @@ fn hexadecimal_mapper(span: Span) -> StdResult<Literal, ParseIntError> { | |||
| ) | |||
| .and_then( | |||
| |hexadecimal| { | |||
| Ok(Literal::Integer(Token::new(hexadecimal, span))) | |||
| Ok(Literal::Real(Token::new(hexadecimal as f64, span))) | |||
There was a problem hiding this comment.
I understand what you did here. Excuse me if it was not clear in #44, but the goal is that it must overflow to a real.
When the hexadecimal value does not fit in i64 (Literal::Integer), it must be then be a f64 (Literal::Real).
Moving everything to real is not optimized, and can complicate things later.
There was a problem hiding this comment.
Aaaah ok, I understand, more stuff like decimal rules ?
i64::from_str(string)
.and_then(...)
.or_else(
|_: ParseIntError| {
f64::from_str(string).and_then(...)
}There was a problem hiding this comment.
Correct. But that's not trivial. In Zend Engine, this conversion from hexa to real is wrong most of the time, and the specification says nothing about that. So that's not a trivial things to do. Maybe there is a crate on crates.io to help us.
There was a problem hiding this comment.
I forget that => bug-compliance that's wright ? Maybe I delete this PR cause more complicated than expected
There was a problem hiding this comment.
I haven't decided yet if we should be bug-compliant here or not. I guess not, and then I would have to fix Zend Engine. That's the best we can do. But until that, yes, if this task is too difficult, feel free to close this PR :-). Would you like another one?
I begin Rust so all comments is welcome.
I don't know if this is like you think this commit.
this PR try to close #44 Issues
Ps : Just thinking about that when I read code, but why Integer is i64 and not i32 ?