Correctly compute line numbers in functions from the function constructor.

Correctly compute line numbers in functions from the function constructor.

R=aandrey@chromium.org
BUG=chromium:109362
LOG=Y

Committed: https://code.google.com/p/v8/source/detail?r=25289

[Yang, 2014-11-10 20:16:26]

ping

[aandrey, 2014-11-10 20:38:18]

https://codereview.chromium.org/701093003/diff/1/src/v8natives.js
File src/v8natives.js (left):

https://codereview.chromium.org/701093003/diff/1/src/v8natives.js#oldcode1862
src/v8natives.js:1862: %FunctionMarkNameShouldPrintAsAnonymous(f);
you should also update src/generator.js

https://codereview.chromium.org/701093003/diff/1/test/mjsunit/regress/regress...
File test/mjsunit/regress/regress-crbug-109362.js (right):

https://codereview.chromium.org/701093003/diff/1/test/mjsunit/regress/regress...
test/mjsunit/regress/regress-crbug-109362.js:5: var g = new Function('1 +
reference_error //@ sourceURL=evaltest');
could you also test that:
  eval('(function () {\n 1 + reference_error //@ sourceURL=evaltest2 \n})')

throws at line 2 (not 1)?

[Yang, 2014-11-11 09:51:55]

https://codereview.chromium.org/701093003/diff/1/src/v8natives.js
File src/v8natives.js (left):

https://codereview.chromium.org/701093003/diff/1/src/v8natives.js#oldcode1862
src/v8natives.js:1862: %FunctionMarkNameShouldPrintAsAnonymous(f);
On 2014/11/10 20:38:18, aandrey wrote:
> you should also update src/generator.js

Thanks for catching this!

https://codereview.chromium.org/701093003/diff/1/test/mjsunit/regress/regress...
File test/mjsunit/regress/regress-crbug-109362.js (right):

https://codereview.chromium.org/701093003/diff/1/test/mjsunit/regress/regress...
test/mjsunit/regress/regress-crbug-109362.js:5: var g = new Function('1 +
reference_error //@ sourceURL=evaltest');
On 2014/11/10 20:38:18, aandrey wrote:
> could you also test that:
>   eval('(function () {\n 1 + reference_error //@ sourceURL=evaltest2 \n})')
> 
> throws at line 2 (not 1)?

Done.

[aandrey, 2014-11-11 10:07:38]

https://codereview.chromium.org/701093003/diff/20001/test/mjsunit/regress/reg...
File test/mjsunit/regress/regress-crbug-109362.js (right):

https://codereview.chromium.org/701093003/diff/20001/test/mjsunit/regress/reg...
test/mjsunit/regress/regress-crbug-109362.js:14: var g = new Function('\n 1 +
reference_error //@ sourceURL=evaltest');
I meant:

var g = eval('(function () {\n 1 + reference_error //@ sourceURL=evaltest\n})')

Otherwise, it is the same test case as above.

I'm asking because AFAIU this change will also affect eval(), which is wrong. I
think we should set -1 line offset only for new Function(), but not for eval().

[Yang, 2014-11-11 10:18:12]

On 2014/11/11 10:07:38, aandrey wrote:
>
https://codereview.chromium.org/701093003/diff/20001/test/mjsunit/regress/reg...
> File test/mjsunit/regress/regress-crbug-109362.js (right):
> 
>
https://codereview.chromium.org/701093003/diff/20001/test/mjsunit/regress/reg...
> test/mjsunit/regress/regress-crbug-109362.js:14: var g = new Function('\n 1 +
> reference_error //@ sourceURL=evaltest');
> I meant:
> 
> var g = eval('(function () {\n 1 + reference_error //@
sourceURL=evaltest\n})')
> 
> Otherwise, it is the same test case as above.
> 
> I'm asking because AFAIU this change will also affect eval(), which is wrong.
I
> think we should set -1 line offset only for new Function(), but not for
eval().

It won't affect eval, which doesn't pass true as second argument to
%CompileString. I added the suggested test.

However I reverted the change related to
%FunctionMarkNameShouldPrintAsAnonymous, because the function it should mark is
a different one than what %CompileString returns. It should mark the function
that is returned from executing the function returned by %CompileString.

[aandrey, 2014-11-11 10:25:58]

Nice fix! LGTM

[Yang, 2014-11-12 10:06:32]

Committed patchset #5 (id:80001) manually as 25289 (presubmit successful).

[kozy, 2015-04-01 04:54:33]

A revert of this CL (patchset #5 id:80001) has been created in
https://codereview.chromium.org/1053563002/ by kozyatinskiy@chromium.org.

The reason for reverting is: Locations from New Function are broken in
DevTools..