Bug 16269 – add `aa.ensureAllocated` method to associative array to clear and initialize it

q&d patch: diff --git a/src/object.d b/src/object.d index 40e2391..f2f29de 100644 --- a/src/object.d +++ b/src/object.d @@ -1876,6 +1876,7 @@ extern (C) inout(void)[] _aaKeys(inout void* p, in size_t keysize, const TypeInfo tiKeyArray) pure nothrow; void* _aaRehash(void** pp, in TypeInfo keyti) pure nothrow; void _aaClear(void* p) pure nothrow; + void _aaClearInit(void* p, const TypeInfo_AssociativeArray ti) pure nothrow; // alias _dg_t = extern(D) int delegate(void*); // int _aaApply(void* aa, size_t keysize, _dg_t dg); @@ -1914,9 +1915,20 @@ void clear(T : Value[Key], Value, Key)(T aa) _aaClear(*cast(void **) &aa); } -void clear(T : Value[Key], Value, Key)(T* aa) +void clear(bool doalloc:true, T : Value[Key], Value, Key)(ref T aa) { - _aaClear(*cast(void **) aa); + static if (!doalloc) + _aaClear(*cast(void **) &aa); + else + _aaClearInit(cast(void **) &aa, typeid(T)); +} + +void clear(bool doalloc=false, T : Value[Key], Value, Key)(T* aa) +{ + static if (!doalloc) + _aaClear(*cast(void **) aa); + else + _aaClearInit(*cast(void **) &aa, typeid(T)); } T rehash(T : Value[Key], Value, Key)(T aa) diff --git a/src/rt/aaA.d b/src/rt/aaA.d index cf8943e..2e5a3a1 100644 --- a/src/rt/aaA.d +++ b/src/rt/aaA.d @@ -443,6 +443,20 @@ extern (C) void _aaClear(AA aa) pure nothrow } } +/// Remove all elements from AA, allocate new AA if it isn't allocated yet. +extern (C) void _aaClearInit(AA* aa, const TypeInfo_AssociativeArray ti) +{ + if (!aa.empty) + { + aa.impl.clear(); + } + else if (aa.impl is null) + { + // alloc implementation + aa.impl = new Impl(ti); + } +} + /// Rehash AA extern (C) void* _aaRehash(AA* paa, in TypeInfo keyti) pure nothrow { -- 2.9.0

Thanks. I don't love the API. aa.clear!true is not "clear" (pun not intended) on what it is doing as opposed to aa.clear. I like the idea though -- have a function that clears, and if it's not allocated, do that too. Why not use your internal name: clearInit?

i just thought that `.clear!true` is better. i don't know why: probably 'cause i just love D templates and want to use 'em everywhere. ;-) still, it's a matter of simple renaming, i don't have real API preferences here. something like `.clear!(init:true)` may be better, but D doesn't support that. T_T

p.s. i created this ER in the hope that people will talk about API too: i'm usually sux at inventig APIs.

another try. this time it adds `ensureAllocated()` method, which can be used like this: ====================================== void test (string[int] aa) { aa[42] = "42"; } void main () { // inline { string[int] aa; test(aa.ensureAllocated); assert(aa[42] == "42"); // check that AA is not cleared aa.ensureAllocated(); assert(aa[42] == "42"); } // function { string[int] bb; bb.ensureAllocated; test(bb); assert(bb[42] == "42"); // check that AA is not cleared bb.ensureAllocated(); assert(bb[42] == "42"); } } ====================================== diff --git a/src/object.d b/src/object.d index 40e2391..0d82f07 100644 --- a/src/object.d +++ b/src/object.d @@ -1876,6 +1876,7 @@ extern (C) inout(void)[] _aaKeys(inout void* p, in size_t keysize, const TypeInfo tiKeyArray) pure nothrow; void* _aaRehash(void** pp, in TypeInfo keyti) pure nothrow; void _aaClear(void* p) pure nothrow; + void _aaEnsureAllocated(void* p, const TypeInfo_AssociativeArray ti); // alias _dg_t = extern(D) int delegate(void*); // int _aaApply(void* aa, size_t keysize, _dg_t dg); @@ -1919,6 +1920,18 @@ void clear(T : Value[Key], Value, Key)(T* aa) _aaClear(*cast(void **) aa); } +T ensureAllocated(T : Value[Key], Value, Key)(ref T aa) +{ + _aaEnsureAllocated(cast(void*)&aa, typeid(T)); + return aa; +} + +T ensureAllocated(T : Value[Key], Value, Key)(T* aa) +{ + _aaEnsureAllocated(cast(void*)aa, typeid(T)); + return *aa; +} + T rehash(T : Value[Key], Value, Key)(T aa) { _aaRehash(cast(void**)&aa, typeid(Value[Key])); diff --git a/src/rt/aaA.d b/src/rt/aaA.d index cf8943e..b16b6d8 100644 --- a/src/rt/aaA.d +++ b/src/rt/aaA.d @@ -443,6 +443,16 @@ extern (C) void _aaClear(AA aa) pure nothrow } } +/// Remove all elements from AA, allocate new AA if it isn't allocated yet. +extern (C) void _aaEnsureAllocated(AA* paa, const TypeInfo_AssociativeArray ti) +{ + if (paa !is null && paa.impl is null) + { + // alloc implementation + paa.impl = new Impl(ti); + } +} + /// Rehash AA extern (C) void* _aaRehash(AA* paa, in TypeInfo keyti) pure nothrow { -- 2.9.0

oops. forgot to fix the comment. sorry. ;-)

