"use of global <x> is aliased by a local" prevents some compat patterns

[jaawerth]

When intentionally shadowing a global with a local, if the expression refers in any way to the global itself you get a compiler error. For example, this common usecase for writing code that's compatible with Lua 5.1-5.3,

Repro

>> (local unpack (or unpack table.unpack))
Compile error: Compile error in unknown:1
  use of global unpack is aliased by a local

(local unpack (or unpack table.unpack))
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
* Try renaming local unpack.
>> unpack
#<function: 0x561b3d5194e0>
>> (local unpack (or unpack table.unpack))

This assertion was added in 59de14c to get around some possible local/global scope collision issues in the event that the mangled form of a local conflicts with that of a global. While we did know about this when the assertion was added, we decided it was worth adding to avoid bugs on the basis that global-shadowing generally isn't a great idea. However, since there are legitimate usecases like the above, it'd be a good idea to address this in the long run.

One can get around this via (local unpack (or _G.unpack table.unpack)), but this is slightly less robust; it wouldn't work, for example, in a custom environment with unpack on it set via _ENV or setfenv unless it was explicitly put on a new _G. That's an edge case, but one I think would be worth eventually fixing.

When to fix?

This is tied into everything going on in destructure, which we've long wanted to pull apart so it isn't coupled so tightly with emitting Lua. We may need to wait until we've broken destructure up before we can remove this assertion.

[technomancy]

I think the original intent here was to highlight places where it looked like there wouldn't be shadowing, but due to mangling there was. Am I remembering correctly or was that a different error?

At the very least we should update the fennelfriend message to recommend using _G.foo.

[jaawerth]

Ahh, so after digging into the history for a refresher, I believe we left that in there because of the behavior mentioned in this comment in #218. The issue itself was about accidental overwrites due to mangling, but this particular compiler error was put in as a workaround to the edge case mentioned in that comment:

(let [print #(print $)] (print 10))

The problem is that because of how destructure works internally, it doesn't differentiate between let and local.

Now that we have walkTree, we may be able to account for this better with a little static analysis on the AST without going too crazy with extra code just to handle this one edge case.

[technomancy]

I've fixed the fennelfriend entry here to recommend using _G; do we want to release 0.4.2 with that as a "good enough" fix and leave the more invasive destructuring change for later?

[jaawerth]

That seems reasonable. It's possible it'll turn out to be less invasive a change than we think once we start digging into it, but failing that, i'm ok with kicking it to 0.5.0. We can always pull it forward if someone gets motivated to tackle it sooner and the fix turns out to be cleaner than we think.

The only issue with _G is the edgiest of edge cases where you've got something set in a custom environment that isn't part of _G and thus need to check implicit globals. For libraries it's something I would want to be able to account for, but on a day to day coding basis I doubt it comes up super often.