LGTM with a nit.

https://codereview.chromium.org/21803002/diff/1/src/d8.cc
File src/d8.cc (right):

https://codereview.chromium.org/21803002/diff/1/src/d8.cc#newcode1639
src/d8.cc:1639: virtual void Free(void* data) { UNREACHABLE(); }
On 2013/08/02 11:32:57, Dmitry Lomov (chromium) wrote:
The second overload is a workaround for gcc warning  ‘virtual void
v8::ArrayBuffer::Allocator::Free(void*)’ was hidden
by ‘virtual void v8::ShellArrayBufferAllocator::Free(void*, size_t)’
[-Werror=overloaded-virtual].

It _looks like_ this is not an error for Blink/chromium (as in, I was
able to
build blink and chromium agains this patch on Linux).
Bug https://code.google.com/p/v8/issues/detail?id=2823 tracks cleaning
up the
interface

nit: Let's add a TODO here referring to the issue number. Because
removing the method from the interface will not be enough to find this
override.

https://codereview.chromium.org/21803002/diff/1/test/cctest/cctest.cc
File test/cctest/cctest.cc (right):

https://codereview.chromium.org/21803002/diff/1/test/cctest/cctest.cc#newcode104
test/cctest/cctest.cc:104: virtual void Free(void* data) {
UNREACHABLE(); }
On 2013/08/02 11:32:57, Dmitry Lomov (chromium) wrote:
Workaround for gcc warning, see above

Likewise.

https://codereview.chromium.org/21803002/

--
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
--- You received this message because you are subscribed to the Google Groups "v8-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to [email protected].
For more options, visit https://groups.google.com/groups/opt_out.


Reply via email to