[Webkit-unassigned] [Bug 16570] Many toNumber() callers should use a JSValue::getNumber() which ASSERTs instead of branches

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Sat Dec 29 02:37:53 PST 2007


http://bugs.webkit.org/show_bug.cgi?id=16570





------- Comment #3 from darin at apple.com  2007-12-29 02:37 PDT -------
(From update of attachment 18089)
I don't like the naming here. This changes getNumber to be different from
getString in this respect, which doesn't make sense to me.

It's fine if we want a faster version that requires that we already know it's a
number, but I don't think it should take over the "getNumber" name nor do I
think that the "OrNaN" suffix makes it clear what the distinction is.

Since this is not a performance win, it has to pass a pretty high bar of
"clarity win".

This patch also includes the StringImp::value change in strictEqual -- seems
like a good change but not consistent with the title of this bug.

Maybe the name thing would be resolved if we went through the getXXX functions
and created fast ones that assume the type for all of them? Booleans too.

111          double n1 = v1->toNumber(exec);
112          double n2 = v2->toNumber(exec);
 111         double n1 = v1->getNumber();
 112         double n2 = v2->getNumber();
113113         if (n1 == n2)
114114             return true;
115115         return false;

There should be no local variables in that code -- it can all be in one line
without named variables.

118118     else if (t2 == BooleanType)
119119         return v1->toBoolean(exec) == v2->toBoolean(exec);

It's bizarre that this checks t2 instead of t1.


-- 
Configure bugmail: http://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.



More information about the webkit-unassigned mailing list