Skip to content

Commit

Permalink
[libc++] Implement LWG3430 disallow implicit conversion of the source…
Browse files Browse the repository at this point in the history
… arguments to `std::filesystem::path` when constructing `std::basic_*fstream` (llvm#85079)

Implement [LWG3430](https://wg21.link/LWG3430).

---------

Signed-off-by: yronglin <[email protected]>
  • Loading branch information
yronglin authored Apr 6, 2024
1 parent 233c030 commit 4761e74
Show file tree
Hide file tree
Showing 7 changed files with 125 additions and 15 deletions.
5 changes: 5 additions & 0 deletions libcxx/docs/ReleaseNotes/19.rst
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,11 @@ Deprecations and Removals
libatomic is not available. If you are one such user, please reach out to the libc++ developers so we can collaborate
on a path for supporting atomics properly on freestanding platforms.

- LWG3430 disallow implicit conversion of the source arguments to ``std::filesystem::path`` when
constructing ``std::basic_*fstream``. This effectively removes the possibility to directly construct
a ``std::basic_*fstream`` from a ``std::basic_string_view``, a input-iterator or a C-string, instead
you can construct a temporary ``std::basic_string``. This change has been applied to C++17 and later.


Upcoming Deprecations and Removals
----------------------------------
Expand Down
2 changes: 1 addition & 1 deletion libcxx/docs/Status/Cxx23Issues.csv
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@
`2818 <https://wg21.link/LWG2818>`__,"``::std::`` everywhere rule needs tweaking","June 2021","|Nothing To Do|",""
`2997 <https://wg21.link/LWG2997>`__,"LWG 491 and the specification of ``{forward_,}list::unique``","June 2021","",""
`3410 <https://wg21.link/LWG3410>`__,"``lexicographical_compare_three_way`` is overspecified","June 2021","|Complete|","17.0","|spaceship|"
`3430 <https://wg21.link/LWG3430>`__,"``std::fstream`` & co. should be constructible from string_view","June 2021","",""
`3430 <https://wg21.link/LWG3430>`__,"``std::fstream`` & co. should be constructible from string_view","June 2021","|Complete|","19.0",""
`3462 <https://wg21.link/LWG3462>`__,"§[formatter.requirements]: Formatter requirements forbid use of ``fc.arg()``","June 2021","|Nothing To Do|","","|format|"
`3481 <https://wg21.link/LWG3481>`__,"``viewable_range`` mishandles lvalue move-only views","June 2021","Superseded by `P2415R2 <https://wg21.link/P2415R2>`__","","|ranges|"
`3506 <https://wg21.link/LWG3506>`__,"Missing allocator-extended constructors for ``priority_queue``","June 2021","|Complete|","14.0"
Expand Down
23 changes: 14 additions & 9 deletions libcxx/include/fstream
Original file line number Diff line number Diff line change
Expand Up @@ -78,8 +78,8 @@ public:
basic_ifstream();
explicit basic_ifstream(const char* s, ios_base::openmode mode = ios_base::in);
explicit basic_ifstream(const string& s, ios_base::openmode mode = ios_base::in);
explicit basic_ifstream(const filesystem::path& p,
ios_base::openmode mode = ios_base::in); // C++17
template<class T>
explicit basic_ifstream(const T& s, ios_base::openmode mode = ios_base::in); // Since C++17
basic_ifstream(basic_ifstream&& rhs);
basic_ifstream& operator=(basic_ifstream&& rhs);
Expand Down Expand Up @@ -117,8 +117,8 @@ public:
basic_ofstream();
explicit basic_ofstream(const char* s, ios_base::openmode mode = ios_base::out);
explicit basic_ofstream(const string& s, ios_base::openmode mode = ios_base::out);
explicit basic_ofstream(const filesystem::path& p,
ios_base::openmode mode = ios_base::out); // C++17
template<class T>
explicit basic_ofstream(const T& s, ios_base::openmode mode = ios_base::out); // Since C++17
basic_ofstream(basic_ofstream&& rhs);
basic_ofstream& operator=(basic_ofstream&& rhs);
Expand Down Expand Up @@ -158,8 +158,8 @@ public:
basic_fstream();
explicit basic_fstream(const char* s, ios_base::openmode mode = ios_base::in|ios_base::out);
explicit basic_fstream(const string& s, ios_base::openmode mode = ios_base::in|ios_base::out);
explicit basic_fstream(const filesystem::path& p,
ios_base::openmode mode = ios_base::in|ios_base::out); C++17
template<class T>
explicit basic_fstream(const T& s, ios_base::openmode mode = ios_base::in | ios_base::out); // Since C++17
basic_fstream(basic_fstream&& rhs);
basic_fstream& operator=(basic_fstream&& rhs);
Expand Down Expand Up @@ -192,6 +192,8 @@ typedef basic_fstream<wchar_t> wfstream;
#include <__config>
#include <__fwd/fstream.h>
#include <__locale>
#include <__type_traits/enable_if.h>
#include <__type_traits/is_same.h>
#include <__utility/move.h>
#include <__utility/swap.h>
#include <__utility/unreachable.h>
Expand Down Expand Up @@ -1101,8 +1103,9 @@ public:
# endif
_LIBCPP_HIDE_FROM_ABI explicit basic_ifstream(const string& __s, ios_base::openmode __mode = ios_base::in);
# if _LIBCPP_STD_VER >= 17
template <class _Tp, class = enable_if_t<is_same_v<_Tp, filesystem::path>>>
_LIBCPP_AVAILABILITY_FILESYSTEM_LIBRARY _LIBCPP_HIDE_FROM_ABI explicit basic_ifstream(
const filesystem::path& __p, ios_base::openmode __mode = ios_base::in)
const _Tp& __p, ios_base::openmode __mode = ios_base::in)
: basic_ifstream(__p.c_str(), __mode) {}
# endif // _LIBCPP_STD_VER >= 17
_LIBCPP_HIDE_FROM_ABI basic_ifstream(basic_ifstream&& __rhs);
Expand Down Expand Up @@ -1255,8 +1258,9 @@ public:
_LIBCPP_HIDE_FROM_ABI explicit basic_ofstream(const string& __s, ios_base::openmode __mode = ios_base::out);

# if _LIBCPP_STD_VER >= 17
template <class _Tp, class = enable_if_t<is_same_v<_Tp, filesystem::path>>>
_LIBCPP_AVAILABILITY_FILESYSTEM_LIBRARY _LIBCPP_HIDE_FROM_ABI explicit basic_ofstream(
const filesystem::path& __p, ios_base::openmode __mode = ios_base::out)
const _Tp& __p, ios_base::openmode __mode = ios_base::out)
: basic_ofstream(__p.c_str(), __mode) {}
# endif // _LIBCPP_STD_VER >= 17

