http://chromiumcodereview.appspot.com/10448007/diff/3001/src/list.h File src/list.h (right):
http://chromiumcodereview.appspot.com/10448007/diff/3001/src/list.h#newcode52 src/list.h:52: typedef typename P::Alloc Allocator; AllocationPolicy instead of P http://chromiumcodereview.appspot.com/10448007/diff/3001/src/splay-tree-inl.h File src/splay-tree-inl.h (right): http://chromiumcodereview.appspot.com/10448007/diff/3001/src/splay-tree-inl.h#newcode37 src/splay-tree-inl.h:37: template<typename Config, class P> AllocatorPolicy, or Policy (P is not very descriptive). Also elsewhere in this file. 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#newcode53 src/splay-tree.h:53: template <typename Config, class P> AllocationPolicy http://chromiumcodereview.appspot.com/10448007/diff/3001/src/splay-tree.h#newcode73 src/splay-tree.h:73: Deallocator deallocator = Deallocator())) { Adding the deallocator here does something different than you think. In this case, you cannot override the deallocator, since there is no way to pass it in a call to delete. So either you want "return Deallocator().Delete(p), or you want to put an "UNREACHABLE" here like you do later in the file. http://chromiumcodereview.appspot.com/10448007/diff/3001/src/splay-tree.h#newcode128 src/splay-tree.h:128: UNREACHABLE(); 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); http://chromiumcodereview.appspot.com/10448007/diff/3001/src/splay-tree.h#newcode209 src/splay-tree.h:209: Deallocator deallocator_; 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. 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" What has changed that you need this extra include? http://chromiumcodereview.appspot.com/10448007/diff/3001/src/zone-inl.h File src/zone-inl.h (right): http://chromiumcodereview.appspot.com/10448007/diff/3001/src/zone-inl.h#newcode115 src/zone-inl.h:115: // allocate ZoneLists and their elements in the current Zone. Does the comment apply to the following code? I'm confused a bit. http://chromiumcodereview.appspot.com/10448007/diff/3001/src/zone-inl.h#newcode122 src/zone-inl.h:122: // De-allocation attempts are silently ignored. This comment seems unrelated? http://chromiumcodereview.appspot.com/10448007/diff/3001/src/zone-inl.h#newcode125 src/zone-inl.h:125: return List<T, ZoneAllocationPolicy>::operator new(size); Why did you move this function? And why does it need to change (you will be eliminating it in the next step anyway, and this seems premature and unrelated to the other changes you are making in this CL. http://chromiumcodereview.appspot.com/10448007/diff/3001/src/zone.h File src/zone.h (right): http://chromiumcodereview.appspot.com/10448007/diff/3001/src/zone.h#newcode180 src/zone.h:180: typedef ZoneAllocator Alloc; Please use the full words for "Allocator" and "Deallocator" http://chromiumcodereview.appspot.com/10448007/diff/3001/test/cctest/test-list.cc File test/cctest/test-list.cc (right): http://chromiumcodereview.appspot.com/10448007/diff/3001/test/cctest/test-list.cc#newcode61 test/cctest/test-list.cc:61: typedef ZeroingAllocator Alloc; Full "Allocator" and "Deallocator" http://chromiumcodereview.appspot.com/10448007/ -- v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev
