[libcxx-commits] [PATCH] D58019: Add is_nothrow_convertible (P0758R1)

[Louis Dionne via Phabricator via libcxx-commits]

ldionne requested changes to this revision.
ldionne added a comment.
This revision now requires changes to proceed.

This is looking better and better.



================
Comment at: include/type_traits:1563
+    
+    template<typename __Fm, typename __To>
+    static bool_constant<noexcept(__test_noexcept<__To>(declval<__Fm>()))>
----------------
The rule is usually 1 underscore followed by a capital letter, or 2 underscores followed by a small letter. For template parameters, we use the former. Here you have 2 underscores followed by a capital.

This is pedantic, but since you're a new contributor, we might as well start on the right foot.


================
Comment at: include/type_traits:1564
+    template<typename __Fm, typename __To>
+    static bool_constant<noexcept(__test_noexcept<__To>(declval<__Fm>()))>
+    __test(int);
----------------
You can hoist this helper function and its friend above outside of the class. This way you don't get an instantiation of `__test_noexcept<_Tp>` for each pair of `_To` and `_From`, only one for each type. You'll still get an instantiation of `__test` for each pair of `_To` and `_From`, but at least you don't need to play games with the names of the template parameters if you hoist the function outside the class.


================
Comment at: include/type_traits:1567
+    
+    template<typename, typename>
+    static false_type
----------------
I don't think you need this overload anymore. Indeed, you already know that `_Fm` is convertible to `_To` because you checked with `std::is_convertible`.


================
Comment at: test/std/type_traits/is_nothrow_convertible.pass.cpp:15
+
+#include "test_macros.h"
+
----------------
I think you don't need this header.


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

https://reviews.llvm.org/D58019