Thanks, this looks really good. I like the returning of itself, so you just put ensureAllocated on the call and it just works in all cases where you normally have an AA. When I get a chance, I'll make a PR. Thanks for the tests too!

Just adding a note for myself when I make the PR, the AA is and always will be an implementation pointer as the only member. I will check for null pointer in wrapper rather than the runtime code to avoid extra opaque calls.

>I get a chance, I'll make a PR thank you. >I will check for null pointer in wrapper rather than the runtime code to avoid extra >opaque calls. yes, this is way better, tnx again. i planned to do exactly that, but somehow put it in the wrong place. ;-)

so here is the fixed patch, and new tests: ======================================= diff --git a/src/object.d b/src/object.d index 40e2391..5ac132c 100644 --- a/src/object.d +++ b/src/object.d @@ -1876,6 +1876,7 @@ extern (C) inout(void)[] _aaKeys(inout void* p, in size_t keysize, const TypeInfo tiKeyArray) pure nothrow; void* _aaRehash(void** pp, in TypeInfo keyti) pure nothrow; void _aaClear(void* p) pure nothrow; + void _aaEnsureAllocated(void* p, const TypeInfo_AssociativeArray ti); // alias _dg_t = extern(D) int delegate(void*); // int _aaApply(void* aa, size_t keysize, _dg_t dg); @@ -1919,6 +1920,25 @@ void clear(T : Value[Key], Value, Key)(T* aa) _aaClear(*cast(void **) aa); } +T ensureAllocated(T : Value[Key], Value, Key)(ref T aa) +{ + _aaEnsureAllocated(cast(void*)&aa, typeid(T)); + return aa; +} + +T ensureAllocated(T : Value[Key], Value, Key)(T* aa) +{ + if (aa !is null) + { + _aaEnsureAllocated(cast(void*)aa, typeid(T)); + return *aa; + } + else + { + return T.init; // we have to return something here + } +} + T rehash(T : Value[Key], Value, Key)(T aa) { _aaRehash(cast(void**)&aa, typeid(Value[Key])); diff --git a/src/rt/aaA.d b/src/rt/aaA.d index cf8943e..48194b0 100644 --- a/src/rt/aaA.d +++ b/src/rt/aaA.d @@ -443,6 +443,15 @@ extern (C) void _aaClear(AA aa) pure nothrow } } +/// Allocate new AA implementation if it isn't allocated yet. +extern (C) void _aaEnsureAllocated(AA* paa, const TypeInfo_AssociativeArray ti) +{ + if (paa.impl is null) + { + paa.impl = new Impl(ti); + } +} + /// Rehash AA extern (C) void* _aaRehash(AA* paa, in TypeInfo keyti) pure nothrow { -- 2.9.0 ======================================= unittest { void test (string[int] aa) { aa[42] = "42"; } // inline { string[int] aa; test(aa.ensureAllocated); assert(aa[42] == "42"); // check that AA is not cleared aa.ensureAllocated(); assert(aa[42] == "42"); } // function { string[int] bb; bb.ensureAllocated; test(bb); assert(bb[42] == "42"); // check that AA is not cleared bb.ensureAllocated(); assert(bb[42] == "42"); } } // test pointer version unittest { void test (string[int] aa) { aa[42] = "42"; } // inline { string[int] aa; string[int]* ptr = &aa; test(ptr.ensureAllocated); assert(aa[42] == "42"); // check that AA is not cleared ptr.ensureAllocated(); assert(aa[42] == "42"); } // case of "null pointer" { string[int]* bb; test(bb.ensureAllocated); // this should not fail } }

(Apologies for such a late comment, hope it's not disruptive.) Would it make sense to consider including a 'reserve' capability with this change? See https://issues.dlang.org/show_bug.cgi?id=2504. Ensuring allocation could be seen as having a relationship with reserving an amount of space. At the API level, it could possibly be done by adding an optional argument to ensureAllocated that would reserve space. The con to this approach might simply be that 'reserve' or setting the 'length' property would be more consistent with other APIs already being used in D.

*** This issue has been marked as a duplicate of issue 10535 ***