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

[Hannes Wallnöfer]

+1, and I like the fact that the patch has become smaller.

Hannes


> Am 12.12.2017 um 13:32 schrieb Priya Lakshmi Muthuswamy <priya.lakshmi.muthuswamy at oracle.com>:
> 
> 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
>>>> 
>>>> 
>> 
>