Skip to content

Commit

Permalink
const hints - improve error detecting and reporting
Browse files Browse the repository at this point in the history
  • Loading branch information
thisismypassport committed Apr 15, 2024
1 parent e90ac1f commit a146c78
Show file tree
Hide file tree
Showing 7 changed files with 65 additions and 29 deletions.
8 changes: 6 additions & 2 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -354,8 +354,6 @@ objs={4,6,6,5}
?2
```

Keep in mind that *Local* variables that aren't declared as `--[[const]]` may still be treated as constants in cases where it's safe & advantageous to do so.

Furthermore, constant `if` and `elseif` branches are removed appropriately, allowing you to easily keep debug code in your source files, enabling it by simply changing the value of a variable:

```lua
Expand All @@ -371,6 +369,12 @@ end
spr(debug_spr,10,10)
```

Some details to keep in mind:
* *Local* variables that **aren't** declared as `--[[const]]` may still be treated as constants in cases where it's safe & advantageous to do so.
* *Local* variables that **are** declared as `--[[const]]` still follow the usual lua scoping rules. They cannot be reassigned but new locals with the same name can be defined.
* *Global* variables that **aren't** declared as `--[[const]]` are currently never treated as constants.
* *Global* variables that **are** declared as `--[[const]]` are assumed to *always* have their constant value. They cannot be reassigned and can only be used below their declaration.

## Passing constants via command line

You can even declare constants in the command line, if you prefer:
Expand Down
11 changes: 9 additions & 2 deletions pico_constfold.py
Original file line number Diff line number Diff line change
Expand Up @@ -474,8 +474,12 @@ def update_node_constval(node):
else:
set_const(node, constval)

elif node.type == NodeType.var and node.var and node.var.constval and not node.new and not node.assignment:
set_const(node, node.var.constval)
elif node.type == NodeType.var and node.var and not node.new and not node.assignment:
if node.var.constval:
set_const(node, node.var.constval)
elif node.var.is_const:
add_error(f"'{node.name}' is marked as constant but is used above where it is assigned to", node)
node.var.is_const = False

elif node.type in (NodeType.local, NodeType.assign):
visit_assign(node)
Expand Down Expand Up @@ -570,7 +574,10 @@ def visit_assign(node):
reason = ""
if const_ctxt_fail:
reason = f" due to '{const_ctxt_fail.source_text}'"
elif not is_const:
reason = f" due to being reassigned"
add_error(f"Local '{target.name}' is marked as const but its value cannot be determined" + reason, target)
target.var.is_const = False

if is_const_ctxt:
in_const_ctxt -= 1
Expand Down
50 changes: 27 additions & 23 deletions run_tests.py
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ def try_read_dir_contents(dir):
return results

def run_test(name, input, output, *args, private=False, check_output=True, from_output=False,
read_stdout=False, norm_stdout=nop, exit_code=0, extra_outputs=None, output_reader=try_file_read,
stdout_output=None, norm_stdout=nop, exit_code=0, extra_outputs=None, output_reader=try_file_read,
pico8_output_val=None, pico8_output=None, pico8_run=None, copy_in_to_out=False):
if g_opts.test:
for wanted_test in g_opts.test:
Expand All @@ -59,15 +59,16 @@ def run_test(name, input, output, *args, private=False, check_output=True, from_
start_test()
prefix = "private_" if private else ""
inpath = path_join(prefix + ("test_output" if from_output else "test_input"), input)
outpath = path_join(prefix + "test_output", output)
cmppath = path_join(prefix + "test_compare", output)
if output:
outpath = path_join(prefix + "test_output", output)
cmppath = path_join(prefix + "test_compare", output)

if copy_in_to_out:
file_write(outpath, file_read(path_join(path_dirname(inpath), output)))

if not input:
args = (outpath,) + args
elif read_stdout:
elif not output:
args = (inpath,) + args
else:
args = (inpath, outpath) + args
Expand All @@ -76,20 +77,26 @@ def run_test(name, input, output, *args, private=False, check_output=True, from_
success = run_success
stdouts = [run_stdout]

if read_stdout:
file_write_text(outpath, norm_stdout(run_stdout))

if run_success and check_output:
if run_success and output and check_output:
if output_reader(outpath) != output_reader(cmppath):
stdouts.append(f"ERROR: File difference: {outpath}, {cmppath}")
stdouts.append(f"ERROR: Output difference: {outpath}, {cmppath}")
success = False

if run_success and stdout_output:
stdout_outpath = path_join(prefix + "test_output", stdout_output)
stdout_cmppath = path_join(prefix + "test_compare", stdout_output)
run_stdout = norm_stdout(run_stdout)
file_write_text(stdout_outpath, run_stdout)
if run_stdout != try_file_read_text(stdout_cmppath):
stdouts.append(f"ERROR: Stdout difference: {stdout_outpath}, {stdout_cmppath}")
success = False

if run_success and extra_outputs:
for extra_output in extra_outputs:
extra_outpath = path_join(prefix + "test_output", extra_output)
extra_cmppath = path_join(prefix + "test_compare", extra_output)
if try_file_read(extra_outpath) != try_file_read(extra_cmppath):
stdouts.append(f"ERROR: Extra file difference: {extra_outpath}, {extra_cmppath}")
stdouts.append(f"ERROR: Extra output difference: {extra_outpath}, {extra_cmppath}")
success = False

if run_success and g_opts.pico8 and not g_opts.no_pico8 and (pico8_output != None or pico8_output_val != None or pico8_run):
Expand All @@ -108,7 +115,7 @@ def run_test(name, input, output, *args, private=False, check_output=True, from_
print(f"\nERROR - test {name} failed")
print(f"Args: {args}")
print(stdout)
if not read_stdout and not g_opts.no_measure:
if output and not g_opts.no_measure:
measure("new", outpath)
measure("old", cmppath)
fail_test()
Expand All @@ -123,9 +130,6 @@ def run_test(name, input, output, *args, private=False, check_output=True, from_
print(stdout)
return True

def run_stdout_test(name, input, *args, output=None, **kwargs):
run_test(name, input, output, *args, **kwargs, read_stdout=True)

def run():
if run_test("minify", "input.p8", "output.p8", "--minify", "--no-minify-consts",
"--preserve", "*.preserved_key,preserved_glob,preserving_obj.*", pico8_output="output.p8.printh"):
Expand Down Expand Up @@ -157,7 +161,7 @@ def run():
pico8_run=True)
run_test("const2", "const2.p8", "const2.p8", "--minify",
"--no-minify-spaces", "--no-minify-lines", "--no-minify-comments", "--no-minify-rename", "--no-minify-tokens",
pico8_run=True)
pico8_run=True, stdout_output="const2.txt", norm_stdout=norm_paths)
run_test("constcl", "constcl.p8", "constcl.p8", "--minify")
run_test("constcl-1", "constcl.p8", "constcl-1.p8", "--minify", "--const", "DEBUG", "true", "--const", "SPEED", "2.5", "--str-const", "VERSION", "v1.2")
run_test("constcl-2", "constcl.p8", "constcl-2.p8", "--minify", "--const", "DEBUG", "true", "--const", "SPEED", "-2.6", "--const", "hero", "~1")
Expand Down Expand Up @@ -193,16 +197,16 @@ def run():
run_test("default", "default.p8", "default.rom")
run_test("default2", "default2.p8", "default2.rom")
run_test("genend", "genend.p8.png", "genend.p8")
run_stdout_test("lint", "bad.p8", "--lint", output="bad.txt", norm_stdout=norm_paths, exit_code=2)
run_stdout_test("linttab", "bad.p8", "--lint", "--error-format", "tabbed",
output="bad-tab.txt", norm_stdout=norm_paths, exit_code=2)
run_stdout_test("count", "bad.p8", "--count", output="badcount.txt")
run_stdout_test("countminus", "minus.p8", "--count", output="minuscount.txt")
run_stdout_test("error", "worse.p8", "--lint", output="worse.txt", norm_stdout=norm_paths, exit_code=1)
run_test("lint", "bad.p8", None, "--lint", stdout_output="bad.txt", norm_stdout=norm_paths, exit_code=2)
run_test("linttab", "bad.p8", None, "--lint", "--error-format", "tabbed",
stdout_output="bad-tab.txt", norm_stdout=norm_paths, exit_code=2)
run_test("count", "bad.p8", None, "--count", stdout_output="badcount.txt")
run_test("countminus", "minus.p8", None, "--count", stdout_output="minuscount.txt")
run_test("error", "worse.p8", None, "--lint", stdout_output="worse.txt", norm_stdout=norm_paths, exit_code=1)
run_test("script", "script.p8", "script.p8", "--script", path_join("test_input", "my_script.py"),
"--script-args", "my-script-arg", "--my-script-opt", "123")
run_stdout_test("sublang.lint", "sublang.p8", "--lint",
"--script", path_join("test_input", "sublang.py"), output="sublang.txt", norm_stdout=norm_paths, exit_code=2)
run_test("sublang.lint", "sublang.p8", None, "--lint",
"--script", path_join("test_input", "sublang.py"), stdout_output="sublang.txt", norm_stdout=norm_paths, exit_code=2)
run_test("sublang", "sublang.p8", "sublang.p8", "--minify",
"--script", path_join("test_input", "sublang.py"))
run_test("unkform1", "unkform1", "unkform1")
Expand Down
2 changes: 1 addition & 1 deletion shrinko8.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
from pico_constfold import parse_constant
import argparse

k_version = 'v1.2.0b'
k_version = 'v1.2.0c'

def SplitBySeps(val):
return k_hint_split_re.split(val)
Expand Down
5 changes: 5 additions & 0 deletions test_compare/const2.p8
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ __lua__
stop() -- for pure syntax check
_ENV["min"]=123
function x()
?ssog4
function max()
end
local function min()
Expand All @@ -21,3 +22,7 @@ local M,m = max(1,2),min(1,2)
--[[preserve]]ssog1 = 123; ?ssog1
--[[const]]; ?123
--[[const]]ssog3 = 123; ssog3 += 1; ?ssog3
--[[const]]ssog4 = 4; ?ssog4
--[[const]]ssog5 = ssog5; ?ssog5
--[[const]]ssog6 = 123; --[[const]]ssog6 = 456; ?ssog6
--[[const]] ?2
9 changes: 9 additions & 0 deletions test_compare/const2.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
Hint usage warnings:
test_input/const2.p8:7:6: 'ssog4' is marked as constant but is used above where it is assigned to
test_input/const2.p8:12:23: Local 'M' is marked as const but its value cannot be determined due to 'max(1,2)'
test_input/const2.p8:12:25: Local 'm' is marked as const but its value cannot be determined due to 'min(1,2)'
test_input/const2.p8:16:19: Local 'M' is marked as const but its value cannot be determined due to 'max(1,2)'
test_input/const2.p8:24:12: Local 'ssog3' is marked as const but its value cannot be determined due to being reassigned
test_input/const2.p8:26:12: Local 'ssog5' is marked as const but its value cannot be determined due to 'ssog5'
test_input/const2.p8:26:20: 'ssog5' is marked as constant but is used above where it is assigned to
test_input/const2.p8:27:12: Local 'ssog6' is marked as const but its value cannot be determined due to being reassigned
9 changes: 8 additions & 1 deletion test_input/const2.p8
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ __lua__
stop() -- for pure syntax check
_ENV["min"]=123
function x()
?ssog4
function max()
end
local function min()
Expand All @@ -20,4 +21,10 @@ local M,m = max(1,2),min(1,2)
?m
--[[preserve]]ssog1 = 123; ?ssog1
--[[const]]ssog2 = 123; ?ssog2
--[[const]]ssog3 = 123; ssog3 += 1; ?ssog3
--[[const]]ssog3 = 123; ssog3 += 1; ?ssog3
--[[const]]ssog4 = 4; ?ssog4
--[[const]]ssog5 = ssog5; ?ssog5
--[[const]]ssog6 = 123; --[[const]]ssog6 = 456; ?ssog6
--[[const]] local cc = 1
--[[const]] local cc = cc + 1
?cc

0 comments on commit a146c78

Please sign in to comment.