On Fri Jun 16 16:08:09 2023 +0000, Jacek Caban wrote:
Unless I'm missing something, we currently need function-named property in variable object only because `identifier_eval` can't handle it otherwise for detached scopes. But if we make `identifier_eval` able to handle it for detached cases, there is no need to involve variable object. BTW, I know that it's quite a large side quest for what you intended to fix and it's fine if you'd limit the scope only to arguments. I'm mentioning it because, due to above, the patch using scope object as variable object looks questionable to me, but you don't really need that part for the original bug.
Yeah, using the scope object was just an optimization I wanted to make since the variable object is barely used now in such cases, so it feels like a bit of a waste.
But as far as I can see `identifier_eval` only treats the "arguments" and the function's name itself as special, though? What I'm talking about is functions in nested scopes getting exposed on the base scope. This call in `detach_scope`:
```C jsdisp_propput_name(frame->variable_obj, name, ctx->stack[local_off(frame, ref)]); ```
or the equivalent in `scope_init_locals` when `detached_vars` is true.
An example of such tests are in "functions scope" test in mshtml/es5.js, at the beginning of it. Even though it exposes them using same name, so base scope does have locals named `f`, but that's just coincidence I think.
That said, probably that should go into separate follow up MR, anyway.
Anyway, I'll leave the scope obj optimization out of this MR, do you think I should resend it later or you still don't like it?