Clang incorrectly rejects default construction of union with nontrivial member, part 2

[StephanTLavavej]

Accepted by VS 2022 17.10 Preview 1, rejected by Clang 17.0.3.

On Compiler Explorer, accepted by GCC 13.2, rejected by Clang 17.0.1 and trunk: https://godbolt.org/z/8fGhWa1ss

Presumably related to #48416 which was previously reported and fixed, but this larger repro is still failing.

C:\Temp>type meow.cpp

enum class BasicFormatArgType { NoType, CustomType };

struct Monostate {};

struct Handle {
    Handle(int, int) {}
};

template <typename Context>
struct BasicFormatArg {
    BasicFormatArg() = default;

    BasicFormatArg(int x, int y) : ActiveState(BasicFormatArgType::CustomType), CustomState(x, y) {}

    BasicFormatArgType ActiveState = BasicFormatArgType::NoType;

    union {
        Monostate NoState = Monostate{};
        Handle CustomState;
    };
};

int main() {
    BasicFormatArg<double> arg{};
    (void) arg;
}

C:\Temp>cl /EHsc /nologo /W4 /std:c++latest /c meow.cpp
meow.cpp

C:\Temp>clang-cl -v
clang version 17.0.3
Target: x86_64-pc-windows-msvc
Thread model: posix
InstalledDir: C:\Program Files\Microsoft Visual Studio\2022\Preview\VC\Tools\Llvm\x64\bin

C:\Temp>clang-cl /EHsc /nologo /W4 /std:c++latest /c meow.cpp
meow.cpp(24,28): error: call to implicitly-deleted default constructor of 'BasicFormatArg<double>'
   24 |     BasicFormatArg<double> arg{};
      |                            ^  ~~
meow.cpp(11,5): note: explicitly defaulted function was implicitly deleted here
   11 |     BasicFormatArg() = default;
      |     ^
meow.cpp(19,16): note: default constructor of 'BasicFormatArg<double>' is implicitly deleted because field 'CustomState'
      has no default constructor
   19 |         Handle CustomState;
      |                ^
1 error generated.

[llvmbot]

@llvm/issue-subscribers-clang-frontend

Author: Stephan T. Lavavej (StephanTLavavej)

Accepted by VS 2022 17.10 Preview 1, rejected by Clang 17.0.3.

On Compiler Explorer, accepted by GCC 13.2, rejected by Clang 17.0.1 and trunk: https://godbolt.org/z/8fGhWa1ss

Presumably related to #48416 which was previously reported and fixed, but this larger repro is still failing.

C:\Temp>type meow.cpp

enum class BasicFormatArgType { NoType, CustomType };

struct Monostate {};

struct Handle {
    Handle(int, int) {}
};

template <typename Context>
struct BasicFormatArg {
    BasicFormatArg() = default;

    BasicFormatArg(int x, int y) : ActiveState(BasicFormatArgType::CustomType), CustomState(x, y) {}

    BasicFormatArgType ActiveState = BasicFormatArgType::NoType;

    union {
        Monostate NoState = Monostate{};
        Handle CustomState;
    };
};

int main() {
    BasicFormatArg<double> arg{};
    (void) arg;
}

C:\Temp>cl /EHsc /nologo /W4 /std:c++latest /c meow.cpp
meow.cpp

C:\Temp>clang-cl -v
clang version 17.0.3
Target: x86_64-pc-windows-msvc
Thread model: posix
InstalledDir: C:\Program Files\Microsoft Visual Studio\2022\Preview\VC\Tools\Llvm\x64\bin

C:\Temp>clang-cl /EHsc /nologo /W4 /std:c++latest /c meow.cpp
meow.cpp(24,28): error: call to implicitly-deleted default constructor of 'BasicFormatArg<double>'
   24 |     BasicFormatArg<double> arg{};
      |                            ^  ~~
meow.cpp(11,5): note: explicitly defaulted function was implicitly deleted here
   11 |     BasicFormatArg() = default;
      |     ^
meow.cpp(19,16): note: default constructor of 'BasicFormatArg<double>' is implicitly deleted because field 'CustomState'
      has no default constructor
   19 |         Handle CustomState;
      |                ^
1 error generated.

[pinskia]

Simplified slightly:

struct Handle {
    Handle(int) {}
};
union a {
        int NoState = 0;
        Handle CustomState;
} b;

This is rejected with the following error message:

<source>:7:3: error: call to implicitly-deleted default constructor of 'union a'
    7 | } b;
      |   ^
<source>:6:16: note: default constructor of 'a' is implicitly deleted because field 'CustomState' has no default constructor
    6 |         Handle CustomState;
      |                ^

