On Wed Jun 7 13:24:47 2023 +0000, Gabriel Ivăncescu wrote:
Well, that ended up not working without adding too much complexity so I gave up. Things like eval() can introduce variables dynamically; currently we detach the scope and use the variable obj, which seamlessly makes it work using the jsdisp's hash lookup (it just adds the new vars as props). It also made dealing with duplicate argument names a challenge, and we need it as that's the whole point of this patch. The double buffer I was thinking of doesn't work either since we need separate ways to access them. But the simplest solution is to just copy the args to the variable obj when the arguments obj is detached, since they're now separate. And that's what I did in the new version. I kept the `buf` in the arguments obj as before, except I split it up into two patches now (it also makes it more obvious why it's simpler to use embedded variable length array). That's because it's now used for all lookups that aren't on the stack, including those that were previously used when looking them up in the variable obj. So it was allocated unconditionally, anyway. Of course it's still used even for detached arguments obj, but in that case the scope has its own copy of the args (those get copied now to the variable obj), so it's correctly looking them up separately. I also added tests to exercise this, as suggested.
It was unconditional only because you made it so in earlier patch. Eg. that patch should really move the allocation to `copy_arguments_from_stack` instead. Also note that in theory vast majority of calls should never need to detach scopes nor arguments (through we likely detach more often than we should).