Added test case for non-zero column offset and sure enough fixed another
issue.
I will be sure to be better about proper test coverage in future patches.
http://codereview.chromium.org/1985004/diff/2002/9002
File src/top.cc (right):
http://codereview.chromium.org/1985004/diff/2002/9002#newcode396
src/top.cc:396: int line_number =
GetScriptLineNumber(Handle<Script>(script), position);
On 2010/05/07 07:57:06, Søren Gjesse wrote:
How about adding a function GetScriptLocalLineNumber which does not
take the
offset into account?
Done.
http://codereview.chromium.org/1985004/diff/2002/9002#newcode422
src/top.cc:422: if (fun_name->ToBoolean()->IsFalse()) {
On 2010/05/07 11:39:52, Michail Naganov wrote:
On 2010/05/07 07:57:06, Søren Gjesse wrote:
> Any reason for changing this?
>
> I just looked at the code, and we have at least three ways of
checking this
>
> if (!fun_name->IsString())
> if (fun_name->IsUndefined())
> if (fun_name->ToBoolean()->IsFalse())
>
> I think we should be consistent, so I suggest adding a function
called
something
> likeGetNameOrInferredName/GetDisplayName/GetFriendlyName to
JSFunction and use
> that in all places where the inferred name is returned if no actual
name is
> found.
I can do this as a separate change. But I'm too not quite
understanding, why use
such extravagant "ToBoolean()->IsFalse()" check. Using
"shared->name()->IsUndefined()" is more straightforward.
Because it is not undefined. name and inferred name get set to the empty
string initially :(. It tripped me up too.
The empty string in javascript boolean coerces to false. In fact, any
whitespace only string boolean coerces to false.
("" == false) == true;
("\r\n" == false) == true;
(" " == false) == true;
Gross eh?
Also, I agree with Michail that we should normalize all the getters for
a function name in a separate change. This one should go in soon since
it fixed API brokenness.
http://codereview.chromium.org/1985004/show
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
