Skip to content

Fix Variant #768

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

Draft
wants to merge 28 commits into
base: main
Choose a base branch
from
Draft

Fix Variant #768

wants to merge 28 commits into from

Conversation

sfod
Copy link
Contributor

@sfod sfod commented Aug 11, 2025

Issue #, if available:

This PR resolves two issues with Crt::Variant.

1. Compilation error with move-only underlying types on Windows platform

When building shared libraries on Windows platform, we use __declspec(dllexport) declaration for classes. According to MS docs, "class templates are explicitly instantiated and the class's members must be defined".

So, for the marked classes compiler will try to generate all special members (copy and move constructors and assignment operators; and default constructor if needed).

For example, take the following class:

struct __declspec(dllexport) MyTest {
  Crt::Variant<Foo> m_variant;
};

The problem is that Crt::Variant has a user-defined special member functions. So, compiler sees that the Crt::Variant<Foo> class has a certain special member defined, so compiler tries to instantiate (or generate) this special member.

The issue appears when the Crt::Variant underlying class (Foo in the example above) is move-only, or copy-only, or non-default-constructible. For example, if Foo is move-only, compiler still sees a user-defined move constructor and move assignment operator in Crt::Variant, decides that they can be used, and tries to instantiate them. And we see something like this:

77>C:\a\aws-crt-cpp\aws-crt-cpp\aws-crt-cpp\include\aws\crt\Variant.h(484,41): error C2280:
's_VariantBasicOperandsCompile::MoveOnlyTest::MoveOnlyTest(const s_VariantBasicOperandsCompile::MoveOnlyTest &)':
attempting to reference a deleted function
[C:\a\aws-crt-cpp\aws-crt-cpp\aws-crt-cpp\build\aws-crt-cpp\tests\aws-crt-cpp-tests.vcxproj]

This issue could be resolved by explicitly deleting special members in user classes (MyTest in the example). However, this approach is not very convenient for Crt::Variant users.

2. Compilation error on old gcc compilers when class's constructor has noexcept specifier

Constructors in Crt::Variant doesn't have noexcept specifier. When user class tries to use noexcept it causes compilation error on older gcc compilers:

<source>:11:5: note: 'Bar::Bar() noexcept' is implicitly deleted because its exception-specification
does not match the implicit exception-specification ''

This stopped to be an error in gcc 10.

Description of changes:

  1. The first problem can be resolved by removing user-defined special members from Crt::Variant. It can be achieved by moving all implementation details into VariantImpl, and Variant members will just delegate all work to VariantImpl.
    The special members in Variant will be either defaulted or deleted, depending on the underlying types traits. This can be achieved by inheriting from special auxiliary classes with defaulted/deleted special members.

    The simplified example can be found here: https://godbolt.org/z/Yz791jKoM

  2. Add noexcept specifier to the Crt::Variant default constructor if the first underlying type is nothrow-constructible.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

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.

2 participants