Skip to content

Conversation

@galpeter
Copy link
Contributor

Fixes #405

JerryScript-DCO-1.0-Signed-off-by: Peter Gal pgal.u-szeged@partner.samsung.com

@galpeter galpeter added bug Undesired behaviour ecma builtins Related to ECMA built-in routines labels Jul 24, 2015
@galpeter galpeter added this to the ECMA builtins milestone Jul 24, 2015
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 we should use an integer type here instead of an ecma_number_t.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

good point, will fix it in the update.

@zherczeg
Copy link
Member

LGTM

Copy link
Member

Choose a reason for hiding this comment

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

Shouldn't we check for eos before doing this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, good catch!

JerryScript-DCO-1.0-Signed-off-by: Peter Gal pgal.u-szeged@partner.samsung.com
@galpeter galpeter force-pushed the unicodify_parseint branch from b7dddd1 to b5e4dc5 Compare July 27, 2015 13:14
@galpeter
Copy link
Contributor Author

PR updated.

Copy link
Member

Choose a reason for hiding this comment

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

There should probably be an eos check here as well.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In this case it is not needed, as the offset check in the if statement above already checked that there are at least two characters, so we are not at the end of the string yet.

Copy link
Member

Choose a reason for hiding this comment

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

Ah, okay.

@dbatyai
Copy link
Member

dbatyai commented Jul 27, 2015

lgtm

@sand1k
Copy link
Contributor

sand1k commented Jul 28, 2015

LGTM.

@egavrin
Copy link
Contributor

egavrin commented Jul 28, 2015

make push

@egavrin egavrin assigned galpeter and unassigned sand1k Jul 28, 2015
@dbatyai
Copy link
Member

dbatyai commented Jul 29, 2015

Landed: 3fbe543

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Undesired behaviour ecma builtins Related to ECMA built-in routines

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants