Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

some parser bugs and small additions #5

Closed
ghoulsblade opened this Issue · 30 comments

3 participants

@ghoulsblade

Hi all, awesome project, i use it to run games made in luascript for Love2D in a experimental webplayer :
https://github.com/ghoulsblade/love-webplayer

i found a few parser bugs, here's the current list, will track new ones in the readme on my project page.

parser bug : clouds demo : parse error in keypressed : >> love.filesystem.load("main.lua")() << workaround : >> (love.filesystem.load("main.lua"))() <<
parser bug : >> myvar = .5 << workaround >> myvar = 0.5 <<
parser bug : iyfct block comment in main.lua >> --[[ bla... ]]-- << workaround : remove comment (js regexp pre load?) or use single line comments
parser bug : iyfct \" in table-save.lua >> "\"..string.char(26)..\"" << workaround : >> '"'.."..string.char(26).."..'"' <<
parser bug : sinescroller >> local mytext = "bla" print(mytext:len()) << workaround >> print(string.len(mytext)) <<

i also added small bits to the lua-parser code that might be helpful to others :
see https://github.com/ghoulsblade/love-webplayer/blob/master/js/lua-parser.js all lines marked with ghoulsblade :

near beginning of parser :
"G = G || lua_newtable2(lua_core);\n" + // added 2012-03-16 by ghoulsblade for love-webplayer
since i use multiple lua files in the same state

"LuaBootStrap(G);\n"+ // added 2012-03-16 by ghoulsblade for love-webplayer
to register some custom functions in the api (love2d engine functionality)

if (arguments.length == 0) return [new Date().getTime()]; // added 2012-03-16 by ghoulsblade for love-webplayer
in os.time

keep up the good work =)

@mherkender
Owner

I'll talk about your parser errors here...

love.filesystem.load("main.lua")()

The grammer rules for setting a variable and calling a function are pretty different, and lua.js chokes on the differences in this scenario

somefn().somevar = 1
-- vs
somefn()()

That workaround is the only way to do it right now, since Jison does not appear to be able to support a parser that can handle this. Suggestions are welcome.

myvar = .5

Fixed. Thanks!

--[[ bla... --]]

I made a mistake in the grammer rules here. It's expected --[[ bla ]]-- but the actual rules are --[[ bla --]]. Fixed.

"\"..string.char(26)..\""

The string rules had some problems. Fixed.

mytext:len()

Ah yes, I never added support for the string metatable that calls functions in the string library. There was a reason, but I can't remember why. I'll check it out later.

As for your other changes. I added a similar change to time() to have it return Javascript time. That was a good idea.

You also added some interesting bootstrap code, but I think this can be made easier by adding those properties to lua_libs and lua_core after lua.js has been loaded but before any love scripts have been run. For example...

<script src="lua+parser.js"></script>
<script>
lua_libs["love"] = {};
lua_libs["love"]["load"] = function () {};
... etc ...
var audio_table = lua_newtable();
lua_libs["love"]["audio"] = audio_table;
lua_tableset(audio_table, "play", function () { /* play audio */});
... etc ...
</script>
<script src="alovegame.js"></script>

You can take a look at lualib.js to see how it does this. If you have any questions send me a message.

@ghoulsblade

thanks for quick fixes =)
lua_libs is interesting for custom api, but i'll still need the bootstrap for overriding the default "require" i think.
i made it so .lua files can be loaded during runtime by downloading the source via javascript ajax.

regarding love.filesystem.load : might be a misunderstanding here, the first one isn't about setting vars,
love.filesystem.load("main.lua")()
here >> love.filesystem.load("main.lua") << returns a function, which is then immediately called.
like

local myfun = love.filesystem.load("main.lua") myfun() <<

i found one more problem with unpack :

print("unpack test1:",unpack({1,2,3,4})) <<
fails, but i fixed it by doing
return list.uints.slice(i - 1, j); // added 2012-03 by ghoulsblade for love-webplayer <<
in the "unpack" implementation, not sure if that breaks anything else since i didn't understand what the throw ReturnValues was supposed to do, but my change seems to work so far

