Trivially Default Constructible with requires + Structure Wrapping

[geometrian]

Consider the following semi-minimal example:

//Compile with -std=c++20

#include <type_traits>

template<int n>
struct Foo {
	constexpr Foo() = default;

	template<class... Ts>
	Foo(Ts... vals) requires(sizeof...(Ts)==n) {}
};

struct Bar {
	Foo<4> foo;
};

static_assert(std::is_trivially_default_constructible_v< Foo<4> >); //Fine
static_assert(std::is_trivially_default_constructible_v< Bar    >); //Fails???

The first static_assert passes, indicating that Foo<4> is trivially default constructible. However, we now wrap Foo<4> in a class Bar which does nothing, and suddenly the type is no longer trivially default constructible and the second static_assert fails:

<source>:18:1: error: static assertion failed due to requirement 'std::is_trivially_default_constructible_v<Bar>'
static_assert(std::is_trivially_default_constructible_v< Bar    >);
^             ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
1 error generated.
Compiler returned: 1

I believe that this is incorrect behavior. We can verify that Foo<4> is indeed trivially default-constructible per the definition, and of course by the static_assert passing. Even if Foo<4> didn't fully constrain the constructors, the non-templated defaulted constructor should be selected preferentially. Next, wrapping Foo<4> in Bar should do nothing; if anything it should ensure that only the default constructor is accessible. And finally, GCC, MSVC, and even Intel Compiler (now based on LLVM) accept this code without complaint.

The tested version:

clang version 16.0.0 (https://github.com/llvm/llvm-project.git 437ccf5af9c2aec915a68a164a95d506fbac2324)

Note that this is a regression. E.g. Clang 15 accepts the code.

[llvmbot]

@llvm/issue-subscribers-c-20

[llvmbot]

@llvm/issue-subscribers-clang-frontend

[royjacobson]

Thanks for reporting, this sounds like the fault of my P0848 patch :)
I'll take a look sometime next week.

[royjacobson]

So, the problem here is that we mark the trivial constructor as ineligible, because it is less constrained than the template constructor. This is morally wrong because the template constructor wouldn't satisfy the constraints if instantiated. But we can't them because we don't (and shouldn't) instantiate the constructor.

Apparently GCC decided to just not compare template to non-template constructors: https://gcc.gnu.org/git/?p=gcc.git;a=commit;h=c75ebe76ae12ac4020f20a24f34606a594a40d15.

I'd like to be conformant, so this needs a CWG issue (not the minor one linked above). I'll try to write a wording suggestion and then we can proceed with this.

[royjacobson]

Funny enough, it appears Clang's the only compiler that got DR1363 'correctly'. We got a bug report about DR1363 a few weeks ago: #58578 but I didn't realize at the time that our behavior deviates from GCC/MSVC.

So we also had a divergence for __is_trivial, well before Clang 16:

struct A {
  A() = default;
  template<class... Args>
  A(Args&&... args) requires (sizeof...(Args) > 0) {}
};

struct B {
  B() = default;
  template<class... Args>
  B(Args&&... args) requires (sizeof...(Args) == 0) {}
};

struct C {
  C() = default;
  template<class... Args>
  C(Args&&... args) {}
};

[royjacobson]

draft: https://reviews.llvm.org/D139038

[royjacobson]

@erichkeane @zygoloid @hubert-reinterpretcast

If you can spare some time, I would appreciate if you have any guidance on how to proceed here. Specifically -

1. Do we want to wait for CWG discussion?
2. Is the ABI break from previous Clang versions reasonable or do we need to be extra careful about it?

I think my current preference is to align with GCC on this.

[erichkeane]

So to answer your second question first:

I think that just updating the ClangABI flag to support both versions of the triviality is sufficient here, that is what Richard told me initially when I looked into the triviality. HOWEVER, this is something we should do ONCE. And we should fix ALL the triviality all at once, not just piecemeal.

I'd encourage us to wait until after the February C++ standards meeting (where C++23 will be 'finalized') and just implement whatever is there at that point. If CWG fixes things after the fact, than so be it.

HOWEVER, all of that is after the Clang16 branch, so the question comes in with what we want to do for Clang 16, particularly with this "Regression". I don't have much of a feeling as to what to do (as I don't completely understand the implications of what I'm saying), but I suspect I'd like us to get back to our previous behavior best-we-can to avoid the extra break.

WDYT?

[royjacobson]

Sounds reasonable to me! I updated the patch to only fix the eligibility regression - and let's wait with the larger breakage until CWG can spare some time to look at this.

[erichkeane]

LGTM! Thanks!

[royjacobson]

@erichkeane any idea if this was discussed on the reflector or something? Didn't get a response on GitHub, unfortunately.

[shafik]

@royjacobson I usually sit in core so if you can draft an explanation of the issue and a proposed resolution I can see if I can get it reviewed in core.