[libc++] Implement P2467R1: Support exclusive mode for fstreams

[ldionne]

Summary

This patch brings std::ios_base::noreplace from P2467R1 to libc++.
This requires compiling the shared library in C++23 mode since otherwise
fstream::open(...) doesn't know about the new flag.

[PragmaTwice]

apply clang format patch

[PragmaTwice]

apply generated docs patch from CI

[PragmaTwice]

generate ios.version.compile.pass.cpp

[PragmaTwice]

fix conditional inclusion

[PragmaTwice]

replace path.c_str() with path.string().c_str()

[philnik]

Thanks for working on this! Please goo through https://libcxx.llvm.org/Contributing.html#pre-commit-check-list and make sure that you did everything listed there.

libcxx/include/fstream

557

We don't want anybody to think that this is configurable. Same for the other places.

libcxx/test/std/input.output/file.streams/fstreams/fstream.cons/noreplace.pass.cpp

1

It seems like you are missing tests for everything but in | out | trunc | noreplace. Please add them.

17

29

We normally avoid this to make it simpler for readers to see where symbols are coming from. In this case you are using fs in a bunch of different places too, making it even harder to read. It also looks like there is just a single use of fs:: anywhere, so it doesn't even remove any verbosity.

54

Why aren't you using std::filesystem::remove(p)?

[PragmaTwice]

replace defined(__cpp_lib_ios_noreplace) with _LIBCPP_STD_VER >= 23

[PragmaTwice]

rebase

[PragmaTwice]

update test file

[PragmaTwice]

finish changes requested by philnik, so thanks!

[ldionne]

Thanks for the patch! This looks reasonable but I think we're missing coverage.

libcxx/include/ios

61

libcxx/test/std/input.output/file.streams/fstreams/fstream.cons/noreplace.pass.cpp

2

This should be inside fstream.cons/path.pass.cpp instead.

And we're missing tests for the other places where this flag can now be used, like basic_filebuf, basic_ofstream, etc. Those tests should be added to their existing tests for constructors that take openmode.

[PragmaTwice]

add since C++23 comment

libcxx/test/std/input.output/file.streams/fstreams/fstream.cons/noreplace.pass.cpp

2

Hi @ldionne, thanks for your review.

I notice that there is a comment UNSUPPORTED: c++03, c++11, c++14 in path.pass.cpp, but it seems we need UNSUPPORTED: c++03, c++11, c++14, c++17, c++20 for noreplace. Hence I wonder if a new test file is more suitable here?

Thanks!

[philnik]

libcxx/test/std/input.output/file.streams/fstreams/fstream.cons/noreplace.pass.cpp

2

You can guard the new test with TEST_STD_VER >= 23.

[PragmaTwice]

Added more tests according to @Idionne 's suggestions

[PragmaTwice]

Hi @ldionne @philnik , more tests are added now, feel free to review it again. So thanks!

[philnik]

LGTM. Let's see what @ldionne thinks.

[philnik]

Please also add a release note to libcxx/docs/ReleaseNotes.rst under the Implemented Papers section and make sure CI is green (you probably have to rebase).

[PragmaTwice]

rebase to latest main branch and add P2467R1 to release note

[Mordante]

I didn't do a review, I was mainly curious what this paper did. I did discover one small issue.

libcxx/docs/ReleaseNotes.rst

52 ↗

