search

ReFreezed / DumbLuaParser

Lua parsing library capable of optimizing and minifying code.
http://refreezed.com/luaparser/
MIT License
33 stars 3 forks source link

Minification problems #2

Closed 2dengine closed 2 years ago

2dengine commented 2 years ago

Hello, I am using your parser with the following tool for the Love2D engine: https://github.com/2dengine/love.maker and we are getting incorrect output after code minification (difficult to say, but it may be related to parsing literals).

I ran another of my tools called SUPERSTRICT https://github.com/2dengine/sstrict.lua over the source code of the parser and we found some issues:

/minify.lua:436: unused variable 'x'
/minify.lua:435: unused variable '_ENV'
/minify.lua:485: undefined variable 'bit32'
/minify.lua:1080: variable name 'i1' redefinition
/minify.lua:1219: variable name 'i1' redefinition
/minify.lua:1219: variable name 'i2' redefinition
/minify.lua:1256: variable name 'i1' redefinition
/minify.lua:1256: variable name 'i2' redefinition
/minify.lua:1277: empty code block
/minify.lua:1344: variable name 'i1' redefinition
/minify.lua:1350: variable name 'b1' redefinition
/minify.lua:1350: variable name 'b2' redefinition
/minify.lua:1363: variable name 'i2' redefinition
/minify.lua:1162: unused variable 'BYTES_NUM_OR_DOT'
/minify.lua:1653: variable name 'tokNext' redefinition
/minify.lua:1653: variable name 'err' redefinition
/minify.lua:1709: variable name 'tokNext' redefinition
/minify.lua:1709: variable name 'err' redefinition
/minify.lua:2184: variable name 'tokNext' redefinition
/minify.lua:2184: variable name 'err' redefinition
/minify.lua:2213: variable name 'tokNext' redefinition
/minify.lua:2213: variable name 'err' redefinition
/minify.lua:2236: variable name 'tokNext' redefinition
/minify.lua:2236: variable name 'err' redefinition
/minify.lua:2250: variable name 'expr' redefinition
/minify.lua:2250: variable name 'tokNext' redefinition
/minify.lua:2250: variable name 'err' redefinition
/minify.lua:2260: variable name 'block' redefinition
/minify.lua:2260: variable name 'tokNext' redefinition
/minify.lua:2260: variable name 'err' redefinition
/minify.lua:2269: variable name 'block' redefinition
/minify.lua:2269: variable name 'tokNext' redefinition
/minify.lua:2269: variable name 'err' redefinition
/minify.lua:2310: variable name 'ok' redefinition
/minify.lua:2310: variable name 'tokNext' redefinition
/minify.lua:2310: variable name 'err' redefinition
/minify.lua:2316: empty code block
/minify.lua:2328: variable name 'tokNext' redefinition
/minify.lua:2328: variable name 'err' redefinition
/minify.lua:2355: variable name 'tokNext' redefinition
/minify.lua:2355: variable name 'err' redefinition
/minify.lua:2374: variable name 'tokNext' redefinition
/minify.lua:2374: variable name 'err' redefinition
/minify.lua:2387: variable name 'tokNext' redefinition
/minify.lua:2387: variable name 'err' redefinition
/minify.lua:2414: variable name 'tokNext' redefinition
/minify.lua:2414: variable name 'err' redefinition
/minify.lua:2438: variable name 'ok' redefinition
/minify.lua:2438: variable name 'tokNext' redefinition
/minify.lua:2438: variable name 'err' redefinition
/minify.lua:2522: variable name 'tokNext' redefinition
/minify.lua:2522: variable name 'err' redefinition
/minify.lua:2537: variable name 'ok' redefinition
/minify.lua:2537: variable name 'tokNext' redefinition
/minify.lua:2537: variable name 'err' redefinition
/minify.lua:2758: empty code block
/minify.lua:3178: variable name 'i' redefinition
/minify.lua:3290: empty code block
/minify.lua:3406: empty code block
/minify.lua:3622: variable name 'node' redefinition
/minify.lua:3941: empty code block
/minify.lua:4106: variable name 'node' redefinition
/minify.lua:4311: empty code block
/minify.lua:4623: variable name 'slot' redefinition
/minify.lua:4634: variable name 'slot' redefinition
/minify.lua:4666: variable name 'statement' redefinition
/minify.lua:4801: variable name 'optimize' redefinition
/minify.lua:5066: variable name 'ok' redefinition
/minify.lua:5092: variable name 'pretty' redefinition
/minify.lua:5114: variable name 'ok' redefinition
/minify.lua:5168: variable name 'ok' redefinition
/minify.lua:5424: variable name 'ok' redefinition
/minify.lua:5541: variable name 'ok' redefinition
/minify.lua:5600: variable name 'ok' redefinition
/minify.lua:5621: variable name 'ok' redefinition
/minify.lua:5646: variable name 'ok' redefinition
/minify.lua:5656: variable name 'ok' redefinition
/minify.lua:5669: variable name 'ok' redefinition
/minify.lua:5700: variable name 'ok' redefinition
/minify.lua:5723: variable name 'ok' redefinition
/minify.lua:5749: variable name 'ok' redefinition
/minify.lua:5759: variable name 'ok' redefinition
/minify.lua:6094: empty code block
/minify.lua:6104: empty code block
/minify.lua:6297: empty code block
/minify.lua:6453: variable name 'v' redefinition
/minify.lua:6453: unused variable 'v'