As far as I know an union can only have one active field at a time and since using NSDMI on the field should produce a valid default constructor.

[shafik]

We are hitting this case here:

llvm-project/clang/lib/Sema/SemaDeclCXX.cpp

Line 9445 in ff409d3

if (SMOR.getKind() == Sema::SpecialMemberOverloadResult::NoMemberOrDeleted)

I believe we need to make a similar change in this case along the lines of the fix in:

765d8a1

something like this:

 if (SMOR.getKind() == Sema::SpecialMemberOverloadResult::NoMemberOrDeleted) {
    if (CSM == Sema::CXXDefaultConstructor && Field && Field->getParent()->isUnion()) {
      // [class.default.ctor]p2:
      //   A defaulted default constructor for class X is defined as deleted if
      //   - X is a union that has a variant member with a non-trivial default
      //     constructor and no variant member of X has a default member
      //     initializer
      const auto *RD = cast<CXXRecordDecl>(Field->getParent());
      if (!RD->hasInClassInitializer())
        DiagKind = !Decl ? 0 : 1;
    }else {
      DiagKind = !Decl ? 0 : 1;
    }
  }

This passes check-clang with a minor diagnostic fix in a test.

CC @royjacobson @AaronBallman @cor3ntin

[AaronBallman]

That seems roughly correct to me, good catch @shafik!

[evaporei]

Any updates on this?

[llvmbot]

Hi!

This issue may be a good introductory issue for people new to working on LLVM. If you would like to work on this issue, your first steps are:

1. Check that no other contributor has already been assigned to this issue. If you believe that no one is actually working on it despite an assignment, ping the person. After one week without a response, the assignee may be changed.
2. In the comments of this issue, request for it to be assigned to you, or just create a pull request after following the steps below. Mention this issue in the description of the pull request.
3. Fix the issue locally.
4. Run the test suite locally. Remember that the subdirectories under test/ create fine-grained testing targets, so you can e.g. use make check-clang-ast to only run Clang's AST tests.
5. Create a Git commit.
6. Run git clang-format HEAD~1 to format your changes.
7. Open a pull request to the upstream repository on GitHub. Detailed instructions can be found in GitHub's documentation. Mention this issue in the description of the pull request.

If you have any further questions about this issue, don't hesitate to ask via a comment in the thread below.

[llvmbot]

@llvm/issue-subscribers-good-first-issue

Author: Stephan T. Lavavej (StephanTLavavej)

Accepted by VS 2022 17.10 Preview 1, rejected by Clang 17.0.3.

On Compiler Explorer, accepted by GCC 13.2, rejected by Clang 17.0.1 and trunk: https://godbolt.org/z/8fGhWa1ss

Presumably related to #48416 which was previously reported and fixed, but this larger repro is still failing.

C:\Temp>type meow.cpp

enum class BasicFormatArgType { NoType, CustomType };

struct Monostate {};

struct Handle {
    Handle(int, int) {}
};

template <typename Context>
struct BasicFormatArg {
    BasicFormatArg() = default;

    BasicFormatArg(int x, int y) : ActiveState(BasicFormatArgType::CustomType), CustomState(x, y) {}

    BasicFormatArgType ActiveState = BasicFormatArgType::NoType;

    union {
        Monostate NoState = Monostate{};
        Handle CustomState;
    };
};

int main() {
    BasicFormatArg<double> arg{};
    (void) arg;
}

C:\Temp>cl /EHsc /nologo /W4 /std:c++latest /c meow.cpp
meow.cpp

C:\Temp>clang-cl -v
clang version 17.0.3
Target: x86_64-pc-windows-msvc
Thread model: posix
InstalledDir: C:\Program Files\Microsoft Visual Studio\2022\Preview\VC\Tools\Llvm\x64\bin

C:\Temp>clang-cl /EHsc /nologo /W4 /std:c++latest /c meow.cpp
meow.cpp(24,28): error: call to implicitly-deleted default constructor of 'BasicFormatArg<double>'
   24 |     BasicFormatArg<double> arg{};
      |                            ^  ~~
meow.cpp(11,5): note: explicitly defaulted function was implicitly deleted here
   11 |     BasicFormatArg() = default;
      |     ^
meow.cpp(19,16): note: default constructor of 'BasicFormatArg<double>' is implicitly deleted because field 'CustomState'
      has no default constructor
   19 |         Handle CustomState;
      |                ^
1 error generated.

[Fedr]

A simpler example:

struct A{};
struct B{ 
    B(int) {}
};

int main() {
    union U {
        A a{};
        B b;
    } u;
}

It is accepted by GCC and MSVC, but not Clang. Online demo: https://gcc.godbolt.org/z/n91W74d3r