Fix fpos requirements & cleanup

[zoecarver]

Summary

This patch implements P0759R1.

[zoecarver]

Update www

[EricWF]

I don't really understand how a lot of the changes in this patch implement the changes in the paper. Can you explain the rationale for each change?

The tests for each individual change need to be put in the exiting test files for the operations they modify.

I think this patch needs some work.

include/string

557 ↗

(On Diff #194426)

fpos<_StateT> is the same as fpos in this context, no?

559 ↗

(On Diff #194426)

This seems wrong. It's just another copy constructor overload, but that takes by value.

test/std/strings/fpos/fpos.pass.cpp

1 ↗

(On Diff #194426)

These tests should be integrated into the existing tests files under std/input.output/iostreams.base/fpos/fpos.operations.

The test name and directory correspond directly to the stable name of the section of the standard it is testing.

33 ↗

(On Diff #194426)

No test for the trivial copy constructor requirements?

[zoecarver]

I got confused about what type o was in the paper. This patch should conform to the paper properly. I will update the tests in the correct places but wanted to get this part out for review first.

[zoecarver]

include/string

553 ↗

(On Diff #194864)

The overloads conflict with the plus and minus overloads for streamoff. I think it would be fine to use those overloads (because they will convert both sides to streamoff), but I am also happy to define the behavior explicitly.

[zoecarver]

• update & add tests
• update synopsis
• clean up members / operators

[zoecarver]

include/string

529 ↗

(On Diff #199950)

I re-ordered and updated these operators and member functions. I don't think it changes any of the functionality so I am fine without the updates (say the word, and I will remove the changes).

[zoecarver]

• remove unneeded assignment operator
• move tests to correct location

(sorry for the repetitive updates)

[mclow.lists]

include/string

529 ↗

(On Diff #199950)

I think that's a good idea. This paper is supposed to be just wording cleanup; no changes to functionality, so lets make this patch just a test cleanup.

The re-ordering, etc of the member functions and operators can be done afterwards as a NFC patch.

[zoecarver]

include/string

529 ↗

(On Diff #199950)

Sounds good. I will remove this part of the patch and re-upload the diff.

[zoecarver]

• remove changes to fpos

[mclow.lists]

test/std/input.output/iostreams.base/fpos/fpos.operations/fpos.pass.cpp

20 ↗

(On Diff #200754)

Why is this unsupported? fpos was a thing in C++98/03.

34 ↗

(On Diff #200754)

error: static_assert with no message is a C++17 extension

[zoecarver]

test/std/input.output/iostreams.base/fpos/fpos.operations/fpos.pass.cpp

20 ↗

(On Diff #200754)

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?

[ldionne]

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?

test/std/input.output/iostreams.base/fpos/fpos.operations/fpos.pass.cpp

20 ↗

(On Diff #200754)

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.

28 ↗

(On Diff #200754)

std::declval<T const&>() instead?

34 ↗

(On Diff #200754)

@zoecarver Action item: add a message!

54 ↗

(On Diff #200754)

I don't think you need any other types.

[zoecarver]

Yes, some of the tests are duplicated (subtraction, addition, comparison, etc.) but, I think it is nice to have it match the order of the table in the standard. Say the word and I'll remove the duplication, though.

[zoecarver]

• fix review comments

[ldionne]

libcxx/www/cxx2a_status.html

96

Can you mark it as being implemented in LLVM 11.0?

[zoecarver]

These tests should be integrated into the existing tests files under std/input.output/iostreams.base/fpos/fpos.operations.

@EricWF Do you still want me to split this up? I'm going to commit this patch as-is (with the change @ldionne asked for) but, if you still want me to split this test up, post a comment here and I'll revert, make the change, and put the patch here for re-review.

[zoecarver]

Committed as df73e36dc6ffd2ede500189b075f7ac52794b3ad. I can't close the revision for some reason (probably because of the blocking reviews).

libcxx/test/std/input.output/iostreams.base/fpos/fpos.operations/fpos.pass.cpp

+//===----------------------------------------------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "test_macros.h"
+
+#include <string>
+#include <cassert>
+#include <type_traits>
+
+// <string>
+
+// template <class stateT>
+// class fpos;
+
+template<class T, class = void>
+struct is_equality_comparable : std::false_type { };
+
+template<class T>
+struct is_equality_comparable
+<T, typename std::enable_if<true, decltype(std::declval<T const&>() == std::declval<T const&>(),
+                                           (void)0)>::type
+> : std::true_type { };
+
+template<class T>
+void test_traits()
+{
+    static_assert(std::is_default_constructible <std::fpos<T> >::value, "");
+    static_assert(std::is_copy_constructible    <std::fpos<T> >::value, "");
+    static_assert(std::is_copy_assignable       <std::fpos<T> >::value, "");
+    static_assert(std::is_destructible          <std::fpos<T> >::value, "");
+    static_assert(is_equality_comparable        <std::fpos<T> >::value, "");
+
+    static_assert(std::is_trivially_copy_constructible<T>::value ==
+                  std::is_trivially_copy_constructible<std::fpos<T> >::value, "");
+    static_assert(std::is_trivially_copy_assignable<T>::value ==
+                  std::is_trivially_copy_assignable<std::fpos<T> >::value, "");
+    static_assert(std::is_trivially_destructible<T>::value ==
+                  std::is_trivially_destructible<std::fpos<T> >::value, "");
+}
+
+struct Foo { };
+
+int main(int, char**)
+{
+    test_traits<std::mbstate_t>();
+    test_traits<int>();
+    test_traits<Foo>();
+
+    // Position type requirements table 106 (in order):
+
+    std::streampos p1(42);
+    std::streamoff o1(p1);
+
+    {
+        assert(o1 == 42);
+    }
+    {
+        std::streampos p2(42);
+        std::streampos q1(43);
+        std::streampos const p3(44);
+        std::streampos const q2(45);
+        assert(p2 != q1);
+        assert(p3 != q2);
+        assert(p2 != q2);
+        assert(p3 != q1);
+    }
+    {
+        std::streampos p2 = p1 + o1;
+        assert(p2 == 84);
+    }
+    {
+        std::streampos& p2 = p1 += o1;
+        assert(p2 == 84);
+        assert(p1 == 84);
+    }
+    {
+        std::streampos p2 = p1 - o1;
+        assert(p2 == 42);
+    }
+    {
+        std::streampos& p2 = p1 -= o1;
+        assert(p2 == 42);
+        assert(p1 == 42);
+    }
+    {
+        std::streampos p2 = o1 + p1;
+        assert(p2 == 84);
+    }
+    {
+        std::streampos q1(42);
+        std::streamoff o2 = q1 - p1;
+        assert(o2 == 0);
+    }
+
+    return 0;
+}

libcxx/www/cxx2a_status.html

-->
   <tr><td><a href="https://wg21.link/P0476R2">P0476R2</a></td><td>LWG</td><td>Bit-casting object representations</td><td>Rapperswil</td><td></td><td></td></tr>
   <tr><td><a href="https://wg21.link/P0528R3">P0528R3</a></td><td>CWG</td><td>The Curious Case of Padding Bits, Featuring Atomic Compare-and-Exchange</td><td>Rapperswil</td><td></td><td></td></tr>
   <tr><td><a href="https://wg21.link/P0542R5">P0542R5</a></td><td>CWG</td><td>Support for contract based programming in C++</td><td>Rapperswil</td><td></td><td></td></tr>
   <tr><td><a href="https://wg21.link/P0556R3">P0556R3</a></td><td>LWG</td><td>Integral power-of-2 operations</td><td>Rapperswil</td><td>Complete</td><td>9.0</td></tr>
   <tr><td><a href="https://wg21.link/P0619R4">P0619R4</a></td><td>LWG</td><td>Reviewing Deprecated Facilities of C++17 for C++20</td><td>Rapperswil</td><td></td><td></td></tr>
   <tr><td><a href="https://wg21.link/P0646R1">P0646R1</a></td><td>LWG</td><td>Improving the Return Value of Erase-Like Algorithms</td><td>Rapperswil</td><td></td><td></td></tr>
   <tr><td><a href="https://wg21.link/P0722R3">P0722R3</a></td><td>CWG</td><td>Efficient sized delete for variable sized classes</td><td>Rapperswil</td><td>Complete</td><td>9.0</td></tr>
   <tr><td><a href="https://wg21.link/P0758R1">P0758R1</a></td><td>LWG</td><td>Implicit conversion traits and utility functions</td><td>Rapperswil</td><td>Complete</td><td></td></tr>
-  <tr><td><a href="https://wg21.link/P0759R1">P0759R1</a></td><td>LWG</td><td>fpos Requirements</td><td>Rapperswil</td><td></td><td></td></tr>
+  <tr><td><a href="https://wg21.link/P0759R1">P0759R1</a></td><td>LWG</td><td>fpos Requirements</td><td>Rapperswil</td><td>Complete</td><td></td></tr>
   <tr><td><a href="https://wg21.link/P0769R2">P0769R2</a></td><td>LWG</td><td>Add shift to &lt;algorithm&gt;</td><td>Rapperswil</td><td></td><td></td></tr>
   <tr><td><a href="https://wg21.link/P0788R3">P0788R3</a></td><td>LWG</td><td>Standard Library Specification in a Concepts and Contracts World</td><td>Rapperswil</td><td></td><td></td></tr>
   <tr><td><a href="https://wg21.link/P0879R0">P0879R0</a></td><td>LWG</td><td>Constexpr for swap and swap related functions Also resolves LWG issue 2800.</td><td>Rapperswil</td><td></td><td></td></tr>
   <tr><td><a href="https://wg21.link/P0887R1">P0887R1</a></td><td>LWG</td><td>The identity metafunction</td><td>Rapperswil</td><td>Complete</td><td>8.0</td></tr>
   <tr><td><a href="https://wg21.link/P0892R2">P0892R2</a></td><td>CWG</td><td>explicit(bool)</td><td>Rapperswil</td><td></td><td></td></tr>
   <tr><td><a href="https://wg21.link/P0898R3">P0898R3</a></td><td>LWG</td><td>Standard Library Concepts</td><td>Rapperswil</td><td></td><td></td></tr>
   <tr><td><a href="https://wg21.link/P0935R0">P0935R0</a></td><td>LWG</td><td>Eradicating unnecessarily explicit default constructors from the standard library</td><td>Rapperswil</td><td></td><td></td></tr>
   <tr><td><a href="https://wg21.link/P0941R2">P0941R2</a></td><td>CWG</td><td>Integrating feature-test macros into the C++ WD</td><td>Rapperswil</td><td><i>In progress</i></td><td></td></tr>