The "redefinition" warnings are not important, although there are some questionable parts like: HANDLE_ENV = not pcall(function() local x = _G end) where "x" is an unused variable and "local x = _G end" does nothing.

Any tips on how we can pinpoint what is causing the incorrect output of the minification? Thank you!

ReFreezed commented 2 years ago

Can you post/link the code that becomes incorrect when minified? I don't know what the issue could be without more information. The minify() function should only rename local variables - not touch any literals or anything else (unless the optimize argument is set, in which case it might be the optimize() function doing something bad).

Looking through your list of reported issues in the parser's source, none is an actual issue. The HANDLE_ENV assignment statement is successfully detecting if the running Lua version supports _ENV.

2dengine commented 2 years ago

Hello. The code in question is not open source and it doesn't raise any errors. However I get weird behavior after minification.

I am not calling the optimize function explicitly:

local parser = require(lib.."minify")
local minify = function(s)
  local ast = parser.parse(s)
  parser.minify(ast)
  return parser.toLua(ast)
end

SUPERSTRICT reports possible issues and while these may not be "issues" that would raise an error, they are certainly worth looking into. For example, Lua is an interpreted language so things like empty code blocks are not "optimized" by a compiler like in the case of other static languages.

Perhaps, I am missing something, but I don't see how pcall could fail in the following case:

    local pcall = pcall
    local _ENV  = nil
    HANDLE_ENV  = not pcall(function() local x = _G end)
ReFreezed commented 2 years ago

Are you using LuaJIT-specific syntax for any numbers (like 64-bit ints)? If you just parse() and toLua() the code without minify(), is the problem still there?

Regarding empty blocks, I do this in several places as an optimization:

if x then
    -- Do nothing. Don't even check any of the remaining if-conditions.
elseif y then
    -- ...
elseif z then
    -- ...
end

-- Instead of doing this:
if not x then
    if y then
        -- ...
    elseif z then
        -- ...
    end
end

If _ENV is supported then x=_G (which means x=_ENV._G) becomes x=(nil)._G which raises an error that pcall catches. Because the value of the local _ENV variable is nil when the function pcall calls is created then the function's environment becomes nil. If _ENV isn't supported then _G is looked up from the main chunk's environment, which is a table like normal, thus no error.

2dengine commented 2 years ago

Hello again. Yes, I can confirm that the issues begin only after the call to "parser.minify(ast)". love.physics starts behaving strangely - for example some shapes seem to be way larger than their intended dimensions. In another game, we are getting crashes and other unexpected results. Unfortunately these are large projects so I cannot give you a simple code example that we can debug. Both of my games are using FFI-related features although this is not directly related to love.physics. I never used a version of Lua where "local x = _G" could raise an error unless _G uses custom metatables. It's fine if the code works - all I am saying is that it's a hacky way to do it. Thank you for the reply!

ReFreezed commented 2 years ago

Then I can only assume the minifier renames some variables incorrectly, and that there's probably no parsing/toLua issue here. I'll try to do some more random testing, I guess.

They reworked environments in Lua 5.2. I'm just detecting if a feature exists by trying to use the feature directly and checking if the code succeeds (instead of checking _VERSION or some other value). I don't think it's much different from e.g. checking pcall(require,"bit") to detect if the bit module exists.

ReFreezed commented 2 years ago

Ok, I believe I found and fixed the problem. It affected declarations/functions/for-loops where multiple declared names were the same (which they often become after getting minified in case some of the variables are unused, like the index variable in ipairs loops).

2dengine commented 2 years ago

Hello and good job! I pulled and ran the latest version of the code and it seems to work fine in my initial tests. I will push the update publicly to our games and promise to report back if anybody finds any other bugs. Thank you for the support, Ivan from 2dengine

ReFreezed commented 2 years ago

Great! :)