From 72b2128e6e5dbbc99464d0345fcd4c6692b078c3 Mon Sep 17 00:00:00 2001 From: Vladimir Panteleev Date: Tue, 15 May 2018 04:47:07 +0000 Subject: [PATCH] Fix Issue 18847 - std.allocator: Region uses .parent before it can be set --- .../building_blocks/bitmapped_block.d | 76 ++++++++++++++++++- .../building_blocks/kernighan_ritchie.d | 11 ++- .../allocator/building_blocks/region.d | 10 ++- 3 files changed, 91 insertions(+), 6 deletions(-) diff --git a/std/experimental/allocator/building_blocks/bitmapped_block.d b/std/experimental/allocator/building_blocks/bitmapped_block.d index f269901963d..48ab82bf50c 100644 --- a/std/experimental/allocator/building_blocks/bitmapped_block.d +++ b/std/experimental/allocator/building_blocks/bitmapped_block.d @@ -156,7 +156,7 @@ private mixin template BitmappedBlockImpl(bool isShared, bool multiBlock) this(data); } - static if (!is(ParentAllocator == NullAllocator)) + static if (!is(ParentAllocator == NullAllocator) && !stateSize!ParentAllocator) this(size_t capacity) { size_t toAllocate = totalAllocation(capacity); @@ -165,14 +165,33 @@ private mixin template BitmappedBlockImpl(bool isShared, bool multiBlock) assert(_blocks * blockSize >= capacity); } + static if (!is(ParentAllocator == NullAllocator) && stateSize!ParentAllocator) + this(ParentAllocator parent, size_t capacity) + { + this.parent = parent; + size_t toAllocate = totalAllocation(capacity); + auto data = cast(ubyte[])(parent.allocate(toAllocate)); + this(data); + } + static if (!is(ParentAllocator == NullAllocator) && - chooseAtRuntime == theBlockSize) + chooseAtRuntime == theBlockSize && + !stateSize!ParentAllocator) this(size_t capacity, uint blockSize) { this._blockSize = blockSize; this(capacity); } + static if (!is(ParentAllocator == NullAllocator) && + chooseAtRuntime == theBlockSize && + stateSize!ParentAllocator) + this(ParentAllocator parent, size_t capacity, uint blockSize) + { + this._blockSize = blockSize; + this(parent, capacity); + } + static if (!is(ParentAllocator == NullAllocator) && hasMember!(ParentAllocator, "deallocate")) ~this() @@ -1212,9 +1231,15 @@ struct BitmappedBlock(size_t theBlockSize, uint theAlignment = platformAlignment /// Ditto this(size_t capacity); + /// Ditto + this(ParentAllocator parent, size_t capacity); + /// Ditto this(size_t capacity, uint blockSize); + /// Ditto + this(ParentAllocator parent, size_t capacity, uint blockSize); + /** If `blockSize == chooseAtRuntime`, `BitmappedBlock` offers a read/write property `blockSize`. It must be set before any use of the allocator. @@ -1435,6 +1460,15 @@ struct BitmappedBlock(size_t theBlockSize, uint theAlignment = platformAlignment assert(a.deallocate(buf)); } +// Test instantiation with stateful allocators +@system unittest +{ + import std.experimental.allocator.mallocator : Mallocator; + import std.experimental.allocator.building_blocks.region : Region; + auto r = Region!Mallocator(1024 * 96); + auto a = BitmappedBlock!(chooseAtRuntime, 8, Region!Mallocator*, No.multiblock)(&r, 1024 * 64, 1024); +} + /** The threadsafe version of the $(LREF BitmappedBlock). The semantics of the `SharedBitmappedBlock` are identical to the regular $(LREF BitmappedBlock). @@ -1478,9 +1512,15 @@ shared struct SharedBitmappedBlock(size_t theBlockSize, uint theAlignment = plat /// Ditto this(size_t capacity); + /// Ditto + this(ParentAllocator parent, size_t capacity); + /// Ditto this(size_t capacity, uint blockSize); + /// Ditto + this(ParentAllocator parent, size_t capacity, uint blockSize); + /** If `blockSize == chooseAtRuntime`, `SharedBitmappedBlock` offers a read/write property `blockSize`. It must be set before any use of the allocator. @@ -2125,6 +2165,8 @@ struct BitmappedBlockWithInternalPointers( { import std.conv : text; import std.typecons : Ternary; + + static if (!stateSize!ParentAllocator) @system unittest { import std.experimental.allocator.mallocator : AlignedMallocator; @@ -2135,7 +2177,7 @@ struct BitmappedBlockWithInternalPointers( } // state { - private BitmappedBlock!(theBlockSize, theAlignment, NullAllocator) _heap; + private BitmappedBlock!(theBlockSize, theAlignment, ParentAllocator) _heap; private BitVector _allocStart; // } @@ -2143,13 +2185,21 @@ struct BitmappedBlockWithInternalPointers( Constructors accepting desired capacity or a preallocated buffer, similar in semantics to those of `BitmappedBlock`. */ + static if (!stateSize!ParentAllocator) this(ubyte[] data) { _heap = BitmappedBlock!(theBlockSize, theAlignment, ParentAllocator)(data); } + static if (stateSize!ParentAllocator) + this(ParentAllocator parent, ubyte[] data) + { + _heap = BitmappedBlock!(theBlockSize, theAlignment, ParentAllocator)(data); + _heap.parent = parent; + } + /// Ditto - static if (!is(ParentAllocator == NullAllocator)) + static if (!is(ParentAllocator == NullAllocator) && !stateSize!ParentAllocator) this(size_t capacity) { // Add room for the _allocStart vector @@ -2157,6 +2207,15 @@ struct BitmappedBlockWithInternalPointers( (capacity + capacity.divideRoundUp(64)); } + /// Ditto + static if (!is(ParentAllocator == NullAllocator) && stateSize!ParentAllocator) + this(ParentAllocator parent, size_t capacity) + { + // Add room for the _allocStart vector + _heap = BitmappedBlock!(theBlockSize, theAlignment, ParentAllocator) + (parent, capacity + capacity.divideRoundUp(64)); + } + // Makes sure there's enough room for _allocStart @safe private bool ensureRoomForAllocStart(size_t len) @@ -2375,6 +2434,15 @@ struct BitmappedBlockWithInternalPointers( assert((() pure nothrow @safe @nogc => h.goodAllocSize(1))() == 4096); } +// Test instantiation with stateful allocators +@system unittest +{ + import std.experimental.allocator.mallocator : Mallocator; + import std.experimental.allocator.building_blocks.region : Region; + auto r = Region!Mallocator(1024 * 1024); + auto h = BitmappedBlockWithInternalPointers!(4096, 8, Region!Mallocator*)(&r, 4096 * 1024); +} + /** Returns the number of most significant ones before a zero can be found in `x`. If `x` contains no zeros (i.e. is equal to `ulong.max`), returns 64. diff --git a/std/experimental/allocator/building_blocks/kernighan_ritchie.d b/std/experimental/allocator/building_blocks/kernighan_ritchie.d index 190cd7274e4..0888a35e8c5 100644 --- a/std/experimental/allocator/building_blocks/kernighan_ritchie.d +++ b/std/experimental/allocator/building_blocks/kernighan_ritchie.d @@ -338,13 +338,22 @@ struct KRRegion(ParentAllocator = NullAllocator) } /// Ditto - static if (!is(ParentAllocator == NullAllocator)) + static if (!is(ParentAllocator == NullAllocator) && !stateSize!ParentAllocator) this(size_t n) { assert(n > Node.sizeof); this(cast(ubyte[])(parent.allocate(n))); } + /// Ditto + static if (!is(ParentAllocator == NullAllocator) && stateSize!ParentAllocator) + this(ParentAllocator parent, size_t n) + { + assert(n > Node.sizeof); + this.parent = parent; + this(cast(ubyte[])(parent.allocate(n))); + } + /// Ditto static if (!is(ParentAllocator == NullAllocator) && hasMember!(ParentAllocator, "deallocate")) diff --git a/std/experimental/allocator/building_blocks/region.d b/std/experimental/allocator/building_blocks/region.d index b13991d1c4d..cb7377156a8 100644 --- a/std/experimental/allocator/building_blocks/region.d +++ b/std/experimental/allocator/building_blocks/region.d @@ -90,12 +90,20 @@ struct Region(ParentAllocator = NullAllocator, } /// Ditto - static if (!is(ParentAllocator == NullAllocator)) + static if (!is(ParentAllocator == NullAllocator) && !stateSize!ParentAllocator) this(size_t n) { this(cast(ubyte[]) (parent.allocate(n.roundUpToAlignment(alignment)))); } + /// Ditto + static if (!is(ParentAllocator == NullAllocator) && stateSize!ParentAllocator) + this(ParentAllocator parent, size_t n) + { + this.parent = parent; + this(cast(ubyte[]) (parent.allocate(n.roundUpToAlignment(alignment)))); + } + /* TODO: The postblit of `BasicRegion` should be disabled because such objects should not be copied around naively.