-
Notifications
You must be signed in to change notification settings - Fork 688
Implement Date constructor. #322
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
Implement Date constructor. #322
Conversation
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Space before =
c2e2472 to
225d533
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should this be ecma_get_number_from_value ? As the parse_res_value is not a completion value.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why is parse_res_value not a completion value?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I missed that. I'll fix it
225d533 to
63a8efe
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
JERRY_ASSERT?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ruben-ayrapetyan, sorry, I don't get it. What do you mean exactly? What would you like to test with the assert?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@LaszloLango, I mean that ret_value can be either empty or contain throw value just before the if block. Here we handle empty case in if and throw in else if. As there should be no other cases, we could just assert this, replacing else if with else and adding JERRY_ASSERT with contents of current else if condition to the else block.
Like the following:
if (ecma_is_completion_value_empty (ret_value))
{
...
}
else
{
JERRY_ASSERT (ecma_is_completion_value_throw (ret_value));
...
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ruben-ayrapetyan, I see. Thanks for explain. I'll do it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@LaszloLango, thank you.
63a8efe to
3d49ab7
Compare
tests/jerry/date-construct.js
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We usually test it this way:
assert (e instanceof Error);
assert (e.message === "foo");but this is also ok imho.
3d49ab7 to
475a7f9
Compare
|
@galpeter, @ruben-ayrapetyan, I've updated the PR. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can use ECMA_SET_NON_NULL_POINTER in the case, as prim_value_num_p is not NULL.
|
Looks good to me |
|
|
|
lgtm |
475a7f9 to
0af000c
Compare
JerryScript-DCO-1.0-Signed-off-by: László Langó llango.u-szeged@partner.samsung.com
0af000c to
c12914c
Compare
JerryScript-DCO-1.0-Signed-off-by: László Langó llango.u-szeged@partner.samsung.com