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