Implement computed properties for object literals. #2481

zherczeg · 2018-08-21T14:29:58Z

No description provided.

zherczeg · 2018-08-22T11:35:05Z

Patch is done, and ready for review.

LaszloLango

Please update the snapshot version

LaszloLango · 2018-08-22T12:21:44Z

jerry-core/vm/vm.h

@@ -121,6 +121,9 @@ typedef enum
  VM_OC_PUSH_LIT_NEG_BYTE,       /**< push literal and number between -1 and -256 */
  VM_OC_PUSH_OBJECT,             /**< push object */
  VM_OC_SET_PROPERTY,            /**< set property */
+#ifndef CONFIG_DISABLE_ES2015_OBJECT_LITERAL


Please do not use #ifndef in this enum. It can cause errors in snapshot execution with different build configurations.

This is reworked in the execution engine. Now the byte codes are translated to a valid VM opcode or a fatal error. This is much safer now, since we at least get an error rather than a random crash if we execute a snapshot with invalid byte codes. Normally this should be detected early, but we have a safety net now.

zherczeg · 2018-08-22T12:43:49Z

Come to think of snapshots, I also increased the snapshot version number.

LaszloLango

LGTM

rerobika · 2018-08-22T19:15:27Z

tests/jerry/es2015/object-computed.js

+  [fxy()] () {
+    return 6;
+  }
+


Please add test cases for:

