http://chromiumcodereview.appspot.com/10448007/diff/3001/src/splay-tree.h File src/splay-tree.h (right):
http://chromiumcodereview.appspot.com/10448007/diff/3001/src/splay-tree.h#newcode128 src/splay-tree.h:128: UNREACHABLE(); On 2012/05/25 11:03:37, danno wrote:
Why unreachable? Because nodes should only be allocated in Zones? Was
the delete
operator never called before? I still don't see what it's a problem to implemented this with Deallocator().Delete(p);
The nodes are deleted using Deallocator::Delete, which sees a void pointer. Thus, the delete method of a Node should never be called. http://chromiumcodereview.appspot.com/10448007/diff/3001/src/splay-tree.h#newcode209 src/splay-tree.h:209: Deallocator deallocator_; On 2012/05/25 11:03:37, danno wrote:
I don't think you ever need to store the Deallocator as a member. I
think we
must be able to always call the Deallocator constructor with no
arguments. I need to store it in case the we're using the PreallocatedStorageAllocationPolicy and want to have the isolate around for deletion without a TLS lookup. Right now it does a TLS lookup. As long as the destructor doesn't do anything funny, I think we should be able to reinterpret_cast the void* in the object's `operator delete' to a pointer to the object, extract the deallocator and call the Destroy (of the Deallocator instance) method on the pointer. http://chromiumcodereview.appspot.com/10448007/diff/3001/src/v8.h File src/v8.h (right): http://chromiumcodereview.appspot.com/10448007/diff/3001/src/v8.h#newcode68 src/v8.h:68: #include "zone-inl.h" On 2012/05/25 11:03:37, danno wrote:
What has changed that you need this extra include?
I get a "inline function `void* v8::internal::ZoneAllocator::New(int)` used but never defined" error when I don't add this. I've checked the preprocessed source files, but didn't notice anything suspicious. http://chromiumcodereview.appspot.com/10448007/ -- v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev
