RFR:8191301:JavaImporter fails to resolve imported elements within functions, that contain too many statements

Priya Lakshmi Muthuswamy priya.lakshmi.muthuswamy at oracle.com
Tue Dec 12 12:32:36 UTC 2017


Hi,

Kindly review. I have modified the fix to work with multiple with scopes.

webrev : http://cr.openjdk.java.net/~pmuthuswamy/8191301/webrev.01/

Thanks,
Priya
On 12/5/2017 12:54 PM, Priya Lakshmi Muthuswamy wrote:
> Hi Attila,
>
> Thanks for review.
> Yes when I checked with two with scopes as suggested(JavaImporter as 
> outer), current fix doesn't work. I will work on that.
>
> Thanks,
> Priya
> On 12/5/2017 12:12 PM, Attila Szegedi wrote:
>> Hm… this seems to be an issue with shared scope calls; that’s why 
>> it’s sensitive to the number of similar statements.
>>
>> That said, here’s some thoughts:
>>
>> 1. Instead of
>>
>>     if (self instanceof ScriptObject && 
>> ((ScriptObject)self).hasWithScope()) {
>>
>> you should be able to just use
>>
>>     if (self instanceof ScriptObject) {
>>
>> As then you’ll evaluate getWithScopeObject and test it for being null 
>> anyway. This way, you avoid walking the prototype chain twice.
>>
>> 2. That said, I guess hasWithScope could be reimplemented simply as
>>
>>     public boolean hasWithScope() {
>>         return getWithScopeObject() != null;
>>     }
>>
>> as both have very similar code, so it’d reduce to it nicely. (I 
>> understand that you haven’t changed that, but since you were in the 
>> vicinity of that code, you might as wel do it… It’s also fine if you 
>> leave it alone as it is.)
>>
>> 3. One of the statements in the test is indented differently than the 
>> others.
>>
>> 4. What happens if there’s _two_ with scopes, and the JavaImporter is 
>> in the outer one? Does this fix still work? E.g.:
>>
>> var imports = new JavaImporter(java.lang);
>> var dummy = { x: 42, y: 13 }
>> with (imports) {
>>      with (dummy) {
>>          function func() {
>>              System.out.println('a');
>>             System.out.println('a');
>>         System.out.println('a');
>>              System.out.println('a');
>>             System.out.println('a');
>>         System.out.println('a');
>>             System.out.println('a');
>>          };
>>          func();
>>     }
>> }
>>
>> Attila.
>>
>>> On Dec 5, 2017, at 7:13 AM, Priya Lakshmi Muthuswamy 
>>> <priya.lakshmi.muthuswamy at oracle.com> wrote:
>>>
>>> Hi,
>>>
>>> please review JDK-8191301 : JavaImporter fails to resolve imported 
>>> elements within functions, that contain too many statements
>>>
>>> JBS : https://bugs.openjdk.java.net/browse/JDK-8191301
>>> webrev : http://cr.openjdk.java.net/~pmuthuswamy/8191301/webrev.00/
>>>
>>> Thanks,
>>> Priya
>>>
>>>
>



More information about the nashorn-dev mailing list