Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add TYPE_SAFE_MSC_EMPTY_BASES to all strong_typedef operations #159

Merged
merged 1 commit into from
Jan 12, 2024

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
7 checks passed
@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