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

XS returns the wrong result for expressions that assign to array length #1123

Closed
gibson042 opened this issue May 26, 2023 · 2 comments
Closed
Labels
confirmed issue reported has been reproduced

Comments

@gibson042
Copy link

Environment: XS 13.0.0 32 4

Description
Evaluating array.length = nonNumber does not conform with the ECMAScript specification.

Steps to Reproduce

typeof ([].length = "4")

Actual behavior
"number"

Expected behavior
"string"

Other information
Evaluation of AssignmentExpression : LeftHandSideExpression `=` AssignmentExpression when |LeftHandSideExpression| is not an object or array literal and the right-hand-side |AssignmentExpression| is not a function definition is specified in step 1.e to return the result of evaluating that |AssignmentExpression| in step 1.c.

Basically, even though the intervening PutValue can perform arbitrary processing on that value (as is the case when setting the length of an array ends up in ArraySetLength and the value is coerced to a uint32 number), the assignment expression itself must evaluate to the input of that process rather than its output.

Note also that this affects all assignment operators, e.g. Array(1).length += "e2" should be the string "1e2".

@phoddie
Copy link
Collaborator

phoddie commented May 26, 2023

Thanks for the report.

XS is very deliberate about returning a number from the Array length setter. Here's the relevant code:

mxResult->value.number = array->value.array.length;
mxResult->kind = XS_NUMBER_KIND;

Changing that to return the input argument gives the expected result for the two test cases you provide:

mxResult->value = mxArgv(0)->value;
mxResult->kind = mxArgv(0)->kind;

But, we'll need to take a more careful look.

@phoddie phoddie added the confirmed issue reported has been reproduced label May 26, 2023
mkellner pushed a commit that referenced this issue Jun 2, 2023
@phoddie
Copy link
Collaborator

phoddie commented Jun 5, 2023

Fixed

@phoddie phoddie closed this as completed Jun 5, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
confirmed issue reported has been reproduced
Projects
None yet
Development

No branches or pull requests

2 participants