llvm / llvm-project

The LLVM Project is a collection of modular and reusable compiler and toolchain technologies.
http://llvm.org
Other
28.9k stars 11.94k forks source link

[Clang] [C++20] Trivial copy of variant member array where some subobjects have been destroyed is rejected in a constant expression #105932

Open MitalAshok opened 2 months ago

MitalAshok commented 2 months ago

Since C++20 P1331R2, a constructor call doesn't need to leave all of an object initialized.

Take this https://godbolt.org/z/63vo7MP9o:

#include <memory>

union U {
  int arr[4];
};

struct S1 {
  union {
    int arr[4];
  };
  S1() = default;
  constexpr S1(const S1& other) {
    arr[0] = other.arr[0];
    std::destroy(arr + 1, arr + 4);
  }
};

struct S2 {
  union {
    int arr[4];
  };
};

template<typename T>
consteval bool f() {
    T u1;
    u1.arr[0] = 123;
    std::destroy(u1.arr + 1, u1.arr + 4);
    // assert(std::is_within_lifetime(&u1.arr) && std::is_within_lifetime(u1.arr + 0) && !std::is_within_lifetime(u1.arr + 1));
    T u2 = u1;
    return u2.arr[0] == 123;
}

static_assert(f<U>());
static_assert(f<S1>());
static_assert(f<S2>());

Clang errors on f<S2>():

<source>:36:15: error: static assertion expression is not an integral constant expression
   36 | static_assert(f<S2>());
      |               ^~~~~~~
<source>:30:12: note: subobject 'arr' is not initialized
   30 |     T u2 = u1;
      |            ^
<source>:30:12: note: in call to 'S2(u1)'
   30 |     T u2 = u1;
      |            ^~
<source>:36:15: note: in call to 'f<S2>()'
   36 | static_assert(f<S2>());
      |               ^~~~~~~
<source>:20:9: note: subobject declared here
   20 |     int arr[4];
      |         ^

It should not be a problem that the entirety of 'arr' isn't initialized (it was fine with the non-trivial copy constructor of S1 and with the union U)

GCC accepts this. MSVC does not accept f<U>() or f<S2>().

llvmbot commented 2 months ago

@llvm/issue-subscribers-clang-frontend

Author: Mital Ashok (MitalAshok)

Since C++20 [P1331R2](https://wg21.link/P1331R2), a constructor call doesn't need to leave all of an object initialized. Take this <https://godbolt.org/z/63vo7MP9o>: ```c++ #include <memory> union U { int arr[4]; }; struct S1 { union { int arr[4]; }; S1() = default; constexpr S1(const S1& other) { arr[0] = other.arr[0]; std::destroy(arr + 1, arr + 4); } }; struct S2 { union { int arr[4]; }; }; template<typename T> consteval bool f() { T u1; u1.arr[0] = 123; std::destroy(u1.arr + 1, u1.arr + 4); // assert(std::is_within_lifetime(&u1.arr) && std::is_within_lifetime(u1.arr + 0) && !std::is_within_lifetime(u1.arr + 1)); T u2 = u1; return u2.arr[0] == 123; } static_assert(f<U>()); static_assert(f<S1>()); static_assert(f<S2>()); ``` Clang errors on `f<S2>()`: ```c++ <source>:36:15: error: static assertion expression is not an integral constant expression 36 | static_assert(f<S2>()); | ^~~~~~~ <source>:30:12: note: subobject 'arr' is not initialized 30 | T u2 = u1; | ^ <source>:30:12: note: in call to 'S2(u1)' 30 | T u2 = u1; | ^~ <source>:36:15: note: in call to 'f<S2>()' 36 | static_assert(f<S2>()); | ^~~~~~~ <source>:20:9: note: subobject declared here 20 | int arr[4]; | ^ ``` It should not be a problem that the entirety of `'arr'` isn't initialized (it was fine with the non-trivial copy constructor of `S1` and with the union `U`) GCC accepts this. MSVC does not accept `f<U>()` or `f<S2>()`.
llvmbot commented 2 months ago

@llvm/issue-subscribers-c-20

Author: Mital Ashok (MitalAshok)

Since C++20 [P1331R2](https://wg21.link/P1331R2), a constructor call doesn't need to leave all of an object initialized. Take this <https://godbolt.org/z/63vo7MP9o>: ```c++ #include <memory> union U { int arr[4]; }; struct S1 { union { int arr[4]; }; S1() = default; constexpr S1(const S1& other) { arr[0] = other.arr[0]; std::destroy(arr + 1, arr + 4); } }; struct S2 { union { int arr[4]; }; }; template<typename T> consteval bool f() { T u1; u1.arr[0] = 123; std::destroy(u1.arr + 1, u1.arr + 4); // assert(std::is_within_lifetime(&u1.arr) && std::is_within_lifetime(u1.arr + 0) && !std::is_within_lifetime(u1.arr + 1)); T u2 = u1; return u2.arr[0] == 123; } static_assert(f<U>()); static_assert(f<S1>()); static_assert(f<S2>()); ``` Clang errors on `f<S2>()`: ```c++ <source>:36:15: error: static assertion expression is not an integral constant expression 36 | static_assert(f<S2>()); | ^~~~~~~ <source>:30:12: note: subobject 'arr' is not initialized 30 | T u2 = u1; | ^ <source>:30:12: note: in call to 'S2(u1)' 30 | T u2 = u1; | ^~ <source>:36:15: note: in call to 'f<S2>()' 36 | static_assert(f<S2>()); | ^~~~~~~ <source>:20:9: note: subobject declared here 20 | int arr[4]; | ^ ``` It should not be a problem that the entirety of `'arr'` isn't initialized (it was fine with the non-trivial copy constructor of `S1` and with the union `U`) GCC accepts this. MSVC does not accept `f<U>()` or `f<S2>()`.
frederick-vs-ja commented 2 months ago

CWG2658 is perhaps related, but that case is different.

The resolution doesn't seem to make copying a partially living array in a union valid in constant evaluation. IIUC, in constant evaluation when the copy construction is trivial, T u2 = u1; is considered to be decomposed to

T u2{.arr{u1.arr[0], u1.arr[1], u1.arr[2], u1.arr[3]}};

And then it's UB to read u1.arr[1] etc. which are dead?

I guess it would be better to submit a CWG issue for this.

Edit: Not A Defect??! So Clang seems to be correct here.