Skip to content

Commit

Permalink
inplace_function: Fix unit-test failures on libstdc++.
Browse files Browse the repository at this point in the history
The failures were my fault, in 0b924dc. It turns out that
libstdc++'s `std::aligned_storage_t` does not report the correct
alignment for capacities less than about 16 bytes, which means
that our struct layout was still broken on libstdc++... only now
there are unit-tests that fail when the layout is broken!

Fix this for now by reinventing the wheel. I am also going to ping
the libstdc++ bug, https://gcc.gnu.org/bugzilla/show_bug.cgi?id=61458
to see if we can get any movement there.

Guard the new code under `#if defined(__GLIBCXX__)` so that we
remember to rip it out if/when libstdc++ gets their act together.
  • Loading branch information
Quuxplusone committed Jan 19, 2018
1 parent 2777d50 commit 98baf1a
Show file tree
Hide file tree
Showing 2 changed files with 55 additions and 9 deletions.
33 changes: 31 additions & 2 deletions SG14/inplace_function.h
Expand Up @@ -36,6 +36,35 @@ namespace inplace_function_detail {

static constexpr size_t InplaceFunctionDefaultCapacity = 32;

#if defined(__GLIBCXX__) // https://gcc.gnu.org/bugzilla/show_bug.cgi?id=61458
template<size_t Cap>
union aligned_storage_helper {
struct double1 { double a; };
struct double4 { double a[4]; };
template<class T> using maybe = std::conditional_t<(Cap >= sizeof(T)), T, char>;
char real_data[Cap];
maybe<int> a;
maybe<long> b;
maybe<long long> c;
maybe<void*> d;
maybe<void(*)()> e;
maybe<double1> f;
maybe<double4> g;
maybe<long double> h;
};

template<size_t Cap, size_t Align = std::alignment_of<aligned_storage_helper<Cap>>::value>
struct aligned_storage {
using type = std::aligned_storage_t<Cap, Align>;
};

template<size_t Cap, size_t Align = std::alignment_of<aligned_storage_helper<Cap>>::value>
using aligned_storage_t = typename aligned_storage<Cap, Align>::type;
#else
using std::aligned_storage;
using std::aligned_storage_t;
#endif

template<typename T> struct wrapper
{
using type = T;
Expand Down Expand Up @@ -116,7 +145,7 @@ struct is_valid_inplace_dst : std::true_type
template<
typename Signature,
size_t Capacity = inplace_function_detail::InplaceFunctionDefaultCapacity,
size_t Alignment = std::alignment_of<std::aligned_storage_t<Capacity>>::value
size_t Alignment = std::alignment_of<inplace_function_detail::aligned_storage_t<Capacity>>::value
>
class inplace_function; // unspecified

Expand All @@ -132,7 +161,7 @@ class inplace_function<R(Args...), Capacity, Alignment>
using capacity = std::integral_constant<size_t, Capacity>;
using alignment = std::integral_constant<size_t, Alignment>;

using storage_t = std::aligned_storage_t<Capacity, Alignment>;
using storage_t = inplace_function_detail::aligned_storage_t<Capacity, Alignment>;
using vtable_t = inplace_function_detail::vtable<R, Args...>;
using vtable_ptr_t = const vtable_t*;

Expand Down
31 changes: 24 additions & 7 deletions SG14_test/inplace_function_test.cpp
@@ -1,5 +1,5 @@
#include "SG14_test.h"
#include "../SG14/inplace_function.h"
#include "inplace_function.h"
#include <cassert>
#include <memory>
#include <string>
Expand Down Expand Up @@ -260,6 +260,22 @@ void AssignmentDifferentFunctor()
EXPECT_EQ(4, calls);
}

template<size_t Cap>
constexpr size_t expected_alignment_for_capacity()
{
constexpr size_t alignof_ptr = std::alignment_of<void*>::value;
constexpr size_t alignof_cap = std::alignment_of<std::aligned_storage_t<Cap>>::value;
#define MIN(a,b) (a < b ? a : b)
#define MAX(a,b) (a > b ? a : b)
#if defined(__GLIBCXX__) // https://gcc.gnu.org/bugzilla/show_bug.cgi?id=61458
return MAX(MIN(Cap, alignof_cap), alignof_ptr);
#else // other STLs
return MAX(alignof_cap, alignof_ptr);
#endif
#undef MAX
#undef MIN
}

void sg14_test::inplace_function_test()
{
// first set of tests (from Optiver)
Expand Down Expand Up @@ -287,12 +303,13 @@ void sg14_test::inplace_function_test()
#endif
static_assert(std::is_nothrow_destructible<IPF>::value, "");

constexpr int alignof_ptr = std::alignment_of<void*>::value;
static_assert(std::alignment_of< stdext::inplace_function<void(int), 1> >::value == std::max(1, alignof_ptr), "");
static_assert(std::alignment_of< stdext::inplace_function<void(int), 2> >::value == std::max(2, alignof_ptr), "");
static_assert(std::alignment_of< stdext::inplace_function<void(int), 4> >::value == std::max(4, alignof_ptr), "");
static_assert(std::alignment_of< stdext::inplace_function<void(int), 8> >::value == std::max(8, alignof_ptr), "");
static_assert(std::alignment_of< stdext::inplace_function<void(int), 16> >::value == std::max(16, alignof_ptr), "");
static_assert(std::alignment_of< stdext::inplace_function<void(int), 1> >::value == expected_alignment_for_capacity<1>(), "");
static_assert(std::alignment_of< stdext::inplace_function<void(int), 2> >::value == expected_alignment_for_capacity<2>(), "");
static_assert(std::alignment_of< stdext::inplace_function<void(int), 4> >::value == expected_alignment_for_capacity<4>(), "");
static_assert(std::alignment_of< stdext::inplace_function<void(int), 8> >::value == expected_alignment_for_capacity<8>(), "");
static_assert(std::alignment_of< stdext::inplace_function<void(int), 16> >::value == expected_alignment_for_capacity<16>(), "");
static_assert(std::alignment_of< stdext::inplace_function<void(int), 32> >::value == expected_alignment_for_capacity<32>(), "");

static_assert(sizeof( stdext::inplace_function<void(int), sizeof(void*)> ) == 2 * sizeof(void*), "");

IPF func;
Expand Down

0 comments on commit 98baf1a

Please sign in to comment.