Skip to content

Commit

Permalink
Merge pull request #127 from mpeterv/propagation-rewrite
Browse files Browse the repository at this point in the history 
Rework local variable access resolution
  • Loading branch information
@mpeterv
mpeterv authored Sep 9, 2017
2 parents d8d639f + 5770e02 commit bb0bd4c
Show file tree
Hide file tree
Showing 6 changed files with 190 additions and 172 deletions.
3 changes: 3 additions & 0 deletions CHANGELOG.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,9 @@
### Fixes

* Added missing definition of `ngx.ERROR` constant to `ngx_lua` std (#123).
* Fixed unused values and initialized accesses not being reported when the
access is in a closure defined in code path incompatible with the value
assignment (#126).

## 0.21.0 (2017-09-04)

Expand Down
30 changes: 30 additions & 0 deletions spec/check_spec.lua
View file
Original file line number Diff line number Diff line change
Expand Up @@ -536,6 +536,36 @@ end
]])
end)

it("detects unused local value referred to from closure in incompatible branch", function()
assert.same({
{code = "311", name = "a", line = 4, column = 4, end_column = 4},
{code = "321", name = "a", line = 6, column = 28, end_column = 28}
}, check[[
local a
if (...)() then
a = 1
else
(...)(function() return a end)
end
]])
end)

it("detects unused upvalue value referred to from closure in incompatible branch", function()
assert.same({
{code = "311", name = "a", line = 4, column = 21, end_column = 21},
{code = "321", name = "a", line = 6, column = 28, end_column = 28}
}, check[[
local a
if (...)() then
(...)(function() a = 1 end)
else
(...)(function() return a end)
end
]])
end)

