Reassignment of retained vars within one repl invocation causes UnboundLocalError

[ioistired]

Summary

Reassignment of retained variables causes UnboundLocalError

Reproduction steps

1. jsk retain 1
2. jsk py x = 1
3. jsk py x = x + 1

Expected results

x is reassigned to 2.

Actual results

Traceback (most recent call last):
  File ".venv/lib/python3.8/site-packages/jishaku/cog_base.py", line 494, in jsk_python
    async for send, result in AsyncSender(executor):
  File ".venv/lib/python3.8/site-packages/jishaku/functools.py", line 109, in _internal
    value = await base.asend(self.send_value)
  File ".venv/lib/python3.8/site-packages/jishaku/repl/compilation.py", line 151, in traverse
    yield await func(*self.args)
  File "<repl>", line 1, in _repl_coroutine
UnboundLocalError: local variable 'x' referenced before assignment

Checklist

• I have updated discord.py and jishaku to the latest available versions and have confirmed that this issue is still present
• I have searched the open issues for duplicates
• I have shown the entire traceback, if possible (other than abbreviating path names)
• I have removed my token from display, if visible

System information

• Python v3.8.0-final
• discord.py v1.3.0-alpha
  • discord.py pkg_resources: v1.3.0a2146+g4ef0fb0
• aiohttp v3.6.2
• websockets v6.0
• system info: Linux 4.19.0-6-amd64 #​1 SMP Debian 4.19.67-2+deb10u1 (2019-09-20)
• jishaku v1.17.0.172

Remarks

This is apparently due to x being a global name. Ideally we could pass in locals to the user-defined code, but this is not possible, even with PyCF_ALLOW_TOP_LEVEL_AWAIT. Fixing this might require transforming all name lookups to globals()['x'] if x in globals() else x.

[scarletcafe]

This is, as remarked, difficult to action upon, because to solve it completely, we'd need a different method of retaining scopes.

In terms of things we can do without modifying code, we're unable to unpack into locals, and globals gives us this issue, because there is now a discrepancy between the 'local x' that we're defining and the 'global `x``' we're evaluating from.

I don't much like the concept of converting all name lookups - it's not really solving the problem as much as silently inserting behavior that masks it, and it's probably liable to break something elsewhere.

The 'safe' way to insert locals would just be to pass extra arguments - this is how stuff like _ctx is handled. My problem with this is I don't much like generating plain text code from user data - someone could do something like globals()["a x"] = 3 and make the command unusable.

I'm thinking a possible way of getting around this and all other scoping problems is probably just to mess with AST. It could be realistically feasible to either generate arguments using AST, or to pad the beginning of the function with x = previous_scope.locals['x']-type generation manually. I don't really like it too much as a solution, but then again, I'm not too much of a fan of the existing global system either.