-
Notifications
You must be signed in to change notification settings - Fork 3.5k
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
Parse<kParseNumbersAsStringsFlag>() raises an error on big integer #1368
Comments
Yes, I think when |
Further study seems to suggest that I was wrong: current master does not exhibit the problem, and by any chance I was tricked by a system include when I compiled the modified simplereader.cpp above (my system's has v1.1.0 rapidjson-dev installed in /usr/include/rapidjson). I installed the following patch in current master: diff --git a/test/unittest/readertest.cpp b/test/unittest/readertest.cpp
index 2deadb79..e3d51481 100644
--- a/test/unittest/readertest.cpp
+++ b/test/unittest/readertest.cpp
@@ -1975,6 +1975,17 @@ TEST(Reader, NumbersAsStrings) {
Reader reader;
EXPECT_TRUE(reader.Parse<kParseNumbersAsStringsFlag>(s, h));
}
+ {
+ char n1e319[321]; // '1' followed by 319 '0'
+ n1e319[0] = '1';
+ for (int i = 1; i < 320; i++)
+ n1e319[i] = '0';
+ n1e319[320] = '\0';
+ StringStream s(n1e319);
+ NumbersAsStringsHandler h(n1e319);
+ Reader reader;
+ EXPECT_TRUE(reader.Parse<kParseNumbersAsStringsFlag>(s, h));
+ }
} and it successfully passes, while it fails with v1.1.0. So I tried to bisecting the problem, and narrowed the fixup between commit 01c7174 (that fails) and commit 6cc3910 (correct). Could not figure out how to spot the exact commit because I was not able to compile most of changes in that range (that is 80dba56..7101911) due to syntax error:
If my analysis is correct, this issue can be closed. Do you think the test above brings any benefit to the coverage? If so, I could submit a PR just for that. |
The fix for this is in f5e5d47, which skips the overflow test in the |
Great, thank you, I was not able to spot the fixing commit... I'll try to propose a PR then! |
Looks like this issue can be closed? |
The following slightly modified simplereader.cpp:
produces the following output:
Why does it try to convert the bigint to a C double, even with the
kParseNumbersAsStringsFlag
flag?Am I missing something or is this an issue?
The text was updated successfully, but these errors were encountered: