Type checker leaks scope on result-after-checks

[jpolitz]

This program has a type error and shouldn't, due to some environment leak:

var result-after-checks = 10

result-after-checks := 11

result-after-checks

[blerner]

Not at my computer so can't check, but does it give bogus results in not-tc mode if you have a check block, and also define this particular variable? It might be a name-resolution or -clobbering bug...

[jpolitz]

This was so hard to reproduce offline!

Here were the steps:

⤇ touch *checks*.arr
⤇ cat checks.arr
provide *
check:
2 + 2 is 4
end
⤇ cat checks2.arr
provide *
check:
2 + 2 is 4
end
⤇ cat import-checks.arr
include file("checks.arr")
include file("checks2.arr")
⤇ env EF=" " make checks.jarr
node -max-old-space-size=8192 build/phaseA/pyret.jarr --outfile checks.jarr \
                      --build-runnable checks.arr \
                      --builtin-js-dir src/js/trove/ \
                      --builtin-arr-dir src/arr/trove/ \
                      --compiled-dir compiled/ \
                        \
                      --require-config src/scripts/standalone-configA.json
26/26 modules compiled (checks.arr)
Cleaning up and generating standalone...
/Users/joe/src/pyret-lang
⤇ env EF=" " make checks2.jarr
node -max-old-space-size=8192 build/phaseA/pyret.jarr --outfile checks2.jarr \
                      --build-runnable checks2.arr \
                      --builtin-js-dir src/js/trove/ \
                      --builtin-arr-dir src/arr/trove/ \
                      --compiled-dir compiled/ \
                        \
                      --require-config src/scripts/standalone-configA.json
26/26 modules compiled (checks2.arr)
Cleaning up and generating standalone...
/Users/joe/src/pyret-lang
⤇ make import-checks.jarr
node -max-old-space-size=8192 build/phaseA/pyret.jarr --outfile import-checks.jarr \
                      --build-runnable import-checks.arr \
                      --builtin-js-dir src/js/trove/ \
                      --builtin-arr-dir src/arr/trove/ \
                      --compiled-dir compiled/ \
                      -no-check-mode \
                      --require-config src/scripts/standalone-configA.json
28/28 modules compiled (import-checks.arr)
Cleaning up and generating standalone...
0The declaration of `result-after-checks1` at file:///Users/joe/src/pyret-lang/import-checks.arr:2:0-2:27 shadows a previous declaration of `result-after-checks1` defined at file:///Users/joe/src/pyret-lang/checks.arr:2:0-5:3 and imported from file:///Users/joe/src/pyret-lang/import-checks.arr:1:0-1:26
The run ended in error:
There were compilation errors
Pyret stack:
  file:///Users/joe/src/pyret-lang/src/arr/compiler/cli-module-loader.arr: line 320, column 2
  file:///Users/joe/src/pyret-lang/src/arr/compiler/pyret.arr: line 130, column 10
  file:///Users/joe/src/pyret-lang/src/arr/compiler/pyret.arr: line 92, column 4
  file:///Users/joe/src/pyret-lang/src/arr/compiler/pyret.arr: line 231, column 12
make: *** [import-checks.jarr] Error 1

That order matters. If you just compile import-checks.arr, the gensym counter in memory won't cause the collision! It has to be deterministic gensyms from the start of the compiler. Sigh. The collisions happen a lot though because check blocks are so early in the process. However, the fact that there need to be (a) multiple includes that (b) were compiled with check mode and have check blocks and (c) were compiled in different compilation passes makes this hard to detect. If you didn't include them and instead imported, no shadowing error. If you didn't compile with check mode (the default for library files, but not main files, in CPO!), no results-after-checks defined. If you compiled all at once after making an edit, you wouldn't see the issue.

However, this happens online because you might open the library file, run it, get the cached copy saved in code.pyret.org.compiled, then do it in the other library, then come back to the main and run.

[blerner]

Does @MichaelMMacLeod 's fix above actually fix it? The fix seems to make an internal name, which (at least on a quick glance) seems independent of gensym numbering issues.

Also -- do we want to consider resetting the gensym counter more often during compilation, to ensure per-file determinism? (Dunno how much that would break, but it would probably be worth finding out!)

[jpolitz]

Gah, I thought this was the fix:

https://github.com/brownplt/pyret-lang/commit/3e6b258703ab074276b8e1f80df32cdf15fba251

But it clearly messed with some subtle scope thing:

https://travis-ci.com/github/brownplt/pyret-lang/builds/214479009