[libcxx-commits] [PATCH] D60491: Fix fpos requirements & cleanup

[Louis Dionne via Phabricator via libcxx-commits]

ldionne requested changes to this revision.
ldionne added a comment.
This revision now requires changes to proceed.
Herald added a subscriber: dexonsmith.

Some of the tests you added are already present in other files, for example `libcxx/test/std/input.output/iostreams.base/fpos/fpos.operations/subtraction.pass.cpp`. Any reason why you're duplicating those checks in `test/std/input.output/iostreams.base/fpos/fpos.operations/fpos.pass.cpp`?



================
Comment at: test/std/input.output/iostreams.base/fpos/fpos.operations/fpos.pass.cpp:20
+
+// UNSUPPORTED: c++98, c++03
+
----------------
zoecarver wrote:
> mclow.lists wrote:
> > Why is this unsupported? `fpos` was a thing in C++98/03.
> > 
> But the updates made here were not a thing until C++20. Should I still have the test for earlier versions? Or should I change this to only work after C++17? 
Since this is a DR, I think it's OK to assume the fix is there in prior standards. So you should remove the `UNSUPPORTED` annotation.


================
Comment at: test/std/input.output/iostreams.base/fpos/fpos.operations/fpos.pass.cpp:28
+<T, typename std::enable_if<true,
+                            decltype(std::declval<T&>() == std::declval<T&>(), (void)0)>::type
+> : std::true_type { };
----------------
`std::declval<T const&>()` instead?


================
Comment at: test/std/input.output/iostreams.base/fpos/fpos.operations/fpos.pass.cpp:34
+{
+    static_assert(std::is_default_constructible <std::fpos<T>>::value);
+    static_assert(std::is_copy_constructible    <std::fpos<T>>::value);
----------------
mclow.lists wrote:
> error: static_assert with no message is a C++17 extension
@zoecarver Action item: add a message!


================
Comment at: test/std/input.output/iostreams.base/fpos/fpos.operations/fpos.pass.cpp:54
+    test_traits<int>();
+    test_traits<Foo>(); // TODO: what other types should I add?
+
----------------
I don't think you need any other types.


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

https://reviews.llvm.org/D60491