Skip to content

Support methods for object initializers. #2567

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

Conversation

zherczeg
Copy link
Member

MethodDefinition in ES-2015 12.2.6.

@zherczeg zherczeg force-pushed the obj_init_method_support branch from b94e537 to 6794c5d Compare October 19, 2018 16:40
@@ -633,6 +633,28 @@ parser_parse_class (parser_context_t *context_p, /**< context */
} /* parser_parse_class */
#endif /* !CONFIG_DISABLE_ES2015_CLASS */

#ifndef CONFIG_DISABLE_ES2015_OBJECT_INITIALIZER
/**
* Parse object method.
Copy link
Contributor

Choose a reason for hiding this comment

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

Please add more description, add the related section number as a note to the description.

@@ -633,6 +633,28 @@ parser_parse_class (parser_context_t *context_p, /**< context */
} /* parser_parse_class */
#endif /* !CONFIG_DISABLE_ES2015_CLASS */

#ifndef CONFIG_DISABLE_ES2015_OBJECT_INITIALIZER
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this feature turned on by default in ES5.1 profile?

Copy link
Member Author

@zherczeg zherczeg Oct 25, 2018

Choose a reason for hiding this comment

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

No. It is the opposite. I think the guard is correct.

Copy link
Member

Choose a reason for hiding this comment

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

Please add a description about the new feature to the profile documentation.

Copy link
Member Author

Choose a reason for hiding this comment

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

The CONFIG_DISABLE_ES2015_OBJECT_INITIALIZER is not a new macro, just a new feature is included into it.

Copy link
Member

Choose a reason for hiding this comment

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

I see. Thanks for the clarification.

MethodDefinition in ES-2015 12.2.6.

JerryScript-DCO-1.0-Signed-off-by: Zoltan Herczeg [email protected]
@zherczeg zherczeg force-pushed the obj_init_method_support branch from 6794c5d to 1e36e4f Compare October 26, 2018 07:26
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

Copy link
Member

@rerobika rerobika left a comment

Choose a reason for hiding this comment

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

Great patch, only a minor request.

@@ -633,6 +633,28 @@ parser_parse_class (parser_context_t *context_p, /**< context */
} /* parser_parse_class */
#endif /* !CONFIG_DISABLE_ES2015_CLASS */

#ifndef CONFIG_DISABLE_ES2015_OBJECT_INITIALIZER
Copy link
Member

Choose a reason for hiding this comment

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

Please add a description about the new feature to the profile documentation.

Copy link
Member

@rerobika rerobika left a comment

Choose a reason for hiding this comment

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

LGTM (informal)

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

@akosthekiss akosthekiss merged commit 93567fb into jerryscript-project:master Oct 26, 2018
@zherczeg zherczeg deleted the obj_init_method_support branch October 28, 2018 20:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants