RFR 8160034: The `this` value in the `with` is broken by the repetition of a function call
Hannes Wallnöfer
hannes.wallnoefer at oracle.com
Thu Jul 21 11:38:31 UTC 2016
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 <szegedia at gmail.com>:
>
> On 20 Jul 2016, at 12:29, Hannes Wallnöfer <hannes.wallnoefer at oracle.com> wrote:
>>
>> Thanks for the review, Attila. Answer below.
>>
>>> Am 19.07.2016 um 19:04 schrieb Attila Szegedi <szegedia at gmail.com>:
>>>
>>>> On 19 Jul 2016, at 18:20, Hannes Wallnöfer <hannes.wallnoefer at 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.
More information about the nashorn-dev
mailing list