Skip to content

Conversation

@ZeroMemes
Copy link
Contributor

@ZeroMemes ZeroMemes commented Jan 12, 2024

Expands upon the PR (#128) by adding the TYPE_SAFE_MSC_EMPTY_BASES macro to all strong_typedef operations. This Fixes an issue where the order of inheritance would affect the size of the derived strong_typedef, for example:

using namespace type_safe;

struct A :
    strong_typedef<A, uint16_t>,
    strong_typedef_op::relational_comparison<A>,
    strong_typedef_op::equality_comparison<A>,
    strong_typedef_op::integer_arithmetic<A>
{
    using strong_typedef::strong_typedef;
};
static_assert(sizeof(A) == 2);

In this specific configuration, the code fails to compile due to sizeof(A) actually evaluating to 4. However, transposing the order of equality_comparison and integer_arithmetic has the surprising result of the static_assert succeeding with a class size equal to 2! I believe this rule comes down to the following statement from the Microsoft documentation

However, the default class layout in Visual Studio doesn't take advantage of EBCO in multiple inheritance scenarios [...] The class layout algorithm is adding 1 byte of padding between any two consecutive empty base classes [...]

The following snippet is a more generic case which shows that my proposed patch works, as it only compiles when __declspec(empty_bases) is applied such that no two consecutive base classes are without the modifier.

struct C {
    uint16_t val;
};
// __declspec(empty_bases) must be applied to D and F *or* E
// in order for the static_assert to pass
struct D {};
struct E {};
struct F {};
struct G : C, D, E, F {};

static_assert(sizeof(G) == 2);

@foonathan foonathan merged commit ae6d075 into foonathan:main Jan 12, 2024
@foonathan
Copy link
Owner

Good idea, thanks.

@BrukerJWD
Copy link
Contributor

@foonathan, are you going to release a new version?

@ZeroMemes ZeroMemes deleted the pr/empty_bases branch January 12, 2024 18:10
@foonathan
Copy link
Owner

I've tagged a new release.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants