[libcxx-commits] [PATCH] D94409: [libc++] [C++2b] [P1048] Add is_scoped_enum and is_scoped_enum_v.

[Louis Dionne via Phabricator via libcxx-commits]

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

We're very very close, but there's still a few nits IMO.



================
Comment at: libcxx/test/std/utilities/meta/meta.unary/meta.unary.prop/is_scoped_enum.pass.cpp:88
+  test_positive<CEnum3>();
+  test_positive<TestMembers::CE1>();
+
----------------
I don't think this one is necessary, it's the same as testing a scoped enum outside of a class. In my mind, it's akin to testing that a scoped enumeration defined in a namespace behaves correctly -- it's testing the compiler, which isn't necessary IMO.


================
Comment at: libcxx/test/std/utilities/meta/meta.unary/meta.unary.prop/is_scoped_enum.pass.cpp:91
+  test_negative<Enum>();
+  test_negative<TestMembers::E1>();
+
----------------
Same, not necessary IMO.


================
Comment at: libcxx/test/std/utilities/meta/meta.unary/meta.unary.prop/is_scoped_enum.pass.cpp:117
+  test_negative<decltype(&func2)>();
+  test_negative<decltype(TestMembers::static_data)>();
+  test_negative<decltype(&TestMembers::static_data)>();
----------------
Unnecessary IMO, this is just testing `long&`.


================
Comment at: libcxx/test/std/utilities/meta/meta.unary/meta.unary.prop/is_scoped_enum.pass.cpp:121
+  test_negative<decltype(&TestMembers::static_method)>();
+  test_negative<decltype(std::declval<TestMembers>().data)>();
+  test_negative<decltype(&TestMembers::data)>();
----------------
Unnecessary IMO, this is just testing `long&`.


================
Comment at: libcxx/test/std/utilities/meta/meta.unary/meta.unary.prop/is_scoped_enum.pass.cpp:123
+  test_negative<decltype(&TestMembers::data)>();
+  test_negative<decltype(std::declval<TestMembers>().method())>();
+  test_negative<decltype(&TestMembers::method)>();
----------------
Not necessary, this is just testing for `int`.


================
Comment at: libcxx/test/std/utilities/meta/meta.unary/meta.unary.prop/is_scoped_enum.pass.cpp:67
+
+typedef void (*FunctionPtr)();
+
----------------
curdeius wrote:
> ldionne wrote:
> > Can you add a test for a function type (not a function pointer)? Also, for pointer to member function, pointer to data members and other fun things like that?
> Is that what you meant? Any other ideas?
Hmm, almost. I would have used `using FunctionType = void()` and then `test_negative<FunctionType>()` instead -- `decltype(func1)` is resolved as a reference to a function unless I'm mistaken.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94409