llvm / llvm-project

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

[Clang] `__is_trivially_copyable` returns wrong result for classes with a `const` member. #100198

Open philnik777 opened 1 month ago

philnik777 commented 1 month ago
struct S {
  int offset_;
};

struct pair
{
  int first;
  const S second;

  pair& operator=(const pair&) = default;
  pair& operator=(pair&&) = default;
};

static_assert(__is_trivially_copyable(pair));

pair should be trivially copyable, but isn't reported as such currently. Note that removing the defaulted assignment operators and adding pair(const pair&) = default; results in Clang reporting pair as trvially copable correctly.

llvmbot commented 1 month ago

@llvm/issue-subscribers-clang-frontend

Author: Nikolas Klauser (philnik777)

```c++ struct S { int offset_; }; struct pair { int first; const S second; pair& operator=(const pair&) = default; pair& operator=(pair&&) = default; }; static_assert(__is_trivially_copyable(pair)); ``` `pair` should be trivially copyable, but isn't reported as such currently. Note that removing the defaulted assignment operators and adding `pair(const pair&) = default;` results in Clang reporting `pair` as trvially copable correctly.
Endilll commented 1 month ago

GCC and EDG agree that the code is valid: https://godbolt.org/z/ch5ndY4ra

llvmbot commented 1 month ago

@llvm/issue-subscribers-c-1

Author: Nikolas Klauser (philnik777)

```c++ struct S { int offset_; }; struct pair { int first; const S second; pair& operator=(const pair&) = default; pair& operator=(pair&&) = default; }; static_assert(__is_trivially_copyable(pair)); ``` `pair` should be trivially copyable, but isn't reported as such currently. Note that removing the defaulted assignment operators and adding `pair(const pair&) = default;` results in Clang reporting `pair` as trvially copable correctly.
zygoloid commented 1 month ago

Should S be trivially copyable? It looks to me like:

AaronBallman commented 1 month ago

References for what @zygoloid posted above:

And the library defers to the language in this case: https://eel.is/c++draft/type.traits#tab:meta.unary.prop-row-5-column-2-sentence-1, https://eel.is/c++draft/basic.types.general#9.sentence-2, https://eel.is/c++draft/class.prop#1

Endilll commented 1 month ago

So http://eel.is/c++draft/class.copy.ctor#11 was a red herring

AaronBallman commented 1 month ago

Not a red herring, I think it's more that it doesn't apply because the copy constructor is deleted and there's no move constructor.

frederick-vs-ja commented 1 month ago

This seems related to CWG1734.

philnik777 commented 1 month ago

If that's the case, shouldn't

struct S {
  int offset_;
};

struct pair
{
  int first = {};
  const S second = {};

  pair() = default;
  pair(const pair&) = default;
  pair(pair&&) = default;
  pair& operator=(const pair&) = default;
  pair& operator=(pair&&) = default;
};

be considered trivially copyable? I can definitely copy construct it, but it's not trivially copyable according to Clang: https://godbolt.org/z/qn4K1G6sY

AaronBallman commented 1 month ago

We have other issues regarding calculating what is trivially copyable, e.g., https://github.com/llvm/llvm-project/issues/38398