comment grammar : i think it's --[[ bla ]] as in [[ ]] is considered one block not interrupted by newlines when commented out by --.
it can also be used to make multi-line strings in lua, but i didn't try if that works yet :
local myst = [[
bla01
bla02
bla03
]]

(even removes first newline if it follows right after the [[ to make code pretty.

@ghoulsblade

fix for string.sub :

"sub": function (s, i, j) {
if (i < 0) {
  i = s.length + 1 + i;
}
if (j == null) {
  return [s.substring(i-1)];
} else if (j < 0) {
  j = s.length + 1 + j;
}
return [s.substring(i-1, j)];
},

to pass some tests :

print("----") print("string.sub test1:",string.sub("abcdef",1,1))       -- original lua : a     
print("----") print("string.sub test2:",string.sub("abcdef",1,-1))  -- original lua : abcdef        
print("----") print("string.sub test3:",string.sub("abcdef",1,-2))  -- original lua : abcde     
print("----") print("string.sub test4:",string.sub("abcdef",4))     -- original lua : def       
print("----") print("string.sub test5:",string.sub("abcdef",-3))        -- original lua : def       
print("----") print("string.sub test11:",string.sub("abcdef",0))        -- original lua : abcdef
print("----") print("string.sub test12:",string.sub("abcdef",0,0))      -- original lua : 
print("----") print("string.sub test13:",string.sub("abcdef",1,0))      -- original lua : 
@mherkender
Owner

lua_libs is interesting for custom api, but i'll still need the bootstrap for overriding the default "require" i think.

You can do that.

lua_core["require"] = function () {...}

regarding love.filesystem.load : might be a misunderstanding here, the first one isn't about setting vars,

You're right, but to the parser those examples are parsed in a same way. A certain class of parsers can't handle that, and I don't see a way to do it with Jison or another Javascript option.

print("unpack test1:",unpack({1,2,3,4}))

Someone found this bug, which I fixed just today. Try updating your copy of lua.js.

it can also be used to make multi-line strings in lua, but i didn't try if that works yet

They work.

comment grammar : i think it's --[[ bla ]] as in [[ ]] is considered one block not interrupted by newlines when commented out by --.

Ah yes, you're right. After looking through more examples of this comment it appears that second "--" is not necessary. I'll remove it from the grammar rules.

fix for string.sub :

I'll take a look at this. You can send pull/merge requests if you want.

@mherkender
Owner

string.sub has been fixed.

@ghoulsblade

awesome, thanks, works like a charm =)
one small thing i still change :

// math
var max = 0x100000000;
var seed = (Math.random() * max) & (max - 1);

here those are javascript global vars, and i had to rename the max var, since i use a global function named max rather than math.max, so one overwrote the other, which can be confusing to debug.
maybe you'll want to use mherkender_luajs_max or something instead ? not pretty but afaik js doesn't have namespaces.
It's just a minor point that can be a little confusing, nothing urgent.

@mherkender
Owner

JS does have namespaces, in a way. That's why it's common to put a library in a one-off function call var library = (function () {...})() so the local variables used are kept internal.

I didn't intend for lua.js to be used this way, so all the useful variables are global, and unfortunately some of the less useful ones. I've renamed them to start with _lua_ to keep them from conflicting with commonly named variables.

@ghoulsblade

found another parser bug : >> print("\") << workaround >> print(string.char(92)) <<
edit: github website messes up the display, it's two backslashes after another, which should result into a backslash in the text

@ghoulsblade

and another one, tricky to work around :
parser bug : >> local MyClass = {} function MyClass.somefun() end << workaround : remove local ? not always possible without changing the meaning of code

@mherkender
Owner

print("\")

This isn't valid Lua. Escape the backslash by writing print("\\") instead.

EDIT: I just noticed you wrote \\ but github seems to have removed it. print("\\") works for me, I'm not sure what the problem is.

local MyClass = {} function MyClass.somefun()

Sorry, this is an oversight. lua.js assumes incorrectly in function declarations like "function A.B()" that A is always a global. I'll fix it as soon as I find time.

The workaround is declaring using A.B = function() instead.

@mherkender
Owner

The function declaration bug has been fixed. Thanks again!

@ghoulsblade

awesome, thanks =)
the print("double backslash") definitely triggers and error for me, despite just updating to latest precompile parser.
maybe because i'm loading lua code from a file, and you as double-quoted string in javascript or something where the double-backslash is already evaluated before it hits the parser ? here's the error i get :
[22:08:37.766] error during main.lua : Error: Parse error on line 53:
...recursiveEnumerate("", "") print("re do
-----------------------^
Expecting 'EOF', ';', '(', ')', 'LOCAL', 'DO', 'END', 'WHILE', 'REPEAT', 'UNTIL', 'IF', 'FOR', ',', 'FUNCTION', 'NAME', 'RETURN', 'BREAK', 'ELSE', 'ELSEIF', 'THEN', '+', '-', '', '/', '^', '%', '..', '<', '>', '<=', '>=', '==', '~=', 'AND', 'OR', '}', ']', got 'STRING' : parseError("Parse error on line 53:\n...recursiveEnumerate(\"\", \"\")\tprint(\"re do\n-----------------------^\nExpecting 'EOF', ';', '(', ')', 'LOCAL', 'DO', 'END', 'WHILE', 'REPEAT', 'UNTIL', 'IF', 'FOR', ',', 'FUNCTION', 'NAME', 'RETURN', 'BREAK', 'ELSE', 'ELSEIF', 'THEN', '+', '-', '', '/', '^', '%', '..', '<', '>', '<=', '>=', '==', '~=', 'AND', 'OR', '}', ']', got 'STRING'",[object Object])

@http://localhost/love-webplayer/js/lua-parser.js:670
...

@mherkender
Owner

When I run this code in a JS console

var text = "print(\"\\\\\")";
console.log(text);
var func = lua_load(text);
func();

I get the output...

print("\\")
\

Can you give me an isolated example? I can't figure out what the problem would be.

@ghoulsblade

aha! i only get the error if there is code afterwards. try this :

    var text = "print(\"\\\\\") print(\"bla\")";
    console.log(text);
    var func = lua_load(text);
    func();

    [23:08:28.163] Parse error on line 1:
    print("\\") print("bla")
    -------------------^
    Expecting ')', ',', got 'NAME' @ http://localhost/love-webplayer/parse-quote/lua+parser.js:667
@ghoulsblade

another thing i found today :
local t1,t2 = {},{} t2[t1] = 1
key type table not supported it seems, could it work if i modify lua_newtable, lua_rawget and lua_rawset to support Object as type and store in some new table.obj[] list of {key,val} pairs and check by stupid/slow iteration ?

@ghoulsblade

it worked =D pull request sent : #6

@ghoulsblade

found a tricky problem with multiple-return-values being put in a table directly

function myfun () return "aa","bb" end
local arr = {myfun()}
print(arr[1],arr[2]) -- prints: aa null
@mherkender
Owner

var text = "print(\"\\\") print(\"bla\")";

I hunted this down, the regex for strings was ignoring the closing quote if a backslash appears and the end of the string. Thanks!

print(arr[1],arr[2]) -- prints: aa null

Damn, I added support for this, but there was a regression at some point. Fixed.

local t1,t2 = {},{} t2[t1] = 1

I was originally planning on adding some kind of unique id to each lua table that could be used as a key, but I think your solution is better. You missed one or two things (pairs() for example, doesn't check for objs) so I added some changes in a later commit, but your commit has been merged :)

@ghoulsblade

awesome, thanks =)
i have incomplete implementations for string.match and string.find that work in some basic tests:
#6
they don't cover all special lua patterns but it's a start.

@ghoulsblade

found another problem with backslash in string:

    var text = "print(\"\\\\\") print(\"bla\")"; // print("\\") print("bla")        WORKS
    var text = "print(\"\\\"\") print(\"bla\")"; // print("\"") print("bla")        FAILS
@mherkender
Owner

var text = "print(\"\\"\") print(\"bla\")"; // print("\"") print("bla")

Sorry, I accidentally disabled escaping instead of making it work properly. It should be fixed now (again).

I also added support for string library functions to be used directly on strings ("lowercase":upper()) which you mentioned earlier.

I should probably add pattern support, I need to think about it a bit, it's actually fairly complicated. See my comments on your pull request for more info.

@ghoulsblade

awesome, thanks =)
i found a problem with ... (varargs) :

function MyPrint (...) print("vararg1",...) end MyPrint(1,2,3,4)

prints a table rather than 1 2 3 4 : ({str:{}, uints:[1, 2, 3, 4], floats:{}, bool:{}, objs:[], arraymode:true})
using unpack(...) and unpack({...}) also leads to weird errors, so the only workaround is using explicit params it seems.

@mherkender
Owner

function MyPrint (...) print("vararg1",...) end MyPrint(1,2,3,4)

Yikes. The vararg variable was being stored as a table, but everywhere else it is interpreted as a list of variables (which is what it's supposed to be). Needless to say this causes a lot of problems. Fixed.

@erickmelo

Hi,

Congratulations for the project.. It's very useful.

I would like to contribute to the project.. How can I add a lua library to the script? The ideia is to implement some missing Lua modules in JS..

@ghoulsblade

try this, but please open a new issue if you have further questions rather than hijacking this thread.

    var luacode = MyLoadLuaAsStringViaAjaxOrSimilar(.........);
    var myfun = lua_load(luacode,"temp_function_name"); // parse code
    var res = myfun(); // run code, returns _G
@mherkender
Owner

Well, technically each issue is supposed to be about one thing, but I don't take bug tracking too seriously, ghoulsblade.

erickmelo. I'm not totally sure what you mean, so I've come up with four answers, one if which is probably the one you're looking for.

  • Do you want to add missing Lua 5.1 features to lua.js? If so check out lualib.js. It's got a handful of TODOs and functions that just call not_supported that could use help. Keep in mind not every Lua 5.1 feature can be supported by lua.js. Submit a pull request to me if you want your change added, or message me if you want more info.
  • If you want to create your own global modules, extending the ones available by default, you can add them to the lua_libs and lua_core variables in a script that runs right after lua.js has loaded. You can take a look at how this is done in lualib.js.
  • If you want to create your own custom modules, like one would normally in Lua, try using Lua's built-in require and module functions. lua_module is also available in Javascript to assist with this.
  • If you want to create basic lua scripts, you can use the example ghoulsblade suggested. lua_load generates a function that, when executed, runs the lua script it was given. Keep in mind the lua2js tool is also available for parsing Lua code, see the README for more info.
@ghoulsblade

webgl game powered by luajs and love2d if you like =) http://ghoulsblade.schattenkind.net/love-webplayer/ludumdare201204/

also found a bug with nested metatables :

local t1 = setmetatable({},{__index={a=5}})
print(t1.a) -- lua=5 luajs=5 -> OK

local t2 = setmetatable({},{__index=t1})
print(t2.a) -- lua=5 luajs=undefined -> ERROR
@mherkender
Owner

http://ghoulsblade.schattenkind.net/love-webplayer/ludumdare201204/

Wow!

local t2 = setmetatable({},{__index=t1})

The current implementation doesn't allow nested metatables, which doesn't match the behavior in Lua 5.1. Fixed.

@ghoulsblade

regarding more detailed error messages (see also closed #10 ) :
when using the runtime parser (lua+parser.js) rather than precompiling the files, the stacktraces are often confusing.
They show what part of luajs caused the error, but it doesn't show which part of the actual lua script caused the problem.
That's because it is usually one big eval, or the result of that stored in one big runtime function object.

one idea that came to my mind for web-site/browserjs):
instead of eval, dynamically adding a script-tag to the html. do you think that would be worth a try ?
can you think of any alternatives to make the stacktrace easier to read ?

@mherkender
Owner

Maybe it would be better to avoid the "eval". I'll check it out.

To help with stack traces, someone suggested using source maps. It's a new technology intended to help with projectiles like lua.js, CoffeeScript, and Dart.

http://www.html5rocks.com/en/tutorials/developertools/sourcemaps/

It seemed very new, and I couldn't find good documentation, so I decided to save it for later.

@mherkender mherkender closed this
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.