it("handles upvalues before infinite loops", function()
assert.same({
{code = "221", name = "x", line = 1, column = 7, end_column = 7},
Expand Down
263 changes: 137 additions & 126 deletions src/luacheck/analyze.lua
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,35 @@
local core_utils = require "luacheck.core_utils"
local utils = require "luacheck.utils"

-- The main part of analysis is connecting assignments to locals or upvalues
-- with accesses that may use the assigned value.
-- Accesses and assignments are split into two groups based on whether they happen
-- in the closure that defines subject local variable (main assignment, main access)
-- or in some nested closure (closure assignment, closure access).
-- To avoid false positives, it's assumed that a closure may be called at any point
-- starting from expression that creates it.
-- Additionally, all operations on upvalues are considered in bulk, as in,
-- when a closure is called, it's assumed that any subset of its upvalue assignments
-- and accesses may happen, in any order.

-- Assignments and accesses are connected based on whether they can reach each other.
-- A main assignment is connected with a main access when the assignment can reach the access.
-- A main assignment is connected with a closure access when the assignment can reach the closure creation
-- or the closure creation can reach the assignment.
-- A closure assignment is connected with a main access when the closure creation can reach the access.
-- A closure assignment is connected with a closure access when either closure creation can reach the other one.

-- To determine what flow graph nodes an assignment or a closure creation can reach,
-- they are independently propagated along the graph.
-- Closure creation propagation is not bounded.
-- Main assignment propagation is bounded by entrance and exit conditions for each reached flow graph node.
-- Entrance condition checks that target local variable is still in scope. If entrance condition fails,
-- nothing in the node can refer to the variable, and the scope can't be reentered later.
-- So, in this case, assignment does not reach the node, and propagation does not continue.
-- Exit condition checks that target local variable is not overwritten by an assignment in the node.
-- If it fails, the assignment still reaches the node (because all accesses in a node are evaluated before any
-- assignments take effect), but propagation does not continue.

local function register_value(values_per_var, var, value)
if not values_per_var[var] then
values_per_var[var] = {}
Expand All @@ -9,192 +38,174 @@ local function register_value(values_per_var, var, value)
table.insert(values_per_var[var], value)
end

local function add_resolution(line, item, var, value, mutation)
-- Called when assignment of `value` is connected to an access.
-- `item` contains the access, and `line` contains the item.
local function add_resolution(line, item, var, value, is_mutation)
register_value(item.used_values, var, value)
value[mutation and "mutated" or "used"] = true
value[is_mutation and "mutated" or "used"] = true
value.using_lines[line] = true

if value.secondaries then
value.secondaries.used = true
end
end

-- Connects accesses in given items array with an assignment of `value`.
-- `items` may be `nil` instead of empty.
local function add_resolutions(line, items, var, value, is_mutation)
if not items then
return
end

for _, item in ipairs(items) do
add_resolution(line, item, var, value, is_mutation)
end
end

-- Connects all accesses (and mutations) in `access_line` with corresponding
-- assignments in `set_line`.
local function cross_resolve_closures(access_line, set_line)
for var, setting_items in pairs(set_line.set_upvalues) do
for _, setting_item in ipairs(setting_items) do
add_resolutions(access_line, access_line.accessed_upvalues[var],
var, setting_item.set_variables[var])
add_resolutions(access_line, access_line.mutated_upvalues[var],
var, setting_item.set_variables[var], true)
end
end
end

local function in_scope(var, index)
return (var.scope_start <= index) and (index <= var.scope_end)
end

-- Called when value of var is live at an item, maybe several times.
-- Registers value as live where variable is accessed or liveness propogation stops.
-- Stops when out of scope of variable, at another assignment to it or at an item
-- encountered already.
-- When stopping at a visited item, only save value if the item is in the current stack
-- of items, i.e. when propogation followed some path from it to previous item
local function value_propogation_callback(line, stack, index, item, visited, var, value)
if not item then
-- Value reach end of line, so even if it's overwritten by a single assignment it's
-- not dominated by it.
-- Called when main assignment propagation reaches a line item.
local function main_assignment_propagation_callback(line, index, item, var, value)
-- Check entrance condition.
if not in_scope(var, index) then
-- Assignment reaches the end of variable scope, so it can't be dominated by any assignment.
value.overwriting_item = false
register_value(line.last_live_values, var, value)
return true
end

if not visited[index] then
if item.accesses and item.accesses[var] then
add_resolution(line, item, var, value)
end
-- Assignment reaches this item, apply its effect.

if item.mutations and item.mutations[var] then
add_resolution(line, item, var, value, true)
end
-- Accesses (and mutations) of the variable can resolve to reaching assignment.
if item.accesses and item.accesses[var] then
add_resolution(line, item, var, value)
end

local is_overwritten = item.set_variables and item.set_variables[var]
local out_of_scope = not in_scope(var, index)
local stop_and_save = not visited[index] and (out_of_scope or is_overwritten)

if stack[index] or stop_and_save then
if is_overwritten then
if value.overwriting_item ~= false then
if value.overwriting_item and value.overwriting_item ~= item then
value.overwriting_item = false
else
value.overwriting_item = item
end
end
elseif out_of_scope then
-- Value reach end of scope, so even if it's overwritten by a single assignment it's
-- not dominated by it.
value.overwriting_item = false
end
if item.mutations and item.mutations[var] then
add_resolution(line, item, var, value, true)
end

if not item.live_values then
item.live_values = {}
-- Accesses (and mutations) of the variable inside closures created in this item
-- can resolve to reaching assignment.
if item.lines then
for _, created_line in ipairs(item.lines) do
add_resolutions(created_line, created_line.accessed_upvalues[var], var, value)
add_resolutions(created_line, created_line.mutated_upvalues[var], var, value, true)
end

register_value(item.live_values, var, value)
return true
end

if visited[index] then
-- Check exit condition.
if item.set_variables and item.set_variables[var] then
if value.overwriting_item ~= false then
if value.overwriting_item and value.overwriting_item ~= item then
value.overwriting_item = false
else
value.overwriting_item = item
end
end

return true
end

visited[index] = true
end

-- For each node accessing variables, adds table {var = {values}} to field `used_values`.
-- A pair `var = {values}` in this table means that accessed local variable `var` can contain one of values `values`.
-- Values that can be accessed locally are marked as used.
local function propogate_values(line)
-- {var = values} live at the end of line.
line.last_live_values = {}

-- It is not very clever to simply propogate every single assigned value.
-- Fortunately, performance hit seems small (can be compenstated by inlining a few functions in lexer).
-- Connects main assignments with main accesses and closure accesses in reachable closures.
-- Additionally, sets `overwriting_item` field of values to an item with an assignment overwriting
-- the value, but only if the overwriting is not avoidable (i.e. it's impossible to reach end of function
-- from the first assignment without going through the second one). Otherwise value of the field may be
-- `false` or `nil`.
local function propagate_main_assignments(line)
for i, item in ipairs(line.items) do
if item.set_variables then
for var, value in pairs(item.set_variables) do
if var.line == line then
-- Values are only live at the item after assignment.
core_utils.walk_line(line, i + 1, value_propogation_callback, {}, var, value)
-- Assignments are not live at their own item, because assignments take effect only after all accesses
-- are evaluated. Items with assignments can't be jumps, so they have a single following item
-- with incremented index.
core_utils.walk_line(line, {}, i + 1, main_assignment_propagation_callback, var, value)
end
end
end
end
end

-- Called when closure (subline) is live at index.
-- Updates variable resolution:
-- When a closure accessing upvalue is live at item where a value of the variable is live,
-- the access can resolve to the value.
-- When a closure setting upvalue is live at item where the variable is accessed,
-- the access can resolve to the value.
-- Live values are only stored when their liveness ends. However, as closure propogation is unrestricted,
-- if there is an intermediate item where value is factually live and closure is live, closure will at some
-- point be propogated to where value liveness ends and is stored as live.
-- (Chances that I will understand this comment six months later: non-existent)
local function closure_propogation_callback(line, _, item, subline)
local live_values

-- Called when closure creation propagation reaches a line item.
local function closure_creation_propagation_callback(line, _, item, propagated_line)
if not item then
live_values = line.last_live_values
else
live_values = item.live_values
return true
end

if live_values then
for _, var_map in ipairs({subline.accessed_upvalues, subline.mutated_upvalues}) do
for var, accessing_items in pairs(var_map) do
if var.line == line then
if live_values[var] then
for _, accessing_item in ipairs(accessing_items) do
for _, value in ipairs(live_values[var]) do
add_resolution(subline, accessing_item, var, value, var_map == subline.mutated_upvalues)
end
end
end
end
end
-- Closure creation reaches this item, apply its effects.

-- Accesses (and mutations) of upvalues in the propagated closure
-- can resolve to assignments in the item.
if item.set_variables then
for var, value in pairs(item.set_variables) do
add_resolutions(propagated_line, propagated_line.accessed_upvalues[var], var, value)
add_resolutions(propagated_line, propagated_line.mutated_upvalues[var], var, value, true)
end
end

if not item then
return true
if item.lines then
for _, created_line in ipairs(item.lines) do
-- Accesses (and mutations) of upvalues in the propagated closure
-- can resolve to assignments in closures created in the item.
cross_resolve_closures(propagated_line, created_line)

-- Accesses (and mutations) of upvalues in closures created in the item
-- can resolve to assignments in the propagated closure.
cross_resolve_closures(created_line, propagated_line)
end
end

for _, action_key in ipairs({"accesses", "mutations"}) do
local item_var_map = item[action_key]
-- Accesses (and mutations) of locals in the item can resolve
-- to assignments in the propagated closure.
for var, setting_items in pairs(propagated_line.set_upvalues) do
if item.accesses and item.accesses[var] then
for _, setting_item in ipairs(setting_items) do
add_resolution(line, item, var, setting_item.set_variables[var])
end
end

if item_var_map then
for var, setting_items in pairs(subline.set_upvalues) do
if var.line == line then
if item_var_map[var] then
for _, setting_item in ipairs(setting_items) do
add_resolution(line, item, var, setting_item.set_variables[var], action_key == "mutations")
end
end
end
if item.mutations and item.mutations[var] then
for _, setting_item in ipairs(setting_items) do
add_resolution(line, item, var, setting_item.set_variables[var], true)
end
end
end
end

-- Updates variable resolution to account for closures and upvalues.
local function propogate_closures(line)
-- Connects main assignments with closure accesses in reaching closures.
-- Connects closure assignments with main accesses and with closure accesses in reachable closures.
-- Connects closure accesses with closure assignments in reachable closures.
local function propagate_closure_creations(line)
for i, item in ipairs(line.items) do
if item.lines then
for _, subline in ipairs(item.lines) do
-- Closures are considered live at the item they are created.
core_utils.walk_line_once(line, {}, i, closure_propogation_callback, subline)
end
end
end

-- It is assumed that all closures are live at the end of the line.
-- Therefore, all accesses and sets inside closures can resolve to each other.
for _, subline in ipairs(line.lines) do
for _, var_map in ipairs({subline.accessed_upvalues, subline.mutated_upvalues}) do
for var, accessing_items in pairs(var_map) do
if var.line == line then
for _, accessing_item in ipairs(accessing_items) do
for _, another_subline in ipairs(line.lines) do
if another_subline.set_upvalues[var] then
for _, setting_item in ipairs(another_subline.set_upvalues[var]) do
add_resolution(subline, accessing_item, var,
setting_item.set_variables[var], var_map == subline.mutated_upvalues)
end
end
end
end
end
for _, created_line in ipairs(item.lines) do
-- Closures are live at the item they are created, as they can be called immediately.
core_utils.walk_line(line, {}, i, closure_creation_propagation_callback, created_line)
end
end
end
end

local function analyze_line(line)
propogate_values(line)
propogate_closures(line)
propagate_main_assignments(line)
propagate_closure_creations(line)
end

local function is_function_var(var)
Expand Down
2 changes: 1 addition & 1 deletion src/luacheck/cache.lua
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ local cache = {}
-- third is check result in lua table format.
-- String fields are compressed into array indexes.

cache.format_version = 21
cache.format_version = 22

local option_fields = {
"ignore", "std", "globals", "unused_args", "self", "compat", "global", "unused", "redefined",
Expand Down
Loading

0 comments on commit bb0bd4c

Please sign in to comment.