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

Wait next RapidJSON release for fixes NaN encoding. #11

Closed
xpol opened this issue Jan 29, 2016 · 7 comments
Closed

Wait next RapidJSON release for fixes NaN encoding. #11

xpol opened this issue Jan 29, 2016 · 7 comments

Comments

@xpol
Copy link
Owner

xpol commented Jan 29, 2016

(Moved form #9 (comment))
NaN encoding, i.e. encode(0/0), is not throwing an error (nil + error message).
I expected so according to: Tencent/rapidjson#509

@stepelu
Copy link

stepelu commented Jan 29, 2016

I thought this had been done already in 1f37cec (see "ad1d22e Fix #509 by checking Nan/Inf when writing a double") ?

@xpol
Copy link
Owner Author

xpol commented Jan 29, 2016

Yes it still result 2.696539702293474e308:

LuaJIT 2.0.4 -- Copyright (C) 2005-2015 Mike Pall. http://luajit.org/
JIT: ON CMOV SSE2 SSE3 SSE4.1 fold cse dce fwd dse narrow loop abc sink fuse 
> j = require('rapidjson')
> print(j.encode(0/0))
2.696539702293474e308

@miloyip
Copy link
Contributor

miloyip commented Jan 29, 2016

Strange... BTW, I think it is needed to check the return bool of Double, and raise error in Lua.

@xpol
Copy link
Owner Author

xpol commented Jan 31, 2016

It does not return false when encoding 0/0 in lua.

@miloyip
Copy link
Contributor

miloyip commented Jan 31, 2016

Yes. I have asked GitHub support that recently some PR was merged but the commit was lost in the history.

@miloyip
Copy link
Contributor

miloyip commented Feb 9, 2016

I have got one e-mail but the admin don't think there is problem.
I manually cherry-pick the merge again and it should be in the master.
https://github.com/miloyip/rapidjson/blob/master/include/rapidjson/writer.h#L245

@xpol
Copy link
Owner Author

xpol commented Feb 15, 2016

Updated and tested in commit 7918f5b.

@xpol xpol closed this as completed Feb 15, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants