Skip to content
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

Array.prototype.join() with this === arg1 #1043

Closed
AidPaike opened this issue Jun 30, 2023 · 6 comments
Closed

Array.prototype.join() with this === arg1 #1043

AidPaike opened this issue Jun 30, 2023 · 6 comments
Labels
enhancement New feature or request need more info Awating additional info before proceeding

Comments

@AidPaike
Copy link

AidPaike commented Jun 30, 2023

Version

Version: 0.12.0 ( 265d0d9 )

Execution steps

/root/hermes_workingdir/build/bin/hermes Testcase.js

Test case 1

var a = [0,1]
var b = Array.prototype.join.call(a, a);
print(a)
print(b)

Output

0,1
01

Test case 2

var a = [0,1]
var b = Array.prototype.join.call(a, [0,1]);
print(b)

Output

00,11

Expected behavior

00,11

Test case 3

var a = [0,1]
var c = [0,1,2,3]
var b = Array.prototype.join.call(a, c);
// print(a)
print(b)

Output

00,1,2,31

Description

When using the Array.prototype.join.call() method, it follows the standard behavior and returns properly when passed arguments such as lists or numbers (Test case 1). However, it does not return properly when passed the arguments variable (Test case 2). To verify if other variables can return properly (Test case 3 is a good example), I suspect it may be related to the "this" context.

Hope hermes gets better and better
Looking forward to your reply :)

@AidPaike AidPaike added the enhancement New feature or request label Jun 30, 2023
@tmikov
Copy link
Contributor

tmikov commented Jun 30, 2023

So, to clarify, the problem you are reporting is that Array.prototype.join(this, arg1) is returning a different result when this and arg1 are the same array.

var a = [0,1]
print(Array.prototype.join.call(a, a));
print(Array.prototype.join.call([0,1], [0,1]));

v8 outputs

00,11
00,11

while Hermes, JSC and SpiderMonkey (JSC and SM from jsvu) produce

01
00,11

I actually don't know off the top of my head what the correct behavior is, but this is related to cycle avoidance.

@tmikov tmikov added the need more info Awating additional info before proceeding label Jun 30, 2023
@tmikov
Copy link
Contributor

tmikov commented Jul 3, 2023

Minimal repro:

var a = [0,1]
print(a.join(a));

@tmikov tmikov changed the title A Array.prototype.join.call error bug Array.prototype.join() with this === arg1 Jul 3, 2023
@AidPaike
Copy link
Author

AidPaike commented Jul 3, 2023

Thank you for your reminder, it may be related to cycle avoidance. Please refer to A.p.join on cyclic arrays does not reflect web reality for more information. It's about 'check for cycles' :)

@tmikov
Copy link
Contributor

tmikov commented Jul 3, 2023

@AidPaike I am sorry, what is your desired outcome here? Do you believe that this is a bug, a violation of EcmaScript, or is this a practical problem that you are having in existing software?

Given the esoteric nature of this case, and that both SM and JSC seem to share the Hermes behavior, and seeing that the linked tc39 issue is still open, I don't think there is a strong case for changing Hermes yet.

However I am happy to discuss it further.

@ljharb
Copy link

ljharb commented Jul 3, 2023

Indeed, until that ecma262 issue is resolved, implementations shouldn't do anything.

@AidPaike
Copy link
Author

AidPaike commented Jul 3, 2023

@AidPaike I am sorry, what is your desired outcome here? Do you believe that this is a bug, a violation of EcmaScript, or is this a practical problem that you are having in existing software?

Given the esoteric nature of this case, and that both SM and JSC seem to share the Hermes behavior, and seeing that the linked tc39 issue is still open, I don't think there is a strong case for changing Hermes yet.

However I am happy to discuss it further.

During my initial journey in learning JavaScript syntax, I experimented with using the argument itself as a delimiter. However, I noticed that the engine automatically removed the delimiter from the returned result, which sparked my interest. Initially, I mistakenly assumed it was a bug, but thanks to your guidance, I discovered that this is a topic also being discussed in ecma262. I am delighted to have been able to explore such depths while still in the early stages of learning JS. Thank you very much for your assistance; I no longer have any doubts.

@tmikov tmikov closed this as completed Jul 3, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request need more info Awating additional info before proceeding
Projects
None yet
Development

No branches or pull requests

3 participants