After trying various things, I think the best solution is to introduce a new „internal“ ScriptObject flag to avoid leaking the global lexical scope (and possibly other internal objects) to scripts. In addition to Global.LexicalScope I also set this flag on WithObjects themselves, although that shouldn’t be required for the current case.
The new webrev is here, please review: http://cr.openjdk.java.net/~hannesw/8160034/webrev.01/ Hannes > Am 20.07.2016 um 13:24 schrieb Attila Szegedi <szege...@gmail.com>: > > On 20 Jul 2016, at 12:29, Hannes Wallnöfer <hannes.wallnoe...@oracle.com> > wrote: >> >> Thanks for the review, Attila. Answer below. >> >>> Am 19.07.2016 um 19:04 schrieb Attila Szegedi <szege...@gmail.com>: >>> >>>> On 19 Jul 2016, at 18:20, Hannes Wallnöfer <hannes.wallnoe...@oracle.com> >>>> wrote: >>>> >>>> Please review: >>>> >>>> Webrev: http://cr.openjdk.java.net/~hannesw/8160034/webrev.00/ >>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8160034 >>>> >>>> The change in ScriptObject.megamorphicGet actually fixes the bug. It took >>>> me quite some time that it could be fixed so elegantly at this level. >>>> Before that, I tried to fix it on the Method handle level in WithObject. >>> >>> So… would you say this is a solution or a workaround? What I mean is that >>> it is a nicely small change, but is it correct in all cases? Maybe it >>> should be further constrained by both isMethod and isScope? (Not sure >>> whether there’s a situation where it’s !isScope and would be incorrect, >>> though) >>> >> >> Excellent questions! > > :-) Thanks. > >> I was convinced this is correct in all cases, and that in fact with >> statements are the only occurrence of such FindProperties. But your remark >> made me check this and indeed the same pattern occurs for global lexical >> declarations in Global.LexicalScope, and in that case this behavior is not >> desired. > > Frankly, I had something else in mind (I was thinking issues with functions > defined in prototypes of the object used with “with”), but I re-read the > relevant code for FindProperty and realized that I was mixing up the roles of > “self" and “prototype” there. But if I nudged you into checking something > else, great :-) > > As far as I’m concerned, upon further reading the FindProperty and > ScriptObject code, +1 from me, unless you find that you need to adjust things > for lexical scope. > >> Although I wasn’t able to reproduce this with lexical bindings, I’ll have to >> do something to make sure functions are only bound when they’re meant to. >> >> Thanks, >> Hannes > > Cheers, > Attila.
