Thanks a lot for comments, Mads.  Hopefully all of them are responded.


http://codereview.chromium.org/660298/diff/33/1027
File src/bootstrapper.cc (right):

http://codereview.chromium.org/660298/diff/33/1027#newcode1477
src/bootstrapper.cc:1477: Object* old_value =
prototype->GetProperty(*key);
On 2010/03/08 11:53:01, Mads Ager wrote:
Since everything else is in handles here, could you use the
handle-based version
of GetProperty as well?

Handle<Object> old_value = GetProperty(prototype, key);

Done.

http://codereview.chromium.org/660298/diff/33/1027#newcode1478
src/bootstrapper.cc:1478: // On ARM we sometimes call this method before
installing natives.
On 2010/03/08 11:53:01, Mads Ager wrote:
But we do call it after installing natives as well, right?  Why do we
call it
before installing natives on ARM and not on the other platforms? I
find that a
little puzzling.

Sorry, comment was somewhat misleading, I've tried to adjust it.

The story: there are test/cctest/test-assembler-arm tests which set
FLAG_natives_file to empty string thus bypassing JS natives
installation.  However that's apparently just an example of more common
problem if someone for some strange reason overrides natives
installation.

BTW, I wonder if InstallNatives should return true in this case.

http://codereview.chromium.org/660298/diff/33/1028
File src/builtins.cc (right):

http://codereview.chromium.org/660298/diff/33/1028#newcode322
src/builtins.cc:322: class FastElementsArrayCheck {
On 2010/03/08 11:53:01, Mads Ager wrote:
I would make this a static method 'IsJSArrayWithFastElements' with a
bool result
type and a couple of out parameters.

Done.

http://codereview.chromium.org/660298

--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev

Reply via email to