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

Implement String.prototype.match #443

Closed

Conversation

galpeter
Copy link
Contributor

Added implementation for the String.prototype.match method.

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

@galpeter galpeter added the ecma builtins Related to ECMA built-in routines label Jul 21, 2015
@galpeter galpeter added this to the ECMA builtins milestone Jul 21, 2015
@ILyoan ILyoan mentioned this pull request Jul 21, 2015
25 tasks
ecma_op_to_string (this_arg),
ret_value);

ecma_value_t regexp_value = 0;
Copy link
Contributor

Choose a reason for hiding this comment

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

0 is not a valid value for ecma_value_t.

Could you, please, change this to ecma_make_simple_value (ECMA_SIMPLE_VALUE_EMPTY) and check below to !ecma_is_value_empty?

Also, maybe, the initialization and check could be removed, as compiler checks for accessing uninitialized values.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I did the initialization because the compiler gave me the warning that it found a case where it is not initialized when it gets to the 385 line.

Copy link
Contributor

Choose a reason for hiding this comment

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

I see. Thanks for explanation.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

however, it's to that the use of the empty simple value would be better.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think so too. The update would be necessary, now or in future, when type checking for ecma-values would be improved.

@galpeter
Copy link
Contributor Author

PR updated.

ecma_op_object_get (regexp_obj_p, global_string_p),
ret_value);

JERRY_ASSERT (ecma_is_completion_value_normal_true (global_value)
Copy link
Contributor

Choose a reason for hiding this comment

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

global_value is not completion value. ecma_is_value_boolean could be used in the case.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

darn, forgot that one

Added implementation for the String.prototype.match method.

JerryScript-DCO-1.0-Signed-off-by: Peter Gal pgal.u-szeged@partner.samsung.com
@galpeter
Copy link
Contributor Author

PR updated again :)

@ruben-ayrapetyan
Copy link
Contributor

Looks good to me

@dbatyai
Copy link
Member

dbatyai commented Jul 22, 2015

lgtm

@dbatyai dbatyai assigned egavrin and unassigned dbatyai Jul 22, 2015
@galpeter
Copy link
Contributor Author

Landed: 11c3103

@galpeter galpeter closed this Jul 23, 2015
@egavrin
Copy link
Contributor

egavrin commented Jul 23, 2015

Merged 11c3103

@egavrin egavrin reopened this Jul 23, 2015
@LaszloLango
Copy link
Contributor

@egavrin, why is this reopened?

@egavrin egavrin closed this Jul 24, 2015
@egavrin
Copy link
Contributor

egavrin commented Jul 24, 2015

@LaszloLango accident

@LaszloLango
Copy link
Contributor

@egavrin, ok :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ecma builtins Related to ECMA built-in routines
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants