Addressed feedback. Merged latest changes (which probably I should have done after the review).
http://codereview.chromium.org/7991007/diff/1/src/objects.cc File src/objects.cc (right): http://codereview.chromium.org/7991007/diff/1/src/objects.cc#newcode2004 src/objects.cc:2004: name, On 2011/09/26 10:37:36, Rico wrote:
indention
Done. http://codereview.chromium.org/7991007/diff/1/src/objects.h File src/objects.h (right): http://codereview.chromium.org/7991007/diff/1/src/objects.h#newcode2008 src/objects.h:2008: On 2011/09/26 10:37:36, Rico wrote:
Could we add a comment here stating that this may return found=true
even if this
is giving back a failure, I at least had to convince myself that in
case of GC
during the callwould always be safe. Also, this would be nice to know
for other
callsites in the future (i.e., that even when found, they should check
the
result if they do not immediately return it)
Done. http://codereview.chromium.org/7991007/diff/1/src/objects.h#newcode2009 src/objects.h:2009: MUST_USE_RESULT MaybeObject* SetPropertyInPrototypes( On 2011/09/26 10:37:36, Rico wrote:
could we use a more descriptive name for this? Like, SetPropertyWithCallbackSetterInPrototypes
Done. http://codereview.chromium.org/7991007/diff/2002/src/objects.cc File src/objects.cc (right): http://codereview.chromium.org/7991007/diff/2002/src/objects.cc#newcode2023 src/objects.cc:2023: bool found = false; On 2011/09/26 10:40:42, Rico wrote:
Move comment about using handlified references up to here, state that SetPropertyWithHandlerIfDefiningSetter can cause GC
Done. http://codereview.chromium.org/7991007/diff/2002/test/cctest/test-api.cc File test/cctest/test-api.cc (right): http://codereview.chromium.org/7991007/diff/2002/test/cctest/test-api.cc#newcode1340 test/cctest/test-api.cc:1340: return self->GetHiddenValue(v8_str(name_str+i)); On 2011/09/26 10:37:36, Rico wrote:
space around +
Done. http://codereview.chromium.org/7991007/diff/2002/test/cctest/test-api.cc#newcode1437 test/cctest/test-api.cc:1437: // All i < 10000 go to interceptor. On 2011/09/26 10:37:36, Rico wrote:
go to interceptor -> go to the interceptor same below and same in next two functions
Done. http://codereview.chromium.org/7991007/ -- v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev
