On 06/05/2022 02:56, Jacek Caban wrote:
On 5/6/22 01:50, Jacek Caban wrote:
Honestly, I don't particularly care about toString, it's just the only builtin I could think off the top of my head that could be tested for this. Because right now we handle it in exec_source, which is only for interpreted functions... but it's a good test case of such behavior.
Do you think a fix like in 2/2 is more appropriate (also for quirks modes, but with null instead of undefined)? And perhaps fix whatever is causing toString to fail, which seems unrelated I guess.
That might mean moving it out of exec_source though.
Same story there, why do you think it's somehow related to detached scopes? The attached test fails with your patch.
I attached a wrong test, here is the right one.
Jacek
Because without handling detached scopes we actually *trigger the assertion* so it's quite bad. But that's just for toString, it's not really important. I've added a test with hasOwnProperty now, which *must* fail on detached scopes, but it doesn't currently (since they're js objects). They need special handling either way.
The attached test looks to me like a different issue: it passes null instead of undefined, which is an ES5 specific thing, and probably deserves a separate fix. It's not much of a problem either way.
Note that we are *already* handling detached scopes in exec_source, so it's not like I'm adding custom code to deal with that separately; we already do. I'm just moving it out of exec_source because:
1) Nothing in the (specified) spec says it's only for source functions. 2) This will become a problem with proxy functions later (which is where it's needed); I'd rather not duplicate that logic in them for no reason. 3) We are already handling it in exec_source "as a special case".
I'm trying to find fix for the attach problem and send it separately but honestly I can't see how such a fix would fix detached scopes at all.