gopher-lua

gopher-lua copied to clipboard

• [ ] GopherLua is a Lua5.1 implementation. You should be familiar with Lua programming language. Have you read Lua 5.1 reference manual carefully?
• [x] GopherLua is a Lua5.1 implementation. In Lua, to keep it simple, it is more important to remove functionalities rather than to add functionalities unlike other languages . If you are going to introduce some new cool functionalities into the GopherLua code base and the functionalities can be implemented by existing APIs, It should be implemented as a library.

Please answer the following before submitting your issue:

1. What version of GopherLua are you using? : GopherLua 0.1 Copyright (C) 2015 -2017 Yusuke Inuzuka
2. What version of Go are you using? : go version go1.15.7
3. What operating system and processor architecture are you using? : darwin/amd64
4. What did you do? : Execute glua ./repro.lua where the file contains:

local a = {}
local d = 'e'
local f = 1

f, a.d = f, d

print(f..", "..a.d)

5. What did you expect to see? : The result of 1, e
6. What did you see instead? : The result of d, e

Somehow the property-access in the multi-assignment causes f to contain the name of the second variable on the right side instead of the value of the 1st variable on the right side.

As a note, swapping the sequence results in the correct behavior: a.d, f = d, f

Reference: #314

It looks like the Problem is within compileAssignStmtLeft. There is some check if the identifier (in this case f) is the last argument. In case it is its Position (ec.reg) is looked up via context.FindLocalVar. If it's not the last argument there is no lookup.

Changing this so the variable is always looked up works and the tests still pass. However I don't know if there is some edge case which requires the variable not to be looked up.

Is this something you would fix @Simerax or do we need to involve @yuin ?

I can create a pull request yes. I was just hoping yuin would maybe give us some insight on this FindLocalVar thing.

Alright after looking into this a little more - I think there is something broken in the way multi assignment works in gopher-lua. There is a multi assign test in _lua5.1-tests/attrib.lua which is currently commented out. This test fails before and after my change.

do
  local a,i,j,b
  a = {'a', 'b'}; i=1; j=2; b=a
  i, a[i], a, j, a[j], a[i+j] = j, i, i, b, j, i
  assert(i == 2 and b[1] == 1 and a == 1 and j == b and b[2] == 2 and
         b[3] == 1)
end

I really don't know enough about how lua or gopher-lua works to fix this. I think this is something @yuin should get involved in.

@yuin any thoughts?

Hooray!

Alright after looking into this a little more - I think there is something broken in the way multi assignment works in gopher-lua. There is a multi assign test in _lua5.1-tests/attrib.lua which is currently commented out. This test fails before and after my change.

do
  local a,i,j,b
  a = {'a', 'b'}; i=1; j=2; b=a
  i, a[i], a, j, a[j], a[i+j] = j, i, i, b, j, i
  assert(i == 2 and b[1] == 1 and a == 1 and j == b and b[2] == 2 and
         b[3] == 1)
end

I really don't know enough about how lua or gopher-lua works to fix this. I think this is something @yuin should get involved in.

The mentioned test still fails with this change.

Lua 5.1
--- FAIL: TestLua (7.57s)
    script_test.go:79: attrib.lua:334: attempt to index a non-table object(number) with key '3'
        stack traceback:
        	attrib.lua:334: in main chunk
        	[G]: ?
-------------------------

This issue has been closed because the original issue was resolved. Please submit a new issue If you have a problem. And I KNEW the problem so I've commented out the test code. It is hard to fix because of the parsing strategy differences between CLua and GohperLua.