Correctly set eval_from_shared value when new function is created by "new Fun...

Correctly set eval_from_shared value when new function is created by "new Function(...)". At the moment it's set to the native script where the Function is defined(v8natives.js) which doesn't make much sense for the user when he tries to debug his code. Moreover, it causes an exception in JSONProtocolSerializer.prototype.serialize_.

Related Chromium bug: http://crbug.com/29062

Committed: http://code.google.com/p/v8/source/detail?r=3755

[Søren Thygesen Gjesse, 2010-02-01 11:09:05]

LGTM, but it looks as if the added test does not test what the change is about.
As far as I can see the function created through 'new Function' do have
evalFromLocation.

Another question is whether we should add another script type 'NEW_FUNCTION' (or
something like that) to distinguish between new Function(...) and eval(...).
Maybe it makes sense when displaying source in devtools...

http://codereview.chromium.org/551227/diff/1004/3001
File test/mjsunit/debug-compile-event-newfunction.js (right):

http://codereview.chromium.org/551227/diff/1004/3001#newcode49
test/mjsunit/debug-compile-event-newfunction.js:49: assertEquals(62,
evalFromLocation.line);
Why is this 62 and not 63?

[yurys, 2010-02-01 12:55:45]

On 2010/02/01 11:09:05, Søren Gjesse wrote:
> LGTM, but it looks as if the added test does not test what the change is
about.
> As far as I can see the function created through 'new Function' do have
> evalFromLocation.
> 
Now it has evalFromLocation, but without the change in complier.cc it was that
event_data.script().evalFromScript().isUndefined() === true and
event_data.script().evalFromLocation() === undefined.

> Another question is whether we should add another script type 'NEW_FUNCTION'
(or
> something like that) to distinguish between new Function(...) and eval(...).
> Maybe it makes sense when displaying source in devtools...
>
I thought about it and it doesn't seem worth implementing (at least not now).
DevTools don't display any additional information about the eval except for its
source code. But if it showed the eval origin it would be 'new Function(...'
line which would make it obvious that the eval originates from 'new
Function(...)' call.
 
> http://codereview.chromium.org/551227/diff/1004/3001
> File test/mjsunit/debug-compile-event-newfunction.js (right):
> 
> http://codereview.chromium.org/551227/diff/1004/3001#newcode49
> test/mjsunit/debug-compile-event-newfunction.js:49: assertEquals(62,
> evalFromLocation.line);
> Why is this 62 and not 63?

[yurys, 2010-02-01 12:56:11]

http://codereview.chromium.org/551227/diff/1004/3001
File test/mjsunit/debug-compile-event-newfunction.js (right):

http://codereview.chromium.org/551227/diff/1004/3001#newcode49
test/mjsunit/debug-compile-event-newfunction.js:49: assertEquals(62,
evalFromLocation.line);
On 2010/02/01 11:09:05, Søren Gjesse wrote:
> Why is this 62 and not 63?

Oops, I removed one commented line above after running the test, fixed.