[libcxx-commits] [PATCH] D102992: [libcxx][type_traits] remove `std::is_literal_type` and `std::result_of` for C++20

[Louis Dionne via Phabricator via libcxx-commits]

ldionne requested changes to this revision.
ldionne added inline comments.
This revision now requires changes to proceed.


================
Comment at: libcxx/test/std/atomics/atomics.types.operations/atomics.types.operations.req/ctor.pass.cpp:38
         typedef std::atomic<Tp> Atomic;
-        static_assert(std::is_literal_type<Atomic>::value, "");
+        static_assert(std::is_trivial<Atomic>::value, "");
         constexpr Tp t(42);
----------------
Wmbat wrote:
> For some reason, this `static_assert` fails under the [GCC/C++20](https://buildkite.com/llvm-project/libcxx-ci/builds/3515#6c83e656-d43d-4f07-abd0-09be8130dcf9) build, but passes for all Clang builds. I have no idea why
Okay, I dug a bit more into this and here's what I think we want:

1. Remove this static assertion, it doesn't test anything anymore.
2. Add `libcxx/test/std/atomics/atomics.types.operations/atomics.types.operations.req/dtor.pass.cpp`, and add the following to it:
```
template <class Tp>
struct CheckTriviallyDestructible {
    void operator()() const {
        typedef std::atomic<Tp> Atomic;
        static_assert(std::is_trivially_destructible<Atomic>::value, "");
    }
};

TestEachIntegralType<CheckTriviallyDestructible>()();
TestEachFloatingPointType<CheckTriviallyDestructible>()(); // needs to be added to `atomic_helpers.h`
TestEachPointerType< CheckTriviallyDestructible >()(); // needs to be added to `atomic_helpers.h`
```

3. Add `libcxx/test/std/atomics/atomics.types.generic/standard_layout.compile.pass.cpp` and add the following in it:

```
template <class Tp>
struct CheckStandardLayout {
    void operator()() const {
        typedef std::atomic<Tp> Atomic;
        static_assert(std::is_standard_layout<Atomic>::value, "");
    }
};

CheckStandardLayout<bool>();
TestEachIntegralType<CheckStandardLayout>()();
TestEachFloatingPointType<CheckStandardLayout>()();
TestEachPointerType<CheckStandardLayout>()();
```

In other words, make sure that `std::atomic<bool|Integral|FloatingPoint|Pointer>` is standard layout, and that `std::atomic<Integral|FloatingPoint|Pointer>` is trivially destructible. I think that's pretty much the extent of the guarantees provided by the standard.



================
Comment at: libcxx/test/std/utilities/any/any.class/not_literal_type.pass.cpp:23
 int main(int, char**) {
     static_assert(!std::is_literal_type<std::any>::value, "");
 
----------------
Quuxplusone wrote:
> I think @ldionne's comment on the `optional` test also applies here: This test is merely testing that `std::any` isn't constexpr-constructible, specifically in C++17? I don't think that's a useful test. I think this entire file should be deleted and never spoken of again. :)  [Any objections?]
Yeah, I would remove this test entirely. I don't think it's checking anything useful anymore.


================
Comment at: libcxx/test/std/utilities/meta/meta.trans/meta.trans.other/result_of.deprecated.fail.cpp:25-26
+int main(int, char**) {
+  typedef std::result_of< // expected-warning {{'result_of<CallableStruct (int)>' is deprecated}}
+      CallableStruct(int)>::type f;
 }
----------------
Why not write this as:

```
typedef std::result_of<CallableStruct(int)>::type f; // expected-warning {{'result_of<CallableStruct (int)>' is deprecated}}
```

Unless there is a reason for the funky indentation?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D102992/new/

https://reviews.llvm.org/D102992