Expand Down Expand Up @@ -1414,8 +1418,9 @@ public:
ios_base::openmode __mode = ios_base::in | ios_base::out);

# if _LIBCPP_STD_VER >= 17
template <class _Tp, class = enable_if_t<is_same_v<_Tp, filesystem::path>>>
_LIBCPP_AVAILABILITY_FILESYSTEM_LIBRARY _LIBCPP_HIDE_FROM_ABI explicit basic_fstream(
const filesystem::path& __p, ios_base::openmode __mode = ios_base::in | ios_base::out)
const _Tp& __p, ios_base::openmode __mode = ios_base::in | ios_base::out)
: basic_fstream(__p.c_str(), __mode) {}
# endif // _LIBCPP_STD_VER >= 17

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,17 +15,50 @@
// 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);
// template<class T>
// explicit basic_fstream(const T& s, ios_base::openmode mode = ios_base::in); // Since C++17
// Constraints: is_same_v<T, filesystem::path> is true

#include <fstream>
#include <filesystem>
#include <cassert>
#include <type_traits>

#include "test_macros.h"
#include "test_iterators.h"
#include "platform_support.h"

namespace fs = std::filesystem;

template <class CharT>
constexpr bool test_non_convert_to_path() {
// String types
static_assert(!std::is_constructible_v<std::fstream, std::basic_string_view<CharT>>);
static_assert(!std::is_constructible_v<std::fstream, const std::basic_string_view<CharT>>);

// Char* pointers
if constexpr (!std::is_same_v<CharT, char>)
static_assert(!std::is_constructible_v<std::fstream, const CharT*>);

// Iterators
static_assert(!std::is_convertible_v<std::fstream, cpp17_input_iterator<const CharT*>>);

return true;
}

static_assert(test_non_convert_to_path<char>());

#if !defined(TEST_HAS_NO_WIDE_CHARACTERS) && !defined(TEST_HAS_OPEN_WITH_WCHAR)
static_assert(test_non_convert_to_path<wchar_t>());
#endif // !TEST_HAS_NO_WIDE_CHARACTERS && !TEST_HAS_OPEN_WITH_WCHAR

#ifndef TEST_HAS_NO_CHAR8_T
static_assert(test_non_convert_to_path<char8_t>());
#endif // TEST_HAS_NO_CHAR8_T

static_assert(test_non_convert_to_path<char16_t>());
static_assert(test_non_convert_to_path<char32_t>());

