Global object unescape routine #606

szledan · 2015-08-28T13:16:11Z

JerryScript-DCO-1.0-Signed-off-by: Szilard Ledan szledan.u-szeged@partner.samsung.com

ruben-ayrapetyan · 2015-08-28T13:33:10Z

jerry-core/ecma/builtin-objects/ecma-builtin-global.cpp

+
+  /* 3. */
+  MEM_DEFINE_LOCAL_ARRAY (input_start_p, input_size, lit_utf8_byte_t);
+  ecma_string_to_utf8_string (input_string_p, input_start_p, (ssize_t) (input_size));


Could you, please, add assertion that the return value is not negative, as in 7064a2c?

szledan · 2015-08-28T14:27:26Z

I've updated the patch

ruben-ayrapetyan · 2015-08-28T14:28:50Z

jerry-core/ecma/builtin-objects/ecma-builtin-global.cpp

+  /* 3. */
+  MEM_DEFINE_LOCAL_ARRAY (input_start_p, input_size, lit_utf8_byte_t);
+  ssize_t sz = ecma_string_to_utf8_string (input_string_p, input_start_p, (ssize_t) (input_size));
+  JERRY_ASSERT (sz > 0);


The string's size can be zero.

zherczeg · 2015-08-28T15:53:36Z

Please try unescape("%ud800\udc00") === "\ud800\udc00"

ruben-ayrapetyan · 2015-08-28T16:05:53Z

jerry-core/ecma/builtin-objects/ecma-builtin-global.cpp

+
+    lit_utf8_size_t lit_size = lit_code_point_to_utf8 (code_point, output_char_p);
+    output_char_p += lit_size;
+    output_length += lit_size;


Seems that the output_length is synchronized with output_char_p. In the case, we could remove output_length and calculate it as output_char_p - input_start_p.

zherczeg · 2015-09-03T06:55:04Z

jerry-core/ecma/builtin-objects/ecma-builtin-global.cpp

+  lit_utf8_size_t input_size = ecma_string_get_size (input_string_p);
+
+  /* 3. */
+  MEM_DEFINE_LOCAL_ARRAY (input_start_p, input_size, lit_utf8_byte_t);


Please add a comment why input_size >= output_size, and an assert at the end. I don't think this is trivial.

E.g. %xx is 3 byte long, and the maximum is 0xff, which encoded as 2 bytes in UTF8. Etc.

szledan · 2015-09-07T11:48:39Z

I've updated the patch.
Could you check it please?

zherczeg · 2015-09-07T13:01:11Z

jerry-core/ecma/builtin-objects/ecma-builtin-global.cpp

+  /* The length of input string is always greater than output string
+   * so we re-use the input string buffer.
+   * E.g. %xx is 3 byte long, and the maximum is 0xff, which encoded
+   * as 2 bytes in UTF8. Etc. */


I just suggested an text idea. I think this would sound better:

The %xx is three byte long, and the maximum encoded value is 0xff, which maximum encoded length is two byte. Similar to this, the maximum encoded length of %uxxxx is four byte.

zherczeg · 2015-09-07T13:01:30Z

One minor thing and LGTM after that.

galpeter · 2015-10-02T08:49:27Z

@zherczeg , do we you have other any comments?

zherczeg · 2015-10-02T09:23:35Z

This patch conflits with CESU8. I would wait to land that first, update this, and land it.

egavrin · 2015-10-20T14:39:23Z

@zherczeg CESU8 landed

szledan · 2015-11-02T12:59:42Z

I've updated the patch.
@dbatyai , could you check it please?

dbatyai · 2015-11-02T15:26:10Z

jerry-core/ecma/builtin-objects/ecma-builtin-global.cpp

+   * 8    found valid '%uwxyz' pattern
+   */
+  uint8_t status = 0;
+  lit_code_point_t hex_digits = 0;


It's enough to use an ecma_char_t for hex_digits, the decoded value will always be less than 0xFFFF.

dbatyai · 2015-11-06T12:09:36Z

LGTM

egavrin · 2015-11-09T12:41:02Z

make push

zherczeg · 2015-11-10T07:20:07Z

LGTM

