Skip to content

Improvements for ecma_op_general_object_define_own_property #140

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

Merged
merged 1 commit into from
Jun 3, 2015

Conversation

galpeter
Copy link
Contributor

@galpeter galpeter commented Jun 2, 2015

List of improvements:

  • Get the [[Enumerable]] and [[Configurable]] attributes before removing a property.
  • Directly check the type of the property in asserts.

Related issues: #115 and #132

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

@ruben-ayrapetyan ruben-ayrapetyan added bug Undesired behaviour normal ecma core Related to core ECMA functionality labels Jun 2, 2015
@ruben-ayrapetyan ruben-ayrapetyan added this to the Core ECMA features milestone Jun 2, 2015
@ruben-ayrapetyan
Copy link
Contributor

Looks good to me.

@galpeter
Copy link
Contributor Author

galpeter commented Jun 2, 2015

The problem is that this patch by itself would only lead to a different assert mentioned in the #132 issue. I was thinking that I'll add the fix for that also into this PR and then it should pass the testes correctly. What do you think? Can I have a PR which fixes two issues or should I stick with one issue fix per PR?

@ruben-ayrapetyan
Copy link
Contributor

I think, in the case, as the two issues are related to one case in one function (change of current_p property's type), it is ok to put fixes for them into one commit.

@galpeter galpeter changed the title Query property attributes before removing Improvements for ecma_op_general_object_define_own_property Jun 2, 2015
@galpeter
Copy link
Contributor Author

galpeter commented Jun 2, 2015

Ok, I've update the PR containing the fixes for both issues.

@ruben-ayrapetyan
Copy link
Contributor

Changes, related to both issues, look good to me.

@ruben-ayrapetyan ruben-ayrapetyan assigned egavrin and unassigned galpeter Jun 2, 2015
@egavrin
Copy link
Contributor

egavrin commented Jun 2, 2015

make push

List of improvements:
 - Get the [[Enumerable]] and [[Configurable]] attributes before removing a property.
 - Directly check the type of the property in asserts.

Related issues: #115 and #132

JerryScript-DCO-1.0-Signed-off-by: Peter Gal pgal.u-szeged@partner.samsung.com
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Undesired behaviour ecma core Related to core ECMA functionality normal
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants