Good comments.


http://codereview.chromium.org/115706/diff/1/2
File src/bootstrapper.cc (right):

http://codereview.chromium.org/115706/diff/1/2#newcode1537
Line 1537: if (!InstallNatives()) return;
I have moved the ifdef inside the function, and even restricted it to
dropping only some of the function body (from where it starts compiling
stuff).

http://codereview.chromium.org/115706/diff/1/4
File src/heap.h (right):

http://codereview.chromium.org/115706/diff/1/4#newcode1052
Line 1052: ASSERT(Page::IsRSetSet(reinterpret_cast<Address>(current),
0));
Done.
Returning true should be a safe approximation (unless someone is
asserting that it returns false somewhere).

http://codereview.chromium.org/115706/diff/1/7
File test/cctest/test-heap.cc (right):

http://codereview.chromium.org/115706/diff/1/7#newcode191
Line 191: ASSERT_EQ(request, OBJECT_SIZE_ALIGN(request));
I couldn't decide. It's not a check in the same sense as the following
ones. They check that something works as intended.  This is more like a
sanity check for the test itself. I.e., if this fails, it's not a bug in
the code being tested, but in the test itself - equivalent to ASSERTs in
the main code.

http://codereview.chromium.org/115706

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

Reply via email to