std.experimental.logger: allow precise scanning of DATA/TLS segment

[rainers]

Buffers for preallocated class instances should be typed void[], not ubyte[] and must be properly aligned.

This blocks dlang/druntime#1798.

[burner]

Looks good

[PetarKirov]

@andralex druntime & phobos are full of such ad hoc implementations of the singleton pattern (the last being 5355) we should really try to get singleton!T merged and fix this once and for all. Do you want me to take over the singleton implementation from dlang/druntime#1710?

[andralex]

@ZombineDev that'd be great, thanks. There seem to be three kinds - thread-local, shared, and gshared.

[PetarKirov]

I imagine they would work like so:

// at module level
Singleton!T instance; // thread-local
shared Singleton!T shared_instance; // shared
__gshared Singleton!T gshared_instance; // __gshared

pragma (msg, typeof(instance.get()); // T
pragma (msg, typeof(shared_instance.get()); // shared(T)
pragma (msg, typeof(gshared_instance.get()); // T

// at function/aggregate scope
/* static is implicit in the implementation*/ Singleton!T instance; // thread-local
/* static is implicit in the implementation */ shared Singleton!T shared_instance; // shared
__gshared Singleton!T gshared_instance; // __gshared

pragma (msg, typeof(instance.get()); // T
pragma (msg, typeof(shared_instance.get()); // shared(T)
pragma (msg, typeof(gshared_instance.get()); // T

I.e. overload Singleton(T)'s methods on shared-ness.

[andralex]

@ZombineDev the singleton wouldn't appear at module level - that's a global. It needs to be a function returning a reference.

[PetarKirov]

If setting alignment explicitly doesn't work, maybe the following would:
size_t[1 + (__traits(classInstanceSize, StdForwardLogger) - 1) / size_t.sizeof] _buffer

[rainers]

Could work with size_t replaced by void*. It get's pretty verbose, though. It also doesn't support possible larger alignment requirements, but these should not be required here.

[andralex]

There's divideRoundUp in std.experimental.allocator, perhaps it should be moved up.

[PetarKirov]

Could work with size_t replaced by void*

At first I thought about proposing void* but then I chose size_t for simplicity. But shouldn't all integral types N have N.alignof == N.sizeof?

It get's pretty verbose, though.

Agreed, that's why I'm proposing to abstract those details in a singleton(T) template ;)

[rainers]

Amended a respective change.

[rainers]

At first I thought about proposing void* but then I chose size_t for simplicity. But shouldn't all integral types N have N.alignof == N.sizeof?

Yes, but using size_t would avoid scanning that area as ubyte[] does.

[PetarKirov]

Ah yes, I missed the obvious :D

[rainers]

Unfortunately, ld for x64 doesn't seem to like the alignment specification for TLS data. I have commented these.

[rainers]

druntime & phobos are full of such ad hoc implementations of the singleton behavior (the last being 5355) we should really try to get singleton!T merged and fix this once and for all.

I grepped for similar code and found a few, but AFAICT none worked on classes with pointers (the monitor pointer is manually managed, so doesn't need to be marked for precise scanning). It would be nice if a generic version could cover all instances.

[PetarKirov]

@rainers feel free to merge now that everything is green.

[rainers]

Thanks for fast review.