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.

Sign up for GitHub

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

parser: implement NaN and Infinity parsing #201

Closed
wants to merge 1 commit into from
Closed

parser: implement NaN and Infinity parsing #201

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
62 changes: 42 additions & 20 deletions src/jsrs_parser.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
#include "jsrs_parser.h"

#include <cctype>
#include <cmath>
#include <cstddef>
#include <cstdlib>
#include <cstring>
Expand All @@ -18,6 +19,8 @@ using std::function;
using std::isalnum;
using std::isalpha;
using std::isdigit;
using std::isinf;
using std::isnan;
using std::isxdigit;
using std::memset;
using std::ptrdiff_t;
Expand Down Expand Up @@ -150,6 +153,11 @@ static bool GetType(const char* begin, const char* end, Type* type) {
}
break;
}
case 'N':
case 'I': {
*type = Type::kNumber;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure about this for the reasons @lundibundi has mentioned...

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@aqrln, why'd you put an ellipsis in the end of your comment?
Also, the problem stated by @lundibundi has nothing to do with this line, it is just incorrect usage of the strtol function, and I will fix this.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oops, I am sorry, I was really talking about atof in the previous comment, not strtol.
strtod has to be used instead of it.

break;
}
default: {
result = false;
if (isdigit(*begin) || *begin == '.' || *begin == '+' || *begin == '-') {
Expand Down Expand Up @@ -318,32 +326,46 @@ MaybeLocal<Value> ParseNumber(Isolate* isolate,
}
}

MaybeLocal<Value> result;

if (base == 10) {
return ParseDecimalNumber(isolate, begin, end, size);
result = ParseDecimalNumber(isolate, number_start, end, size,
negate_result);
} else {
auto value = ParseIntegerNumber(isolate, number_start, end, size,
base, negate_result);
auto offset = static_cast<size_t>(number_start - begin);
*size += offset;
return value;
result = ParseIntegerNumber(isolate, number_start, end, size,
base, negate_result);
}
*size += number_start - begin;
return result;
}

Local<Value> ParseDecimalNumber(Isolate* isolate,
const char* begin,
const char* end,
size_t* size) {
auto result = Number::New(isolate, atof(begin));
*size = end - begin;
size_t i = 0;
while (begin[i] != ',' &&
begin[i] != '}' &&
begin[i] != ']' &&
i < *size) {
i++;
MaybeLocal<Value> ParseDecimalNumber(Isolate* isolate,
const char* begin,
const char* end,
size_t* size,
bool negate_result) {
char* number_end;
double number = strtod(begin, &number_end);

if (negate_result) {
number = -number;
}
*size = i;
return result;

// strictly allow only "NaN" and "Infinity"
if (isnan(number)) {
if (strncmp(begin + 1, "aN", 2) != 0) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

wow, such a pain =(

Copy link
Member Author

@belochub belochub Jun 3, 2017 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's so painful here? I just avoided testing for the same character twice here, that's it.

Copy link
Member

@lundibundi lundibundi Jun 3, 2017 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The need to actually check this. But anyway I see no other way.

THROW_EXCEPTION(SyntaxError, "Invalid format: expected NaN");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's better to say 'Invalid number format: ...'.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wrote it like that to be consistent with the other error messages, it still isn't perfect everywhere, but hopefully I will fix it someday (see #69).

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What I wanted to say is that it is an issue for another PR which will improve error reporting in the native parser.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@belochub Ok, let's hope it will be done eventually =)

return MaybeLocal<Value>();
}
} else if (isinf(number)) {
if (strncmp(begin + 1, "nfinity", 7) != 0) {
THROW_EXCEPTION(SyntaxError, "Invalid format: expected Infinity");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ditto.

return MaybeLocal<Value>();
}
}

*size = number_end - begin;
return Number::New(isolate, number);
}

Local<Value> ParseIntegerNumber(Isolate* isolate,
Expand Down
9 changes: 5 additions & 4 deletions src/jsrs_parser.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -104,10 +104,11 @@ v8::MaybeLocal<v8::Value> ParseObject(v8::Isolate* isolate,
std::size_t* size);

// Parses a decimal number, either integer or float.
v8::Local<v8::Value> ParseDecimalNumber(v8::Isolate* isolate,
const char* begin,
const char* end,
std::size_t* size);
v8::MaybeLocal<v8::Value> ParseDecimalNumber(v8::Isolate* isolate,
const char* begin,
const char* end,
std::size_t* size,
bool negate_result);

// Parses an integer number in arbitrary base without prefixes.
v8::Local<v8::Value> ParseIntegerNumber(v8::Isolate* isolate,
Expand Down