class C { ["const" + "ructor"] { // 1. this.a = 5; } get ["constructor"]() { // 2. return this.a; } static ["prototype"] () { // 3. return 10; } }

In the first case it must be a normal class method instead of the real constructor of the class.

The second one is a bit similar. In normal case the get constructor() {} is not allowed for classes (the real constructor cannot be an accessor), but as a computed property name it must be a simple accessor with constructor name.

Finally, the third one must throw a TypeError in runtime as the same way as static prototype () {} throws a SyntaxError during the script parsing.

rerobika · 2018-08-22T19:23:13Z

jerry-core/vm/vm.c

+
+            if (ECMA_IS_VALUE_ERROR (result))
+            {
+              goto error;


If the result is an error, should not the left_value be freed?

The error handling automatically frees both left and right values.

OK, thanks for clarifying it.

rerobika · 2018-08-22T19:36:54Z

jerry-core/config.h

@@ -39,6 +39,7 @@
 # define CONFIG_DISABLE_ES2015_ARROW_FUNCTION
 # define CONFIG_DISABLE_ES2015_CLASS
 # define CONFIG_DISABLE_ES2015_BUILTIN
+# define CONFIG_DISABLE_ES2015_OBJECT_LITERAL


It is a new feature, so please update the profile documentation.

zherczeg · 2018-08-23T07:03:13Z

Thank you for the review. Some bigger changes applied, @LaszloLango you might want to take another look (I changed literal to initializer, because es6 uses this term and reworked property setting in the vm). New tests added as well.

rerobika

LGTM (informal)

LaszloLango · 2018-08-23T07:36:21Z

@zherczeg Ok, I will take a look on the changes.

LaszloLango · 2018-08-24T06:20:34Z

still LGTM

AnthonyCalandra

Left some minor comments.

Thanks for helping out by the way!

AnthonyCalandra · 2018-08-26T23:03:45Z

jerry-core/parser/js/js-parser-expr.c

+
+#ifdef CONFIG_DISABLE_ES2015_OBJECT_INITIALIZER
+#error "Class support requires ES2015 object literal support"
+#endif /* CONFIG_DISABLE_ES2015_OBJECT_INITIALIZER */


If we're making this a requirement, should maybe this code be moved to config.h?

I searched through the code base and only a few #error is there. It seems there is a configuration check in the normal code base:
https://github.com/jerryscript-project/jerryscript/blob/master/jerry-core/jcontext/jcontext.h#L194

But config.h also have a check.
https://github.com/jerryscript-project/jerryscript/blob/master/jerry-core/config.h#L81

The truth is I don't know we have a policy for that. It depends config.h is mandatory, or user supplied. Anybody knows this?

(In most projects I know config.h is auto generated, and contains no dependency checks. In JerryScript config.h is not auto generated.)

AnthonyCalandra · 2018-08-26T23:07:30Z

jerry-core/parser/js/js-parser-expr.c

  while (context_p->stack_top_uint8 != PARSER_OBJECT_PROPERTY_START)
  {
    parser_stack_pop (context_p, NULL, 3);
  }

  parser_stack_pop_uint8 (context_p);
+#endif /* !CONFIG_DISABLE_ES2015_OBJECT_INITIALIZER */


Shouldn't this be /* CONFIG_DISABLE_ES2015_OBJECT_INITIALIZER */?

Also disable ES5.1 property name dumplication checks when ES2015 object literals are enabled. JerryScript-DCO-1.0-Signed-off-by: Zoltan Herczeg [email protected]

robertsipka

LGTM

Picked from upstream jerryscript-project/jerryscript#2481

zherczeg mentioned this pull request Aug 21, 2018

[WIP] Support for shorthand object literal notation. #2432

Closed

zherczeg force-pushed the computed_props branch 3 times, most recently from 559180f to 5b13d44 Compare August 22, 2018 09:51

LaszloLango requested changes Aug 22, 2018

View reviewed changes

zherczeg force-pushed the computed_props branch from 5b13d44 to 95943e7 Compare August 22, 2018 12:52

LaszloLango approved these changes Aug 22, 2018

View reviewed changes

rerobika requested changes Aug 22, 2018

View reviewed changes

zherczeg force-pushed the computed_props branch from 95943e7 to ef88df9 Compare August 23, 2018 07:01

rerobika approved these changes Aug 23, 2018

View reviewed changes

zherczeg force-pushed the computed_props branch from ef88df9 to f2d469e Compare August 23, 2018 09:25

AnthonyCalandra approved these changes Aug 26, 2018

View reviewed changes

Implement computed properties for object literals.

ec4f18c

Also disable ES5.1 property name dumplication checks when ES2015 object literals are enabled. JerryScript-DCO-1.0-Signed-off-by: Zoltan Herczeg [email protected]

zherczeg force-pushed the computed_props branch from f2d469e to ec4f18c Compare August 27, 2018 07:09

akosthekiss added the ES2015 Related to ES2015 features label Aug 28, 2018

robertsipka approved these changes Aug 28, 2018

View reviewed changes

zherczeg merged commit bd42403 into jerryscript-project:master Aug 28, 2018

zherczeg deleted the computed_props branch August 28, 2018 13:28

legendecas added a commit to yodaos-project/ShadowNode that referenced this pull request Nov 29, 2018

jerry: implement computed properties for object literals

26a465f

Picked from upstream jerryscript-project/jerryscript#2481

legendecas added a commit to yodaos-project/ShadowNode that referenced this pull request Nov 29, 2018

jerry: implement computed properties for object literals

6bb5805

Picked from upstream jerryscript-project/jerryscript#2481

legendecas added a commit to yodaos-project/ShadowNode that referenced this pull request Nov 29, 2018

jerry: implement computed properties for object literals

37cdb7e

Picked from upstream jerryscript-project/jerryscript#2481

zherczeg mentioned this pull request Mar 19, 2019

Nominating Robert Fancsik (@rerobika) to become a JerryScript reviewer #2799

Closed

Implement computed properties for object literals. #2481

Implement computed properties for object literals. #2481

Uh oh!

Conversation

zherczeg commented Aug 21, 2018

Uh oh!

zherczeg commented Aug 22, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

LaszloLango left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

zherczeg commented Aug 22, 2018

Uh oh!

LaszloLango left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

zherczeg commented Aug 23, 2018

Uh oh!

rerobika left a comment

Choose a reason for hiding this comment

Uh oh!

LaszloLango commented Aug 23, 2018

Uh oh!

LaszloLango commented Aug 24, 2018

Uh oh!

AnthonyCalandra left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

robertsipka left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

zherczeg commented Aug 22, 2018 •

edited

Loading

AnthonyCalandra left a comment •

edited

Loading