(On Diff #481838)

Can you update libcxx/docs/Status/Cxx2bPapers.csv` too?

[ldionne]

Thanks for adding test coverage! This looks good to me, however we have several CI failures that would need to be fixed. It looks like UBSan, Windows and a few others are not happy with this change as-is.

[Mordante]

@PragmaTwice we're moving to GitHub PRs it would be great when this patch can be finished before moving. Are you interested in finishing it? If not I'll take over. Please start by rebasing the patch to see whether it passes the CI.

[ldionne]

[Github PR Transition cleanup]

Commandeering to finish.

[ldionne]

Rebase, fix tests. In particular, this patch requires building the dylib with C++23.

[ldionne]

Blocked on https://github.com/llvm/llvm-project/pull/66824.

[PragmaTwice]

Hi @Mordante @ldionne, sorry for the delayed response.

Is there anything I can assist with now, or it is ready for merging?

[ldionne]

In D137640#4656452, @PragmaTwice wrote:

Hi @Mordante @ldionne, sorry for the delayed response.

Is there anything I can assist with now, or it is ready for merging?

No, I think this is almost ready to go now.

[ldionne]

Rebase.

[ldionne]

I'll XFAIL the few remaining tests on AIX and ping the AIX folks to get some support. The rest looks good so I'm shipping this.

[ldionne]

XFAILs

libcxx/docs/FeatureTestMacroTable.rst

.. table:: Current Status
     ``__cpp_lib_constexpr_memory``                    ``202202L``
     ------------------------------------------------- -----------------
     ``__cpp_lib_constexpr_typeinfo``                  *unimplemented*
     ------------------------------------------------- -----------------
     ``__cpp_lib_forward_like``                        ``202207L``
     ------------------------------------------------- -----------------
     ``__cpp_lib_invoke_r``                            *unimplemented*
     ------------------------------------------------- -----------------
+    ``__cpp_lib_ios_noreplace``                       ``202207L``
+    ------------------------------------------------- -----------------
     ``__cpp_lib_is_scoped_enum``                      ``202011L``
     ------------------------------------------------- -----------------
     ``__cpp_lib_move_only_function``                  *unimplemented*
     ------------------------------------------------- -----------------
     ``__cpp_lib_optional``                            ``202110L``
     ------------------------------------------------- -----------------
     ``__cpp_lib_out_ptr``                             *unimplemented*
     ------------------------------------------------- -----------------

libcxx/include/fstream

case ios_base::in | ios_base::binary:
     return "rb" _LIBCPP_FOPEN_CLOEXEC_MODE;
   case ios_base::in | ios_base::out | ios_base::binary:
     return "r+b" _LIBCPP_FOPEN_CLOEXEC_MODE;
   case ios_base::in | ios_base::out | ios_base::trunc | ios_base::binary:
     return "w+b" _LIBCPP_FOPEN_CLOEXEC_MODE;
   case ios_base::in | ios_base::out | ios_base::app | ios_base::binary:
   case ios_base::in | ios_base::app | ios_base::binary:
     return "a+b" _LIBCPP_FOPEN_CLOEXEC_MODE;
+#if defined(__cpp_lib_ios_noreplace)
+  case ios_base::out | ios_base::noreplace:
+  case ios_base::out | ios_base::trunc | ios_base::noreplace:
+    return "wx" _LIBCPP_FOPEN_CLOEXEC_MODE;
+  case ios_base::in | ios_base::out | ios_base::trunc | ios_base::noreplace:
+    return "w+x" _LIBCPP_FOPEN_CLOEXEC_MODE;
+  case ios_base::out | ios_base::binary | ios_base::noreplace:
+  case ios_base::out | ios_base::trunc | ios_base::binary | ios_base::noreplace:
+    return "wbx" _LIBCPP_FOPEN_CLOEXEC_MODE;
+  case ios_base::in | ios_base::out | ios_base::trunc | ios_base::binary | ios_base::noreplace:
+    return "w+bx" _LIBCPP_FOPEN_CLOEXEC_MODE;
+#endif
   default:
     return nullptr;
   }
   __libcpp_unreachable();
 }
 
 template <class _CharT, class _Traits>
 basic_filebuf<_CharT, _Traits>*
if (__file_ == nullptr)
             break;
         case ios_base::in | ios_base::out | ios_base::trunc | ios_base::binary:
             __mdstr = L"w+b";
             break;
         case ios_base::in | ios_base::out | ios_base::app | ios_base::binary:
         case ios_base::in | ios_base::app | ios_base::binary:
             __mdstr = L"a+b";
             break;
+#  if defined(__cpp_lib_ios_noreplace)
+        case ios_base::out | ios_base::noreplace:
+        case ios_base::out | ios_base::trunc | ios_base::noreplace:
+          __mdstr = L"wx";
+          break;
+        case ios_base::in | ios_base::out | ios_base::trunc | ios_base::noreplace:
+          __mdstr = L"w+x";
+          break;
+        case ios_base::out | ios_base::binary | ios_base::noreplace:
+        case ios_base::out | ios_base::trunc | ios_base::binary | ios_base::noreplace:
+          __mdstr = L"wbx";
+          break;
+        case ios_base::in | ios_base::out | ios_base::trunc | ios_base::binary | ios_base::noreplace:
+          __mdstr = L"w+bx";
+          break;
+#  endif
         default:
             __rt = nullptr;
             break;
         }
         if (__rt)
         {
             __file_ = _wfopen(__s, __mdstr);
             if (__file_)

libcxx/include/ios

public:
     static constexpr iostate failbit;
     static constexpr iostate goodbit;
 
     typedef T3 openmode;
     static constexpr openmode app;
     static constexpr openmode ate;
     static constexpr openmode binary;
     static constexpr openmode in;
+    static constexpr openmode noreplace;
     static constexpr openmode out;
     static constexpr openmode trunc;
 
     typedef T4 seekdir;
     static constexpr seekdir beg;
     static constexpr seekdir cur;
     static constexpr seekdir end;
 
public:
     static const iostate failbit = 0x4;
     static const iostate goodbit = 0x0;
 
     typedef unsigned int openmode;
     static const openmode app    = 0x01;
     static const openmode ate    = 0x02;
     static const openmode binary = 0x04;
     static const openmode in     = 0x08;
-    static const openmode out    = 0x10;
-    static const openmode trunc  = 0x20;
+#if defined(__cpp_lib_ios_noreplace)
+    static const openmode noreplace = 0x40;
+#endif
+    static const openmode out   = 0x10;
+    static const openmode trunc = 0x20;
 
     enum seekdir {beg, cur, end};
 
 #if _LIBCPP_STD_VER <= 14
     typedef iostate      io_state;
     typedef openmode     open_mode;
     typedef seekdir      seek_dir;
 

libcxx/include/version

 __cpp_lib_incomplete_container_elements                 201505L <forward_list> <list> <vector>
 __cpp_lib_int_pow2                                      202002L <bit>
 __cpp_lib_integer_comparison_functions                  202002L <utility>
 __cpp_lib_integer_sequence                              201304L <utility>
 __cpp_lib_integral_constant_callable                    201304L <type_traits>
 __cpp_lib_interpolate                                   201902L <cmath> <numeric>
 __cpp_lib_invoke                                        201411L <functional>
 __cpp_lib_invoke_r                                      202106L <functional>
+__cpp_lib_ios_noreplace                                 202207L <ios>
 __cpp_lib_is_aggregate                                  201703L <type_traits>
 __cpp_lib_is_constant_evaluated                         201811L <type_traits>
 __cpp_lib_is_final                                      201402L <type_traits>
 __cpp_lib_is_invocable                                  201703L <type_traits>
 __cpp_lib_is_layout_compatible                          201907L <type_traits>
 __cpp_lib_is_nothrow_convertible                        201806L <type_traits>
 __cpp_lib_is_null_pointer                               201309L <type_traits>
 __cpp_lib_is_pointer_interconvertible                   201907L <type_traits>

 # define __cpp_lib_constexpr_bitset                     202207L
 # define __cpp_lib_constexpr_charconv                   202207L
 // # define __cpp_lib_constexpr_cmath                      202202L
 # undef  __cpp_lib_constexpr_memory
 # define __cpp_lib_constexpr_memory                     202202L
 // # define __cpp_lib_constexpr_typeinfo                   202106L
 # define __cpp_lib_forward_like                         202207L
 // # define __cpp_lib_invoke_r                             202106L
+# define __cpp_lib_ios_noreplace                        202207L
 # define __cpp_lib_is_scoped_enum                       202011L
 // # define __cpp_lib_move_only_function                   202110L
 # undef  __cpp_lib_optional
 # define __cpp_lib_optional                             202110L
 // # define __cpp_lib_out_ptr                              202106L
 // # define __cpp_lib_ranges_chunk                         202202L
 // # define __cpp_lib_ranges_chunk_by                      202202L
 // # define __cpp_lib_ranges_iota                          202202L

libcxx/test/std/input.output/file.streams/fstreams/fstream.cons/noreplace.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
+//
+//===----------------------------------------------------------------------===//
+
+// UNSUPPORTED: c++03, c++11, c++14, c++17, c++20
+// UNSUPPORTED: no-filesystem
+
+// Filesystem is supported on Apple platforms starting with macosx10.15.
+// UNSUPPORTED: use_system_cxx_lib && target={{.+}}-apple-macosx10.{{9|10|11|12|13|14}}
+
+// <fstream>
+
+// plate <class charT, class traits = char_traits<charT> >
+// class basic_fstream
+
+// explicit basic_fstream(const filesystem::path& s,
+//     ios_base::openmode mode = ios_base::in|ios_base::out);
+
+#include <fstream>
+#include <filesystem>
+#include <cassert>
+#include "test_macros.h"
+#include "platform_support.h"
+
+namespace fs = std::filesystem;
+
+int main(int, char**) {
+  fs::path p = get_temp_file_name();
+  {
+    std::fstream fs(
+        p.c_str(), std::ios_base::in | std::ios_base::out | std::ios_base::trunc | std::ios_base::noreplace);
+    assert(fs.rdstate() == std::ios_base::failbit);
+  }
+  std::remove(p.string().c_str());
+
+  {
+    std::fstream fs(
+        p.c_str(), std::ios_base::in | std::ios_base::out | std::ios_base::trunc | std::ios_base::noreplace);
+    double x = 0;
+    fs << 3.25;
+    fs.seekg(0);
+    fs >> x;
+    assert(x == 3.25);
+
+    fs.close();
+    std::fstream fs2(
+        p.c_str(), std::ios_base::in | std::ios_base::out | std::ios_base::trunc | std::ios_base::noreplace);
+    assert(fs2.rdstate() == std::ios_base::failbit);
+  }
+  std::remove(p.string().c_str());
+
+#ifndef TEST_HAS_NO_WIDE_CHARACTERS
+  {
+    std::wfstream fs(
+        p.c_str(), std::ios_base::in | std::ios_base::out | std::ios_base::trunc | std::ios_base::noreplace);
+    double x = 0;
+    fs << 3.25;
+    fs.seekg(0);
+    fs >> x;
+    assert(x == 3.25);
+
+    fs.close();
+    std::wfstream fs2(
+        p.c_str(), std::ios_base::in | std::ios_base::out | std::ios_base::trunc | std::ios_base::noreplace);
+    assert(fs2.rdstate() == std::ios_base::failbit);
+  }
+  std::remove(p.string().c_str());
+#endif
+}

libcxx/test/std/language.support/support.limits/support.limits.general/ios.version.compile.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
+//
+//===----------------------------------------------------------------------===//
+//
+// WARNING: This test was generated by generate_feature_test_macro_components.py
+// and should not be edited manually.
+//
+// clang-format off
+
+// UNSUPPORTED: no-localization
+
+// <ios>
+
+// Test the feature test macros defined by <ios>
+
+/*  Constant                   Value
+    __cpp_lib_ios_noreplace    202207L [C++2b]
+*/
+
+#include <ios>
+#include "test_macros.h"
+
+#if TEST_STD_VER < 14
+
+# ifdef __cpp_lib_ios_noreplace
+#   error "__cpp_lib_ios_noreplace should not be defined before c++2b"
+# endif
+
+#elif TEST_STD_VER == 14
+
+# ifdef __cpp_lib_ios_noreplace
+#   error "__cpp_lib_ios_noreplace should not be defined before c++2b"
+# endif
+
+#elif TEST_STD_VER == 17
+
+# ifdef __cpp_lib_ios_noreplace
+#   error "__cpp_lib_ios_noreplace should not be defined before c++2b"
+# endif
+
+#elif TEST_STD_VER == 20
+
+# ifdef __cpp_lib_ios_noreplace
+#   error "__cpp_lib_ios_noreplace should not be defined before c++2b"
+# endif
+
+#elif TEST_STD_VER > 20
+
+# ifndef __cpp_lib_ios_noreplace
+#   error "__cpp_lib_ios_noreplace should be defined in c++2b"
+# endif
+# if __cpp_lib_ios_noreplace != 202207L
+#   error "__cpp_lib_ios_noreplace should have the value 202207L in c++2b"
+# endif
+
+#endif // TEST_STD_VER > 20
+

libcxx/test/std/language.support/support.limits/support.limits.general/version.version.compile.pass.cpp

/*  Constant                                       Value
     __cpp_lib_incomplete_container_elements        201505L [C++17]
     __cpp_lib_int_pow2                             202002L [C++20]
     __cpp_lib_integer_comparison_functions         202002L [C++20]
     __cpp_lib_integer_sequence                     201304L [C++14]
     __cpp_lib_integral_constant_callable           201304L [C++14]
     __cpp_lib_interpolate                          201902L [C++20]
     __cpp_lib_invoke                               201411L [C++17]
     __cpp_lib_invoke_r                             202106L [C++2b]
+    __cpp_lib_ios_noreplace                        202207L [C++2b]
     __cpp_lib_is_aggregate                         201703L [C++17]
     __cpp_lib_is_constant_evaluated                201811L [C++20]
     __cpp_lib_is_final                             201402L [C++14]
     __cpp_lib_is_invocable                         201703L [C++17]
     __cpp_lib_is_layout_compatible                 201907L [C++20]
     __cpp_lib_is_nothrow_convertible               201806L [C++20]
     __cpp_lib_is_null_pointer                      201309L [C++14]
     __cpp_lib_is_pointer_interconvertible          201907L [C++20]

 # ifdef __cpp_lib_invoke
 #   error "__cpp_lib_invoke should not be defined before c++17"
 # endif
 
 # ifdef __cpp_lib_invoke_r
 #   error "__cpp_lib_invoke_r should not be defined before c++2b"
 # endif
 
+# ifdef __cpp_lib_ios_noreplace
+#   error "__cpp_lib_ios_noreplace should not be defined before c++2b"
+# endif
+
 # ifdef __cpp_lib_is_aggregate
 #   error "__cpp_lib_is_aggregate should not be defined before c++17"
 # endif
 
 # ifdef __cpp_lib_is_constant_evaluated
 #   error "__cpp_lib_is_constant_evaluated should not be defined before c++20"
 # endif
 

 # ifdef __cpp_lib_invoke
 #   error "__cpp_lib_invoke should not be defined before c++17"
 # endif
 
 # ifdef __cpp_lib_invoke_r
 #   error "__cpp_lib_invoke_r should not be defined before c++2b"
 # endif
 
+# ifdef __cpp_lib_ios_noreplace
+#   error "__cpp_lib_ios_noreplace should not be defined before c++2b"
+# endif
+
 # ifdef __cpp_lib_is_aggregate
 #   error "__cpp_lib_is_aggregate should not be defined before c++17"
 # endif
 
 # ifdef __cpp_lib_is_constant_evaluated
 #   error "__cpp_lib_is_constant_evaluated should not be defined before c++20"
 # endif
 

 # if __cpp_lib_invoke != 201411L
 #   error "__cpp_lib_invoke should have the value 201411L in c++17"
 # endif
 
 # ifdef __cpp_lib_invoke_r
 #   error "__cpp_lib_invoke_r should not be defined before c++2b"
 # endif
 
+# ifdef __cpp_lib_ios_noreplace
+#   error "__cpp_lib_ios_noreplace should not be defined before c++2b"
+# endif
+
 # ifndef __cpp_lib_is_aggregate
 #   error "__cpp_lib_is_aggregate should be defined in c++17"
 # endif
 # if __cpp_lib_is_aggregate != 201703L
 #   error "__cpp_lib_is_aggregate should have the value 201703L in c++17"
 # endif
 
 # ifdef __cpp_lib_is_constant_evaluated

 # if __cpp_lib_invoke != 201411L
 #   error "__cpp_lib_invoke should have the value 201411L in c++20"
 # endif
 
 # ifdef __cpp_lib_invoke_r
 #   error "__cpp_lib_invoke_r should not be defined before c++2b"
 # endif
 
+# ifdef __cpp_lib_ios_noreplace
+#   error "__cpp_lib_ios_noreplace should not be defined before c++2b"
+# endif
+
 # ifndef __cpp_lib_is_aggregate
 #   error "__cpp_lib_is_aggregate should be defined in c++20"
 # endif
 # if __cpp_lib_is_aggregate != 201703L
 #   error "__cpp_lib_is_aggregate should have the value 201703L in c++20"
 # endif
 
 # ifndef __cpp_lib_is_constant_evaluated

 #     error "__cpp_lib_invoke_r should have the value 202106L in c++2b"
 #   endif
 # else // _LIBCPP_VERSION
 #   ifdef __cpp_lib_invoke_r
 #     error "__cpp_lib_invoke_r should not be defined because it is unimplemented in libc++!"
 #   endif
 # endif
 
+# ifndef __cpp_lib_ios_noreplace
+#   error "__cpp_lib_ios_noreplace should be defined in c++2b"
+# endif
+# if __cpp_lib_ios_noreplace != 202207L
+#   error "__cpp_lib_ios_noreplace should have the value 202207L in c++2b"
+# endif
+
 # ifndef __cpp_lib_is_aggregate
 #   error "__cpp_lib_is_aggregate should be defined in c++2b"
 # endif
 # if __cpp_lib_is_aggregate != 201703L
 #   error "__cpp_lib_is_aggregate should have the value 201703L in c++2b"
 # endif
 
 # ifndef __cpp_lib_is_constant_evaluated

libcxx/utils/generate_feature_test_macro_components.py

feature_test_macros = [ add_version_header(x) for x in [
     "values": { "c++17": 201411 },
     "headers": ["functional"],
   }, {
     "name": "__cpp_lib_invoke_r",
     "values": { "c++2b": 202106 },
     "headers": ["functional"],
     "unimplemented": True,
   }, {
+    "name": "__cpp_lib_ios_noreplace",
+    "values": { "c++2b": 202207 },
+    "headers": ["ios"],
+  }, {
     "name": "__cpp_lib_is_aggregate",
     "values": { "c++17": 201703 },
     "headers": ["type_traits"],
   }, {
     "name": "__cpp_lib_is_constant_evaluated",
     "values": { "c++20": 201811 },
     "headers": ["type_traits"],
   }, {