[libcxx-commits] [PATCH] D77961: WIP: Adds `std::convertible_to` to <concepts>

[Michael Schellenberger Costa via Phabricator via libcxx-commits]

miscco added a comment.

I left mostly some nits with respect to readability. Great job



================
Comment at: libcxx/include/concepts:168
+
+template<class _Fp, class _Tp>
+inline constexpr auto __is_explicitly_convertible =
----------------
The standard depicts those as `From` and `To` and in the [[ https://github.com/llvm-mirror/libcxx/blob/78d6a7767ed57b50122a161b91f59f19c9bd0d19/include/type_traits#L1494-L1497| type trait ]] it is also `_From` and `_To`.

So lets be consistent and use the same naming


================
Comment at: libcxx/test/std/concepts/lang/convertible_to.pass.cpp:21
+constexpr void NotConvertible() {
+  static_assert(
+      !std::convertible_to<typename Mod1<From>::type, typename Mod2<To>::type>);
----------------
Does that compile without the second string argument? It is used everywhere else int eh test suite even if its empty


================
Comment at: libcxx/test/std/concepts/lang/convertible_to.pass.cpp:146
+  OneWayConvertible<int&, int, Identity, Mod2>();
+  TwoWayConvertible<const int&, int, Identity, Identity>();
+  TwoWayConvertible<const int&, const int, Identity, Identity>();
----------------
Should we move this fundamental checks that do not rely on any Modifier to their own function? These are the most common ones so as a reader it would be easier to check them in isolation and then have the specialized function do the tricky stuff


================
Comment at: libcxx/test/std/concepts/lang/convertible_to.pass.cpp:218
+
+struct ImplicitlyConstructibleAndConvertible {
+  ImplicitlyConstructibleAndConvertible(float);
----------------
I am wondering whether it is cleaner to define those basic types above and for the constructors and then create all the others by inheritance, e.g:

```
struct ImplicitlyConstructibleAndConvertible : public ImplicitelyConstructible, public Convertible {
{
    using ImplicitelyConstructible::ImplicitelyConstructible;
};
```


================
Comment at: libcxx/test/std/concepts/lang/convertible_to.pass.cpp:261
+};
+struct ExplicitlyConvertibleConst {
+  using T = void (*)();
----------------
Newlines missing from here


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77961