From fd856035768fc54690615afe8de1d3db2f84e6fa Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?S=C3=B6nke=20Ludwig?= <sludwig@rejectedsoftware.com>
Date: Tue, 16 Apr 2019 09:41:16 +0200
Subject: [PATCH 1/2] Compute the proper struct alignment for TaggedUnion.

Before this change, the alignment of TaggedAlgebraic was always 4, which in turn had a 50% chance of aligning 8-byte pointers on a non-8-byte boundary, causing the pointer to become invisible to the GC (and less bad, causing performance degradation when dereferencing the pointer). This would happen in particular when the total size of the TaggedUnion/TaggedAlgebraic was not divisible by 8 and an array of values was built.
---
 source/taggedalgebraic/taggedunion.d | 41 +++++++++++++++++++++++++++-
 1 file changed, 40 insertions(+), 1 deletion(-)

diff --git a/source/taggedalgebraic/taggedunion.d b/source/taggedalgebraic/taggedunion.d
index 9c1afb9..22b8059 100644
--- a/source/taggedalgebraic/taggedunion.d
+++ b/source/taggedalgebraic/taggedunion.d
@@ -29,7 +29,8 @@ import std.traits : EnumMembers, FieldNameTuple, Unqual, isInstanceOf;
 		$(LI `getFoo` - equivalent to `get!(Kind.foo)`)
 	)
 */
-struct TaggedUnion(U) if (is(U == union) || is(U == struct) || is(U == enum))
+template TaggedUnion(U) if (is(U == union) || is(U == struct) || is(U == enum)) {
+align(commonAlignment!(UnionKindTypes!(UnionFieldEnum!U))) struct TaggedUnion
 {
 	import std.traits : FieldTypeTuple, FieldNameTuple, Largest,
 		hasElaborateCopyConstructor, hasElaborateDestructor, isCopyable;
@@ -294,6 +295,7 @@ struct TaggedUnion(U) if (is(U == union) || is(U == struct) || is(U == enum))
 
 	package @trusted @property ref inout(T) trustedGet(T)() inout { return *cast(inout(T)*)m_data.ptr; }
 }
+}
 
 ///
 @safe nothrow unittest {
@@ -409,6 +411,33 @@ unittest { // non-copyable types
 	tu.setS(S.init);
 }
 
+unittest { // alignment
+	union S1 { int v; }
+	union S2 { ulong v; }
+	union S3 { void* v; }
+
+	// sanity check for the actual checks - this may differ on non-x86 architectures
+	static assert(S1.alignof == 4);
+	static assert(S2.alignof == 8);
+	version (D_LP64) static assert(S3.alignof == 8);
+	else static assert(S3.alignof == 4);
+
+	// test external struct alignment
+	static assert(TaggedUnion!S1.alignof == 4);
+	static assert(TaggedUnion!S2.alignof == 8);
+	version (D_LP64) static assert(TaggedUnion!S3.alignof == 8);
+	else static assert(TaggedUnion!S3.alignof == 4);
+
+	// test internal struct alignment
+	TaggedUnion!S1 s1;
+	assert((cast(ubyte*)&s1.vValue() - cast(ubyte*)&s1) % 4 == 0);
+	TaggedUnion!S1 s2;
+	assert((cast(ubyte*)&s2.vValue() - cast(ubyte*)&s2) % 8 == 0);
+	TaggedUnion!S1 s3;
+	version (D_LP64) assert((cast(ubyte*)&s3.vValue() - cast(ubyte*)&s3) % 8 == 0);
+	else assert((cast(ubyte*)&s3.vValue() - cast(ubyte*)&s3) % 4 == 0);
+}
+
 
 /** Dispatches the value contained on a `TaggedUnion` to a set of visitors.
 
@@ -751,6 +780,16 @@ package template AmbiguousTypes(Types...) {
 	alias AmbiguousTypes = impl!0;
 }
 
+/// Computes the minimum alignment necessary to align all types correctly
+private size_t commonAlignment(TYPES...)()
+{
+	import std.numeric : gcd;
+
+	size_t ret = 1;
+	foreach (T; TYPES)
+		ret = (T.alignof * ret) / gcd(T.alignof, ret);
+	return ret;
+}
 
 package void rawEmplace(T)(void[] dst, ref T src)
 {

From ba79a252983fd8d0f231483766200da8ae987c70 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?S=C3=B6nke=20Ludwig?= <sludwig@rejectedsoftware.com>
Date: Tue, 16 Apr 2019 10:09:27 +0200
Subject: [PATCH 2/2] Add explicit unit test for commonAlignment.

---
 source/taggedalgebraic/taggedunion.d | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/source/taggedalgebraic/taggedunion.d b/source/taggedalgebraic/taggedunion.d
index 22b8059..49803bc 100644
--- a/source/taggedalgebraic/taggedunion.d
+++ b/source/taggedalgebraic/taggedunion.d
@@ -791,6 +791,21 @@ private size_t commonAlignment(TYPES...)()
 	return ret;
 }
 
+unittest {
+	align(2) struct S1 { ubyte x; }
+	align(4) struct S2 { ubyte x; }
+	align(8) struct S3 { ubyte x; }
+
+	static if (__VERSION__ > 2076) { // DMD 2.076 ignores the alignment
+		assert(commonAlignment!S1 == 2);
+		assert(commonAlignment!S2 == 4);
+		assert(commonAlignment!S3 == 8);
+		assert(commonAlignment!(S1, S3) == 8);
+		assert(commonAlignment!(S1, S2, S3) == 8);
+		assert(commonAlignment!(S2, S2, S1) == 4);
+	}
+}
+
 package void rawEmplace(T)(void[] dst, ref T src)
 {
 	T[] tdst = () @trusted { return cast(T[])dst[0 .. T.sizeof]; } ();