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

Lua comma evaluation order issue? tfound[1],num = num, num - 1 breaks on luau sometime after 0.541 #754

Closed
SteveDE opened this issue Nov 22, 2022 · 4 comments · Fixed by #768
Assignees
Labels
bug Something isn't working

Comments

@SteveDE
Copy link

SteveDE commented Nov 22, 2022

We have some binary search copy pasta from here http://lua-users.org/wiki/BinarySearch

It has this pesky line:
tfound[1],num = num, num - 1

In 0.541 this line emits these opcodes, and everything works as it did on stock Lua:

   41:                 tfound[1],num = num, num - 1
41: MOVE R11 R10
41: SUBK R12 R10 K13
41: SETTABLEN R11 R9 1
41: MOVE R10 R12

In 0.554, it emits this, which breaks our search:

   41:                 tfound[1],num = num, num - 1
41: SUBK R10 R10 K13
41: SETTABLEN R10 R9 1

If I remove the comma, it works again. The order is correct and is nicely optimized as 0.554

   39:                 tfound[1] = num
39: SETTABLEN R10 R9 1
   40:                 num = num - 1
40: SUBK R10 R10 K13

I'm ready to believe this is ambiguous or a bug in the original, but I thought I should mention it for compatibility reasons.

@SteveDE SteveDE added the bug Something isn't working label Nov 22, 2022
@vegorov-rbx vegorov-rbx self-assigned this Nov 22, 2022
@vegorov-rbx
Copy link
Collaborator

I can confirm that this is an issue with multiple-assignment in Luau compiler that's related to a not-so-recent optimization that we've added.
We're now working on a fix for this case.

@vegorov-rbx
Copy link
Collaborator

We have a fix ready internally, next release is planned for Dec 2nd.

@SteveDE
Copy link
Author

SteveDE commented Nov 22, 2022

Fast! Thank you very much!

vegorov-rbx added a commit that referenced this issue Dec 2, 2022
* Type mismatch errors now mention if unification failed in covariant or
invariant context, to explain why sometimes derived class can't be
converted to base class or why `T` can't be converted into `T?` and so
on
* Class type indexing is no longer an error in non-strict mode (still an
error in strict mode)
* Fixed cyclic type packs not being displayed in the type
* Added an error when unrelated types are compared with `==`/`~=`
* Fixed false positive errors involving sub-type tests an `never` type
* Fixed miscompilation of multiple assignment statements (Fixes
#754)
* Type inference stability improvements
@SteveDE
Copy link
Author

SteveDE commented Dec 2, 2022

Merged to 0.555 and the byte code generate is unfortunately in the wrong order still

22: tfound[1],num = num,num - 1
22: SUBK R10 R10 K4
22: SETTABLEN R10 R9 1

RomanKhafizianov pushed a commit to RomanKhafizianov/luau that referenced this issue Nov 27, 2023
* Type mismatch errors now mention if unification failed in covariant or
invariant context, to explain why sometimes derived class can't be
converted to base class or why `T` can't be converted into `T?` and so
on
* Class type indexing is no longer an error in non-strict mode (still an
error in strict mode)
* Fixed cyclic type packs not being displayed in the type
* Added an error when unrelated types are compared with `==`/`~=`
* Fixed false positive errors involving sub-type tests an `never` type
* Fixed miscompilation of multiple assignment statements (Fixes
luau-lang/luau#754)
* Type inference stability improvements
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Development

Successfully merging a pull request may close this issue.

2 participants