Skip to content

Commit 0982cab

Browse files
committed
Fixed updateReferences() not always updating ident.declaration properly when ident.declaration==ident should have been true (which sometimes broke minify() and maybe more).
1 parent d985e62 commit 0982cab

File tree

2 files changed

+79
-9
lines changed

2 files changed

+79
-9
lines changed

dumbParser.lua

Lines changed: 20 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -3501,8 +3501,8 @@ end
35013501

35023502

35033503

3504-
-- declIdent|nil = findIdentifierDeclaration( ident )
3505-
-- declIdent.parent = decl|func|forLoop
3504+
-- declIdent | nil = findIdentifierDeclaration( ident )
3505+
-- declIdent.parent = decl | func | forLoop
35063506
local function findIdentifierDeclaration(ident)
35073507
local name = ident.name
35083508
local parent = ident
@@ -3516,20 +3516,30 @@ local function findIdentifierDeclaration(ident)
35163516
if parent.type == "declaration" then
35173517
local decl = parent
35183518

3519-
if lastChild.container ~= decl.values then
3520-
local declIdent = lastItemWith1(decl.names, "name", name)
3521-
if declIdent then return declIdent end
3519+
if lastChild.container == decl.names then
3520+
assert(lastChild == ident)
3521+
return ident -- ident is the declaration node.
35223522
end
35233523

35243524
elseif parent.type == "function" then
3525-
local func = parent
3526-
local declIdent = lastItemWith1(func.parameters, "name", name) -- Note: This will ignore any vararg parameter.
3527-
if declIdent then return declIdent end
3525+
local func = parent
3526+
3527+
if lastChild.container == func.parameters then
3528+
assert(lastChild == ident)
3529+
return ident -- ident is the declaration node.
3530+
else
3531+
local func = parent
3532+
local declIdent = lastItemWith1(func.parameters, "name", name) -- Note: This will ignore any vararg parameter.
3533+
if declIdent then return declIdent end
3534+
end
35283535

35293536
elseif parent.type == "for" then
35303537
local forLoop = parent
35313538

3532-
if lastChild.container ~= forLoop.values then
3539+
if lastChild.container == forLoop.names then
3540+
assert(lastChild == ident)
3541+
return ident -- ident is the declaration node.
3542+
elseif lastChild.container ~= forLoop.values then
35333543
local declIdent = lastItemWith1(forLoop.names, "name", name)
35343544
if declIdent then return declIdent end
35353545
end
@@ -4144,6 +4154,7 @@ local function getInformationAboutIdentifiersAndUpdateReferences(node)
41444154
statementOrInterest, block = findParentStatementAndBlockOrNodeOfInterest(block, currentDeclIdent)
41454155

41464156
if not statementOrInterest then
4157+
-- We should only get here for globals (i.e. there should be no declaration).
41474158
assert(not currentDeclIdent)
41484159
return
41494160
end

testsuite.lua

Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -915,6 +915,32 @@ test("Minify", function()
915915
[[ for e,t in ipairs(global)do globalFunc1(t);for e=e,#global do globalFunc2(e);end end ]]
916916
)
917917

918+
-- Make sure we minify multi-name declaration-likes as much as possible without shadowing too much.
919+
testMinify(
920+
[[ local x, y = 1, 2 ; return x ]],
921+
[[ local e,t=1,2;return e; ]]
922+
)
923+
testMinify(
924+
[[ local x, y = 1, 2 ; return y ]],
925+
[[ local e,e=1,2;return e; ]]
926+
)
927+
testMinify(
928+
[[ for x, y in 1, 2 do return x end ]],
929+
[[ for e,t in 1,2 do return e;end ]]
930+
)
931+
testMinify(
932+
[[ for x, y in 1, 2 do return y end ]],
933+
[[ for e,e in 1,2 do return e;end ]]
934+
)
935+
testMinify(
936+
[[ function f(x, y) return x end ]],
937+
[[ function f(e,t)return e;end ]]
938+
)
939+
testMinify(
940+
[[ function f(x, y) return y end ]],
941+
[[ function f(e,e)return e;end ]]
942+
)
943+
918944
if _VERSION >= "Lua 5.2" then
919945
testMinify(
920946
[[
@@ -926,6 +952,39 @@ test("Minify", function()
926952
[[ local _ENV=_ENV;local function e()return _ENV;end ]]
927953
)
928954
end
955+
956+
-- Check that declaration references don't change.
957+
for _, filename in ipairs{"test.lua","dumbParser.lua"} do
958+
local ast = assert(parser.parseFile(filename))
959+
local names = {}
960+
local decls = {}
961+
962+
parser.updateReferences(ast)
963+
964+
parser.traverseTree(ast, function(node)
965+
if node.type == "identifier" then
966+
names[node] = node.name
967+
decls[node] = node.declaration
968+
end
969+
end)
970+
971+
parser.minify(ast)
972+
parser.updateReferences(ast)
973+
-- print("----------------------------------------------------------------") ; print(parser.toLua(ast, true))
974+
975+
local ok = true
976+
977+
parser.traverseTree(ast, function(node)
978+
if node.type == "identifier" and node.declaration ~= decls[node] then
979+
ok = false
980+
print(parser.formatMessage(node, "Declaration changed for '%s' (is '%s').", names[node], node.name))
981+
print(decls[node] and parser.formatMessage(decls[node] , "from..."):gsub("[^\n]+", " %0") or "from... nil")
982+
print(node.declaration and parser.formatMessage(node.declaration, "to..."):gsub("[^\n]+", " %0") or "to... nil")
983+
end
984+
end)
985+
986+
assert(ok, "Declarations changed!")
987+
end
929988
end)
930989

931990

0 commit comments

Comments
 (0)