On 2010/05/21 08:09:29, Mads Ager wrote:
LGTM
Could you update the change with these comments and I will land the
change for
you. Did the use of the PreCompilation API land? If it did, we need to
coordinate landing this change so we don't break Chromium when pushing a
new
version of V8.
No, that patch is pending here:
https://bugs.webkit.org/show_bug.cgi?id=38661
The way it is written now it uses the old API, if you look it'll show why I
wanted to change the API first. I'll update it after this rolls. By the
way, do
you know how rolling V8 works and/or when this might roll up into Chromium?
http://codereview.chromium.org/2118010/diff/1/4
File src/api.cc (right):
http://codereview.chromium.org/2118010/diff/1/4#newcode1110
src/api.cc:1110: static const int kCharToUnsignedFactor =
sizeof(unsigned) /
sizeof(char);
Just use sizeof(unsigned)?
I guess if char isn't 1 byte then all bets are off anyway. Done.
http://codereview.chromium.org/2118010/diff/1/4#newcode1113
src/api.cc:1113: // Note that Script::New() will ASSERT if this empty
ScriptData
is used.
This note does not seem necessary. Many API entries rely on the user
passing
in
non-null handles.
Removed.
http://codereview.chromium.org/2118010/diff/1/4#newcode1120
src/api.cc:1120: // will fail causing Script::New() to ASSERT.
Is the note needed? Your API comment already says that the ScriptData is
architecture dependent.
Removed.
http://codereview.chromium.org/2118010/diff/1/2
File test/cctest/test-api.cc (right):
http://codereview.chromium.org/2118010/diff/1/2#newcode8039
test/cctest/test-api.cc:8039: const char *script = "function foo(a) {
return
a+1; }";
Placement of the '*' here and in the rest of the test code.
Done. I also cleaned up the other Precompile* test cases where this was
branched
from.
http://codereview.chromium.org/2118010/show
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
