Skip to content

Conversation

@rerobika
Copy link
Member

This patch fixes #2724.

JerryScript-DCO-1.0-Signed-off-by: Robert Fancsik frobert@inf.u-szeged.hu

…e_join

This patch fixes jerryscript-project#2724.

JerryScript-DCO-1.0-Signed-off-by: Robert Fancsik frobert@inf.u-szeged.hu
@rerobika rerobika requested a review from galpeter January 22, 2019 20:09
@rerobika rerobika added bug Undesired behaviour ecma builtins Related to ECMA built-in routines labels Jan 22, 2019
@rerobika rerobika requested a review from akosthekiss January 22, 2019 20:17
Copy link
Member

@akosthekiss akosthekiss left a comment

Choose a reason for hiding this comment

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

LGTM

}
ecma_free_value (separator_value);

ECMA_OP_TO_NUMBER_FINALIZE (length_number);
Copy link
Contributor

Choose a reason for hiding this comment

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

Something is messed up here. There should be no return statements between ECMA_OP_TO_NUMBER_TRY_CATCH and ECMA_OP_TO_NUMBER_FINALIZE. IMHO this function needs a serious refactoring.

Copy link
Member Author

Choose a reason for hiding this comment

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

Probably it's not the best practice to return between these macros, but in this case all early return statements contains the required ecma_free_values which generally present at the and of the function. I'd also vote for a refactor, but it may worth an other PR, since this PR's aim to fix the reported issue only.

Copy link
Contributor

@LaszloLango LaszloLango left a comment

Choose a reason for hiding this comment

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

LGTM

@akosthekiss akosthekiss merged commit 924f4bb into jerryscript-project:master Jan 25, 2019
rerobika added a commit to rerobika/jerryscript that referenced this pull request Jan 31, 2019
…e_join (jerryscript-project#2726)

This patch fixes jerryscript-project#2724.

JerryScript-DCO-1.0-Signed-off-by: Robert Fancsik frobert@inf.u-szeged.hu
@rerobika rerobika deleted the fix_issue_2724 branch February 28, 2019 11:11
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.

Assertion 'ECMA_STRING_IS_REF_EQUALS_TO_ONE (string_p)' failed in ecma_free_string_list

3 participants