[libcxx-commits] [libcxx] [libc++] Fix insufficiently guarded call to `copy_file_impl_fstream` (PR #214706)

[Louis Dionne via libcxx-commits]

================
@@ -362,7 +362,13 @@ bool copy_file_impl(FileDescriptor& read_fd, FileDescriptor& write_fd, error_cod
 }
 #elif defined(_LIBCPP_FILESYSTEM_USE_FSTREAM)
 bool copy_file_impl(FileDescriptor& read_fd, FileDescriptor& write_fd, error_code& ec) {
+#  if defined(_LIBCPP_FILESYSTEM_NEED_FSTREAM)
   return copy_file_impl_fstream(read_fd, write_fd, ec);
+#  else
+  // iostreams are unavailable in the no-locale build, just fail.
----------------
ldionne wrote:

This is tricky. We do something like that in the other code paths (where we use copy_file_range or sendfile), but the *key* difference is that we actually try something else before we error out due to not being able to fall back on `fstream` due to the lack of localization support.

In this patch, we'd end up unconditionally erroring out. This basically makes `std::filesystem::copy_file` a landmine. I don't think we can do that. It seems to me that the code path we instead want to take is `#  error "Unknown implementation for copy_file_impl"`.

Note to self: there is both `_LIBCPP_FILESYSTEM_NEED_FSTREAM` and `_LIBCPP_FILESYSTEM_USE_FSTREAM`, and the distinction between them is essential to this patch. That caught me offguard in a previous (now deleted) review of this patch.

https://github.com/llvm/llvm-project/pull/214706