int main(int, char**) {
fs::path p = get_temp_file_name();
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,18 +17,49 @@
// template <class charT, class traits = char_traits<charT> >
// class basic_ifstream

// explicit basic_ifstream(const filesystem::path& s,
// ios_base::openmode mode = ios_base::in);
// template<class T>
// explicit basic_ifstream(const T& s, ios_base::openmode mode = ios_base::in); // Since C++17
// Constraints: is_same_v<T, filesystem::path> is true

#include <cassert>
#include <filesystem>
#include <fstream>
#include <type_traits>

#include "test_macros.h"
#include "test_iterators.h"

namespace fs = std::filesystem;

template <class CharT>
constexpr bool test_non_convert_to_path() {
// String types
static_assert(!std::is_constructible_v<std::ifstream, std::basic_string_view<CharT>>);
static_assert(!std::is_constructible_v<std::ifstream, const std::basic_string_view<CharT>>);

// Char* pointers
if constexpr (!std::is_same_v<CharT, char>)
static_assert(!std::is_constructible_v<std::ifstream, const CharT*>);

// Iterators
static_assert(!std::is_convertible_v<std::ifstream, cpp17_input_iterator<const CharT*>>);

return true;
}

static_assert(test_non_convert_to_path<char>());

#if !defined(TEST_HAS_NO_WIDE_CHARACTERS) && !defined(TEST_HAS_OPEN_WITH_WCHAR)
static_assert(test_non_convert_to_path<wchar_t>());
#endif // !TEST_HAS_NO_WIDE_CHARACTERS && !TEST_HAS_OPEN_WITH_WCHAR

#ifndef TEST_HAS_NO_CHAR8_T
static_assert(test_non_convert_to_path<char8_t>());
#endif // TEST_HAS_NO_CHAR8_T

static_assert(test_non_convert_to_path<char16_t>());
static_assert(test_non_convert_to_path<char32_t>());

int main(int, char**) {
{
fs::path p;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,9 @@
// plate <class charT, class traits = char_traits<charT> >
// class basic_ofstream

// explicit basic_ofstream(const filesystem::path& s, ios_base::openmode mode = ios_base::out);
// template<class T>
// explicit basic_ifstream(const T& s, ios_base::openmode mode = ios_base::in); // Since C++17
// Constraints: is_same_v<T, filesystem::path> is true

#include <cassert>
#include <filesystem>
Expand All @@ -24,9 +26,39 @@

#include "platform_support.h"
#include "test_macros.h"
#include "test_iterators.h"

namespace fs = std::filesystem;

template <class CharT>
constexpr bool test_non_convert_to_path() {
// String types
static_assert(!std::is_constructible_v<std::ofstream, std::basic_string_view<CharT>>);
static_assert(!std::is_constructible_v<std::ofstream, const std::basic_string_view<CharT>>);

// Char* pointers
if constexpr (!std::is_same_v<CharT, char>)
static_assert(!std::is_constructible_v<std::ofstream, const CharT*>);

// Iterators
static_assert(!std::is_convertible_v<std::ofstream, cpp17_input_iterator<const CharT*>>);

return true;
}

static_assert(test_non_convert_to_path<char>());

#if !defined(TEST_HAS_NO_WIDE_CHARACTERS) && !defined(TEST_HAS_OPEN_WITH_WCHAR)
static_assert(test_non_convert_to_path<wchar_t>());
#endif // !TEST_HAS_NO_WIDE_CHARACTERS && !TEST_HAS_OPEN_WITH_WCHAR

#ifndef TEST_HAS_NO_CHAR8_T
static_assert(test_non_convert_to_path<char8_t>());
#endif // TEST_HAS_NO_CHAR8_T

static_assert(test_non_convert_to_path<char16_t>());
static_assert(test_non_convert_to_path<char32_t>());

int main(int, char**) {
fs::path p = get_temp_file_name();
{
Expand Down
4 changes: 4 additions & 0 deletions libcxx/test/support/test_macros.h
Original file line number Diff line number Diff line change
Expand Up @@ -385,6 +385,10 @@ inline Tp const& DoNotOptimize(Tp const& value) {
# define TEST_HAS_NO_UNICODE
#endif

#if defined(_LIBCPP_HAS_OPEN_WITH_WCHAR)
# define TEST_HAS_OPEN_WITH_WCHAR
#endif

#if defined(_LIBCPP_HAS_NO_INT128) || defined(_MSVC_STL_VERSION)
# define TEST_HAS_NO_INT128
#endif
Expand Down

0 comments on commit 4761e74

Please sign in to comment.