From: Jonathan Wakely Date: Tue, 15 May 2018 12:07:09 +0000 (+0100) Subject: PR libstdc++/84159 fix appending strings to paths X-Git-Url: https://git.libre-soc.org/?a=commitdiff_plain;h=6cda876da273e36bb65f843a9bf39576258ebf19;p=gcc.git PR libstdc++/84159 fix appending strings to paths The path::operator/=(const Source&) and path::append overloads were still following the semantics of the Filesystem TS not C++17. Only the path::operator/=(const path&) overload was correct. This change adds more tests for path::operator/=(const path&) and adds new tests to verify that the other append operations have equivalent behaviour. PR libstdc++/84159 * include/bits/fs_path.h (path::operator/=, path::append): Construct temporary path before calling _M_append. (path::_M_append): Change parameter to path and implement C++17 semantics. * testsuite/27_io/filesystem/path/append/path.cc: Add helper function and more examples from the standard. * testsuite/27_io/filesystem/path/append/source.cc: New. * testsuite/27_io/filesystem/path/decompose/filename.cc: Add comment. * testsuite/27_io/filesystem/path/nonmember/append.cc: New. From-SVN: r260255 --- diff --git a/libstdc++-v3/ChangeLog b/libstdc++-v3/ChangeLog index 22bcc0ffe1b..22eeaa3da72 100644 --- a/libstdc++-v3/ChangeLog +++ b/libstdc++-v3/ChangeLog @@ -1,5 +1,16 @@ 2018-05-15 Jonathan Wakely + PR libstdc++/84159 + * include/bits/fs_path.h (path::operator/=, path::append): Construct + temporary path before calling _M_append. + (path::_M_append): Change parameter to path and implement C++17 + semantics. + * testsuite/27_io/filesystem/path/append/path.cc: Add helper function + and more examples from the standard. + * testsuite/27_io/filesystem/path/append/source.cc: New. + * testsuite/27_io/filesystem/path/decompose/filename.cc: Add comment. + * testsuite/27_io/filesystem/path/nonmember/append.cc: New. + * include/std/variant (__gen_vtable_impl::__visit_invoke): Qualify __invoke to prevent ADL. diff --git a/libstdc++-v3/include/bits/fs_path.h b/libstdc++-v3/include/bits/fs_path.h index 6703e9167e4..53bf237b547 100644 --- a/libstdc++-v3/include/bits/fs_path.h +++ b/libstdc++-v3/include/bits/fs_path.h @@ -265,20 +265,17 @@ _GLIBCXX_BEGIN_NAMESPACE_CXX11 template _Path<_Source>& operator/=(_Source const& __source) - { return append(__source); } + { return _M_append(path(__source)); } template _Path<_Source>& append(_Source const& __source) - { - return _M_append(_S_convert(_S_range_begin(__source), - _S_range_end(__source))); - } + { return _M_append(path(__source)); } template _Path<_InputIterator, _InputIterator>& append(_InputIterator __first, _InputIterator __last) - { return _M_append(_S_convert(__first, __last)); } + { return _M_append(path(__first, __last)); } // concatenation @@ -406,17 +403,17 @@ _GLIBCXX_BEGIN_NAMESPACE_CXX11 enum class _Split { _Stem, _Extension }; - path& _M_append(string_type&& __str) + path& + _M_append(path __p) { + if (__p.is_absolute()) + operator=(std::move(__p)); #ifdef _GLIBCXX_FILESYSTEM_IS_WINDOWS - operator/=(path(std::move(__str))); -#else - if (!_M_pathname.empty() && !_S_is_dir_sep(_M_pathname.back()) - && (__str.empty() || !_S_is_dir_sep(__str.front()))) - _M_pathname += preferred_separator; - _M_pathname += __str; - _M_split_cmpts(); + else if (__p.has_root_name() && __p.root_name() != root_name()) + operator=(std::move(__p)); #endif + else + operator/=(const_cast(__p)); return *this; } diff --git a/libstdc++-v3/testsuite/27_io/filesystem/path/append/path.cc b/libstdc++-v3/testsuite/27_io/filesystem/path/append/path.cc index 3bc135c6cd2..0330bcf6c88 100644 --- a/libstdc++-v3/testsuite/27_io/filesystem/path/append/path.cc +++ b/libstdc++-v3/testsuite/27_io/filesystem/path/append/path.cc @@ -19,7 +19,7 @@ // with this library; see the file COPYING3. If not see // . -// 30.10.7.4.3 path appends [fs.path.append] +// C++17 30.10.8.4.3 path appends [fs.path.append] #include #include @@ -28,56 +28,75 @@ using std::filesystem::path; using __gnu_test::compare_paths; +// path::operator/=(const path&) + +path append(path l, const path& r) +{ + l /= r; + return l; +} + void test01() { - const path p("/foo/bar"); - - path pp = p; - pp /= p; - compare_paths( pp, p ); + compare_paths( append("/foo/bar", "/foo/"), "/foo/" ); + +#ifndef _GLIBCXX_FILESYSTEM_IS_WINDOWS + compare_paths( append("baz", "baz"), "baz/baz" ); +#else + compare_paths( append("baz", "baz"), "baz\\baz" ); +#endif + compare_paths( append("baz/", "baz"), "baz/baz" ); + compare_paths( append("baz", "/foo/bar"), "/foo/bar" ); + compare_paths( append("baz/", "/foo/bar"), "/foo/bar" ); + + VERIFY( append("", "").empty() ); + VERIFY( !append("", "rel").is_absolute() ); + + compare_paths( append("dir/", "/file"), "/file" ); + compare_paths( append("dir/", "file"), "dir/file" ); +} - path q("baz"); +void +test02() +{ + // C++17 [fs.path.append] p4 +#ifndef _GLIBCXX_FILESYSTEM_IS_WINDOWS + compare_paths( append("//host", "foo"), "//host/foo" ); - path qq = q; - qq /= q; - compare_paths( qq, "baz/baz" ); + compare_paths( append("//host/", "foo"), "//host/foo" ); - q /= p; - compare_paths( q, p ); + // path("foo") / ""; // yields "foo/" + compare_paths( append("foo", ""), "foo/" ); - path r = ""; - r /= path(); - VERIFY( r.empty() ); + // path("foo") / "/bar"; // yields "/bar" + compare_paths( append("foo", "/bar"), "/bar" ); +#else + compare_paths( append("//host", "foo"), "//host\\foo" ); - r /= path("rel"); - VERIFY( !r.is_absolute() ); + compare_paths( append("//host/", "foo"), "//host/foo" ); - path s = "dir/"; - s /= path("/file"); - compare_paths( s, "/file" ); + // path("foo") / ""; // yields "foo/" + compare_paths( append("foo", ""), "foo\\" ); - s = "dir/"; - s /= path("file"); - compare_paths( s, "dir/file" ); -} + // path("foo") / "/bar"; // yields "/bar" + compare_paths( append("foo", "/bar"), "/bar" ); -void -test02() -{ - // C++17 [fs.path.append] p4 + // path("foo") / "c:/bar"; // yields "c:/bar" + compare_paths( append("foo", "c:/bar"), "c:/bar" ); - path p = path("//host") / "foo"; - compare_paths( p, "//host/foo" ); + // path("foo") / "c:"; // yields "c:" + compare_paths( append("foo", "c:"), "c:" ); - path pp = path("//host/") / "foo"; - compare_paths( pp, "//host/foo" ); + // path("c:") / ""; // yields "c:" + compare_paths( append("c:", ""), "c:" ); - path q = path("foo") / ""; - compare_paths( q, "foo/" ); + // path("c:foo") / "/bar"; // yields "c:/bar" + compare_paths( append("c:foo", "/bar"), "c:/bar" ); - path qq = path("foo") / "/bar"; - compare_paths( qq, "/bar" ); + // path("c:foo") / "c:bar"; // yields "c:foo/bar" + compare_paths( append("foo", "c:\\bar"), "c:\\bar" ); +#endif } int diff --git a/libstdc++-v3/testsuite/27_io/filesystem/path/append/source.cc b/libstdc++-v3/testsuite/27_io/filesystem/path/append/source.cc new file mode 100644 index 00000000000..316d6313b0a --- /dev/null +++ b/libstdc++-v3/testsuite/27_io/filesystem/path/append/source.cc @@ -0,0 +1,106 @@ +// { dg-options "-std=gnu++17 -lstdc++fs" } +// { dg-do run { target c++17 } } +// { dg-require-filesystem-ts "" } + +// Copyright (C) 2018 Free Software Foundation, Inc. +// +// This file is part of the GNU ISO C++ Library. This library is free +// software; you can redistribute it and/or modify it under the +// terms of the GNU General Public License as published by the +// Free Software Foundation; either version 3, or (at your option) +// any later version. + +// This library is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU General Public License for more details. + +// You should have received a copy of the GNU General Public License along +// with this library; see the file COPYING3. If not see +// . + +// C++17 30.10.8.4.3 path appends [fs.path.append] + +#include +#include +#include +#include + +using std::filesystem::path; +using __gnu_test::compare_paths; + +// path::operator/=(const Source& source) +// path::append(const Source& source) +// Equivalent to: return operator/=(path(source)); + +// path::append(InputIterator first, InputIterator last) +// Equivalent to: return operator/=(path(first, last)); + +void test(const path& p, std::string_view s) +{ + path expected = p; + expected /= path(s); + + path oper = p; + oper /= s; + + path func = p; + func.append(s); + + __gnu_test::test_container + input_range(s.begin(), s.end()); + path range = p; + range.append(input_range.begin(), input_range.end()); + + compare_paths( oper, expected ); + compare_paths( func, expected ); + compare_paths( range, expected ); +} + +void +test01() +{ + test( "/foo/bar", "/foo/" ); + + test( "baz", "baz" ); + test( "baz/", "baz" ); + test( "baz", "/foo/bar" ); + test( "baz/", "/foo/bar" ); + + test( "", "" ); + test( "", "rel" ); + + test( "dir/", "/file" ); + test( "dir/", "file" ); +} + +void +test02() +{ + // C++17 [fs.path.append] p4 + test( "//host", "foo" ); + test( "//host/", "foo" ); + test( "foo", "" ); + test( "foo", "/bar" ); + test( "foo", "c:/bar" ); + test( "foo", "c:" ); + test( "c:", "" ); + test( "c:foo", "/bar" ); + test( "foo", "c:\\bar" ); +} + +void +test03() +{ + for (const path& p : __gnu_test::test_paths) + for (const path& q : __gnu_test::test_paths) + test(p, q.native()); +} + +int +main() +{ + test01(); + test02(); + test03(); +} diff --git a/libstdc++-v3/testsuite/27_io/filesystem/path/decompose/filename.cc b/libstdc++-v3/testsuite/27_io/filesystem/path/decompose/filename.cc index ffabe5388b1..7b4549617dd 100644 --- a/libstdc++-v3/testsuite/27_io/filesystem/path/decompose/filename.cc +++ b/libstdc++-v3/testsuite/27_io/filesystem/path/decompose/filename.cc @@ -30,6 +30,7 @@ using std::filesystem::path; void test01() { + // [fs.path.decompose] p7 VERIFY( path("/foo/bar.txt").filename() == "bar.txt" ); VERIFY( path("/foo/bar").filename() == "bar" ); VERIFY( path("/foo/bar/").filename() == "" ); diff --git a/libstdc++-v3/testsuite/27_io/filesystem/path/nonmember/append.cc b/libstdc++-v3/testsuite/27_io/filesystem/path/nonmember/append.cc new file mode 100644 index 00000000000..2fbb9c246db --- /dev/null +++ b/libstdc++-v3/testsuite/27_io/filesystem/path/nonmember/append.cc @@ -0,0 +1,84 @@ +// { dg-options "-std=gnu++17 -lstdc++fs" } +// { dg-do run { target c++17 } } +// { dg-require-filesystem-ts "" } + +// Copyright (C) 2018 Free Software Foundation, Inc. +// +// This file is part of the GNU ISO C++ Library. This library is free +// software; you can redistribute it and/or modify it under the +// terms of the GNU General Public License as published by the +// Free Software Foundation; either version 3, or (at your option) +// any later version. + +// This library is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU General Public License for more details. + +// You should have received a copy of the GNU General Public License along +// with this library; see the file COPYING3. If not see +// . + +// C++17 30.10.8.6 path non-member functions [fs.path.nonmember] + +#include +#include + +using std::filesystem::path; +using __gnu_test::compare_paths; + +// operator/(const path&, const path&) +// Equivalent to: return path(lhs) /= rhs; + +void test(const path& lhs, const path& rhs) +{ + compare_paths( lhs / rhs, path(lhs) /= rhs ); +} + +void +test01() +{ + test( "/foo/bar", "/foo/" ); + + test( "baz", "baz" ); + test( "baz/", "baz" ); + test( "baz", "/foo/bar" ); + test( "baz/", "/foo/bar" ); + + test( "", "" ); + test( "", "rel" ); + + test( "dir/", "/file" ); + test( "dir/", "file" ); +} + +void +test02() +{ + // C++17 [fs.path.append] p4 + test( "//host", "foo" ); + test( "//host/", "foo" ); + test( "foo", "" ); + test( "foo", "/bar" ); + test( "foo", "c:/bar" ); + test( "foo", "c:" ); + test( "c:", "" ); + test( "c:foo", "/bar" ); + test( "foo", "c:\\bar" ); +} + +void +test03() +{ + for (const path& p : __gnu_test::test_paths) + for (const path& q : __gnu_test::test_paths) + test(p, q); +} + +int +main() +{ + test01(); + test02(); + test03(); +}