http://codereview.chromium.org/1985004/diff/2002/9002
File src/top.cc (right):
http://codereview.chromium.org/1985004/diff/2002/9002#newcode422
src/top.cc:422: if (fun_name->ToBoolean()->IsFalse()) {
On 2010/05/07 15:25:27, Michail Naganov wrote:
On 2010/05/07 14:49:42, jaimeyap wrote:
> 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.
Yeah, I knew that. I'm sorry, I looked into somebody's wrong code
which checks
for undefined. Indeed, function's name is usually a string and it's
empty if
it's anonymous. This is another point for introducing a function that
returns a
"friendly" name -- there is already a piece of code that seem to do it
in a
wrong way (Accessors::ScriptGetEvalFromFunctionName). I meant that
converting a
string to boolean is an overkill, since parser already does a good job
in
squeezing whitespaces, so function's name can't be e.g. " " (two
spaces), it
can only have zero length, if it's anonymous.
Im going to go with the principle of least surprise. If in JS I write:
var name = fun.name() || fun.inferred_name();
It is doing a proper boolean coercion. I would hate to be sensitive to a
change down the line in the default for name that boolean coerces but
isn't exactly the emtpy string :).
In any case, I think we should figure out the correct semantics for
"friendly name" when we normalize all the callers to use this shared
getter (in a subsequent patch).
http://codereview.chromium.org/1985004/show
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