JerryScript-DCO-1.0-Signed-off-by: Szilard Ledan szledan.u-szeged@partner.samsung.com

ruben-ayrapetyan reviewed Aug 28, 2015
View reviewed changes

ruben-ayrapetyan added normal ecma builtins Related to ECMA built-in routines development Feature implementation labels Aug 28, 2015

ruben-ayrapetyan added this to the ECMA builtins milestone Aug 28, 2015

szledan force-pushed the global-unescape branch from dad3532 to fb045a1 Compare August 28, 2015 14:26

ruben-ayrapetyan reviewed Aug 28, 2015
View reviewed changes

szledan force-pushed the global-unescape branch from fb045a1 to 5e287fa Compare August 28, 2015 15:18

ruben-ayrapetyan reviewed Aug 28, 2015
View reviewed changes

szledan force-pushed the global-unescape branch from 5e287fa to 1824e76 Compare September 1, 2015 08:30

zherczeg reviewed Sep 3, 2015
View reviewed changes

szledan force-pushed the global-unescape branch from 1824e76 to fe63e79 Compare September 7, 2015 09:14

zherczeg reviewed Sep 7, 2015
View reviewed changes

sand1k force-pushed the master branch from 5a09ff2 to a26c454 Compare September 7, 2015 15:47

szledan force-pushed the global-unescape branch 2 times, most recently from f0f0e4a to b2ede9d Compare September 9, 2015 08:06

egavrin modified the milestones: Maintenance of ECMA functionality, ECMA builtins Oct 20, 2015

LaszloLango assigned szledan Oct 21, 2015

szledan force-pushed the global-unescape branch 2 times, most recently from 664e4de to 8d6dabe Compare November 2, 2015 12:54

egavrin assigned dbatyai and unassigned szledan Nov 2, 2015

dbatyai reviewed Nov 2, 2015
View reviewed changes

szledan force-pushed the global-unescape branch from 8d6dabe to 494c5ad Compare November 4, 2015 16:32

dbatyai assigned egavrin and unassigned dbatyai Nov 6, 2015

egavrin assigned zherczeg and unassigned egavrin Nov 9, 2015

egavrin assigned szledan and unassigned zherczeg Nov 10, 2015

szledan force-pushed the global-unescape branch from 494c5ad to 846bfb0 Compare November 11, 2015 09:38

Global object unescape routine

36e90d9

JerryScript-DCO-1.0-Signed-off-by: Szilard Ledan szledan.u-szeged@partner.samsung.com

szledan force-pushed the global-unescape branch from 846bfb0 to 36e90d9 Compare November 11, 2015 09:49

dbatyai merged commit 36e90d9 into jerryscript-project:master Nov 11, 2015

zherczeg mentioned this pull request Jul 20, 2016

Nominating Dániel Bátyai (dbatyai) for JerryScript Maintainer status #1220

Closed

Global object unescape routine #606

Global object unescape routine #606

Uh oh!

Conversation

szledan commented Aug 28, 2015

Uh oh!

ruben-ayrapetyan Aug 28, 2015

Choose a reason for hiding this comment

Uh oh!

szledan commented Aug 28, 2015

Uh oh!

ruben-ayrapetyan Aug 28, 2015

Choose a reason for hiding this comment

Uh oh!

zherczeg commented Aug 28, 2015

Uh oh!

ruben-ayrapetyan Aug 28, 2015

Choose a reason for hiding this comment

Uh oh!

zherczeg Sep 3, 2015

Choose a reason for hiding this comment

Uh oh!

szledan commented Sep 7, 2015

Uh oh!

zherczeg Sep 7, 2015

Choose a reason for hiding this comment

Uh oh!

zherczeg commented Sep 7, 2015

Uh oh!

galpeter commented Oct 2, 2015

Uh oh!

zherczeg commented Oct 2, 2015

Uh oh!

egavrin commented Oct 20, 2015

Uh oh!

szledan commented Nov 2, 2015

Uh oh!

dbatyai Nov 2, 2015

Choose a reason for hiding this comment

Uh oh!

dbatyai commented Nov 6, 2015

Uh oh!

egavrin commented Nov 9, 2015

Uh oh!

zherczeg commented Nov 10, 2015

Uh oh!

Uh oh!