256836 - Dynamic scope and nested functions

Status: RESOLVED FIXED

(Rhino Graveyard :: Core, enhancement)

Description

[Igor Bukanov]

The dynamic scope in the present form does not work with nested functions
created by functions from shared scope.

Consider the following modification of the example from
http://lxr.mozilla.org/mozilla/source/js/rhino/examples/DynamicScopes.java :

var x = 'sharedScope';
function f() { return x; }
function initClosure(prefix) {
    return function test() { return prefix+x; }
}
closure = initClosure('nested:');

which adds closure variable to the shared scope initialized with the nested
function.

If per-thread source is also modified to include call to closure:

function g() { var x = 'local'; return f(); }
java.lang.System.out.println(g());
function g2() { var x = 'local'; return closure(); }
java.lang.System.out.println(g2());

then the expected behavior for the program is to print 

sharedScope
nested:sharedScope
sharedScope
nested:sharedScope
sharedScope
nested:sharedScope
thread0
nested:thread0
thread1
nested:thread1
thread2
nested:thread2

since the idea is that the variable x should be accessed from the thread scope
both in the case of f() and closure() calls.

In fact the program prints:

sharedScope
nested:sharedScope
sharedScope
nested:sharedScope
sharedScope
nested:sharedScope
thread0
nested:sharedScope
thread1
nested:sharedScope
thread2
nested:sharedScope

which shows that closure() still access the shared scope while looking for x.

The reason for this is that dynamic scoping in the current form does not affect
nested functions since they need to access their lexical scope which defines
parent scope variables.

The proper solution for this would be to change the name lookup to patch the top
scope dynamically. That is the name lookup when it reaches the top scope while
looking for the name in the scope chain should not look in the top scope but
rather switch to the top dynamic scope.

Then the question is what the top dynamic scope should be? The answer can be
simple: it is the top scope of the scope for the first JS stack frame.

Effectively it would mean that that the name resolution when it reaches the top
scope of the scope from the current JS stack frame should ignore it and instead
look for the name in the top scope of the first JS frame.

But then it does not make any sense to have per function flag and instead just
assume one global settings that can always be on to do the right job.

Comment 1

[Igor Bukanov]

Attachment: Implementation

The essential part of the patch is changes to ScriptRuntime.java. The patch
takes advantage of the fact that due to E4X-related changes the top scope of
the first JS frame is stored in Context and Context is passed to name lookup
methods so all the necessary information is readily available for dynamic
alteration of scope chain lookup. 

In addition patched code only switches to the dynamic top scope from the static
top scope only if static scope is present on the prototype chain of the dynamic
scope. In this way only functions defined in the shared scope are affected and
calls across multiple private top-scope would not alter name lookup there.

Then patch deprecates
hasCompileFunctionsWithDynamicScope/setCompileFunctionsWithDynamicScope methods
and points instead to Context.FEATURE_DYNAMIC_SCOPE. Note that the patch does
not change anything regarding setCompileFunctionsWithDynamicScope: the old code
should run as is including nested function problems. 

To take advantage of this the application should explicitly overrides
hasFeature to return true for Context.FEATURE_DYNAMIC_SCOPE as demonstrated in
the updated examples/DynamicScopes.java.

Comment 2

[Igor Bukanov]

Attachment: DynamicScope example from the patch using new API

Comment 3

[Igor Bukanov]

http://www.mir2.org/igor/rhino contains prebuild rhino distro with the patch applied

Comment 4

[Igor Bukanov]

Attachment: Implementation update to reflect CVS changes

Comment 5

[Igor Bukanov]

I committed the patch: folks at helma.org confirmed that it worked for them