[libcxx-commits] [PATCH] D58987: Make underlying_type SFINAE-friendly

Zoe Carver via Phabricator via libcxx-commits libcxx-commits at lists.llvm.org
Wed Mar 6 17:10:45 PST 2019


zoecarver marked 2 inline comments as done.
zoecarver added inline comments.


================
Comment at: test/std/utilities/meta/meta.trans/meta.trans.other/underlying_type.fail.cpp:22
+{
+	test_sfinae(0); // expected-error {{no matching function for call to 'test_sfinae'}}
+
----------------
EricWF wrote:
> No need to get SFINAE involved here. Simply write:
> 
> ```
> int main() {
>   using T = std::underlying_type<int>::type; // expected-error {{boom}}
> }
> ```
Yes, your right. That's what happens when you copy and paste. 


================
Comment at: test/std/utilities/meta/meta.trans/meta.trans.other/underlying_type.pass.cpp:30
 
+template<class T>
+std::enable_if_t<
----------------
mclow.lists wrote:
> EricWF wrote:
> > I'm not sure what this test is actually testing. How about:
> > 
> > ```
> > template <class T, class = typename std::underlying_type<T>::type>
> > std::true_type test_sfinae(int);
> > template <class T>
> > std::false_type test_sfinae(...);
> > 
> > int main() {
> >   static_assert(decltype(test_sfinae<int>(0))::value == false, "not an enum");
> >    static_assert(decltype(test_sfinae<E>(0))::value == true, "is an enum");
> > 
> > }
> > ```
> Now that we have the failing test, do we need this test at all?
I like that idea @EricWF. 

Thinking more about how to structure these tests, I think the "fail" test is not super helpful (as is). What this should be testing for is whether passing `underlying_type` a non-enum type breaks the compiler. Given that it _shouldn't_ break the compiler, I think it is more helpful to make sure it passes than to try and make sure it fails in the "right" way. Let me know if you disagree though.


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

https://reviews.llvm.org/D58987





More information about the libcxx-commits mailing list