184107 - with(...) { function f ...} should set f in the global scope

Status: VERIFIED FIXED

(Rhino Graveyard :: Core, defect)

Description

[Igor Bukanov]

User-Agent:       Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.0.1) Gecko/20021003
Build Identifier: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.0.1) Gecko/20021003

When evaluationg the following test:

var x = {y:10};
with (x) {
	function f()  {
		return y;
	}
}

var result = f();

var ok1 = (x.f === undefined)
print(ok1)

var ok2 = (result === x.y);
print(ok2)


Rhino incorrectly put f into the x object instead of the global scope. 

The test would work as expected if function f()... would be replaced by f =
function()...



Reproducible: Always

Steps to Reproduce:

Comment 1

[Phil Schwartau]

Testcase added to JS testsuite:

      mozilla/js/tests/js1_5/Scope/regress-184107.js

Currently passing in SpiderMonkey. In Rhino, the test fails in both
compiled and interpreted modes. As Igor states, function expressions

      f = function()...

are correctly created by Rhino in global scope, but function statements

      function f() ...

are being created in local scope.

Comment 2

[Igor Bukanov]

Attachment: putting function expression statements always into activation object ignoring scopes introduced by with

After the patch js1_5/Scope/regress-185485.js will fail as the patch follows SM
path there and SM copes with this Ecma extension in a different way as Netscape
4.* and it was decided that it was OK to keep the current SM behavior in this
obscure case.

Comment 3

[Igor Bukanov]

I commited the fix.

Comment 4

[Phil Schwartau]

Verified FIXED. 

The above testcase, js1_5/Scope/regress-184107.js, now passes in 
both Rhino interpreted and comiled modes.

The testcase js1_5/Scope/regress-185485.js has now been corrected.
It now passes in Rhino as well as in SpiderMonkey -

Comment 5

[Igor Bukanov]

Targeting as resolved against 1.5R4