TempAlloc review starts now
dsimcha
dsimcha at yahoo.com
Mon Jun 6 12:10:49 PDT 2011
== Quote from Steven Schveighoffer (schveiguy at yahoo.com)'s article
> On Sun, 05 Jun 2011 18:25:07 -0400, Lars T. Kyllingstad
> <public at kyllingen.nospamnet> wrote:
> > All right, folks, it's time to get the review queue started again. First
> > up is David Simcha's TempAlloc, which, if accepted, is to be included in
> > the core.memory module in druntime.
> newVoid: I don't really like the name of this -- void is a type. It
> suggests you are allocating a new void array. I'm not sure we should
> adopt the hacky syntax that D uses to say "don't initialize" as a symbol
> name.
> Particularly, this looks odd:
> newVoid!double
> What the hell does that mean? :)
> I suggest a name like newUninit or newRaw or something that means more
> "uninitailized" than "no type".
Seems there's a strong consensus that the "newVoid" name sucks. I agree in
hindsight. Will change.
> alignedMalloc:
> I seriously question having such a feature. According to my C++ book
> malloc returns a pointer "suitably aligned for any type". According to
> Microsoft, malloc is 16-byte aligned (of course, D doesn't use microsoft's
> runtime, DMC doesn't seem to identify alignment in malloc docs). GNU
> appears to guarantee 8-byte alignment on 32-bit systems, 16 on 64-bit
> systems (making this function mostly useless on 64-bit dmd).
> There are also some posix functions that align a malloc to a requested
> size (see memalign).
I definitely need it in the implementation of TempAlloc, so it's gonna be there
but it can be made private if there's a consensus that it's not needed in the
public API.
> At the very least, the function should identify what the alignment is if
> you *don't* use it. I'd like to see a good use case for this feature in
> an example, otherwise, I think it should be killed.
The alignment if you don't use it depends on the C malloc function for your
platform. A use case might be if you need 16-byte aligned arrays to use SSE
instructions, but that's hard to demonstrate in a short example.
> That being said, wrapping malloc might have some nice other features too.
> I like the auto-adding of the range to the GC.
> tempdup:
> 1. If this uses ElemType!(R)[] as the return type, duping a char[] will
> give you a dchar[]. I don't think this is very desirable.
This seems right to me. tempdup() is supposed to be basically a TempAlloc version
of array(). IMHO it should **ALWAYS** return a random-access range. Returning a
narrow string creates an obscure special case where it doesn't.
> 2. What happens for something like immutable(uint *)[]? Does it become
> uint *[]? Because that would be bad...
No. It becomes an immutable(uint*)[]. ElementType!(immutable(uint*)[]) ==
immutable(uint*).
> TempAlloc.malloc:
> I don't like void * as a return value. Would it not be more appropriate
> to use at least void[]? I'd suggest actually for malloc to take a
> template parameter of the type to return, defaulting to void. i.e.:
> T[] malloc(T = void)(size_t size);
Here I'm going to have to strongly disagree. I want to keep TempAlloc.malloc
consistent with GC.malloc and core.stdc.stdlib.malloc.
Thanks for your review and the points you've raised.
More information about the Digitalmars-d
mailing list