PR libstdc++/84159 fix appending strings to paths

Message ID 20180515120648.GA27506@redhat.com
State New
Headers show
Series
  • PR libstdc++/84159 fix appending strings to paths
Related show

Commit Message

Jonathan Wakely May 15, 2018, 12:06 p.m.
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.

Tested powerpc64le-linux, committed to trunk and will backport to
gcc-8-branch too.
commit 0dbff221ca95e7fa4260fded79373c36b90019de
Author: Jonathan Wakely <jwakely@redhat.com>
Date:   Tue May 15 01:09:20 2018 +0100

    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.

Patch

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 <class _Source>
       _Path<_Source>&
       operator/=(_Source const& __source)
-      { return append(__source); }
+      { return _M_append(path(__source)); }
 
     template<typename _Source>
       _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<typename _InputIterator>
       _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<const path&>(__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
 // <http://www.gnu.org/licenses/>.
 
-// 30.10.7.4.3 path appends [fs.path.append]
+// C++17 30.10.8.4.3 path appends [fs.path.append]
 
 #include <filesystem>
 #include <testsuite_hooks.h>
@@ -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");
+  compare_paths( append("/foo/bar", "/foo/"), "/foo/" );
 
-  path pp = p;
-  pp /= p;
-  compare_paths( pp, p );
+#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" );
 
-  path q("baz");
+  VERIFY( append("", "").empty() );
+  VERIFY( !append("", "rel").is_absolute() );
 
-  path qq = q;
-  qq /= q;
-  compare_paths( qq, "baz/baz" );
-
-  q /= p;
-  compare_paths( q, p );
-
-  path r = "";
-  r /= path();
-  VERIFY( r.empty() );
-
-  r /= path("rel");
-  VERIFY( !r.is_absolute() );
-
-  path s = "dir/";
-  s /= path("/file");
-  compare_paths( s, "/file" );
-
-  s = "dir/";
-  s /= path("file");
-  compare_paths( s, "dir/file" );
+  compare_paths( append("dir/", "/file"), "/file" );
+  compare_paths( append("dir/", "file"),  "dir/file" );
 }
 
 void
 test02()
 {
   // C++17 [fs.path.append] p4
+#ifndef _GLIBCXX_FILESYSTEM_IS_WINDOWS
+  compare_paths( append("//host", "foo"),  "//host/foo" );
 
-  path p = path("//host") / "foo";
-  compare_paths( p, "//host/foo" );
+  compare_paths( append("//host/", "foo"),  "//host/foo" );
 
-  path pp = path("//host/") / "foo";
-  compare_paths( pp, "//host/foo" );
+  // path("foo") / ""; // yields "foo/"
+  compare_paths( append("foo", ""), "foo/" );
 
-  path q = path("foo") / "";
-  compare_paths( q, "foo/" );
+  // path("foo") / "/bar"; // yields "/bar"
+  compare_paths( append("foo", "/bar"), "/bar" );
+#else
+  compare_paths( append("//host", "foo"),  "//host\\foo" );
 
-  path qq = path("foo") / "/bar";
-  compare_paths( qq, "/bar" );
+  compare_paths( append("//host/", "foo"), "//host/foo" );
+
+  // path("foo") / ""; // yields "foo/"
+  compare_paths( append("foo", ""), "foo\\" );
+
+  // path("foo") / "/bar"; // yields "/bar"
+  compare_paths( append("foo", "/bar"),  "/bar" );
+
+  // path("foo") / "c:/bar"; // yields "c:/bar"
+  compare_paths( append("foo", "c:/bar"),  "c:/bar" );
+
+  // path("foo") / "c:"; // yields "c:"
+  compare_paths( append("foo", "c:"),  "c:" );
+
+  // path("c:") / ""; // yields "c:"
+  compare_paths( append("c:", ""),  "c:" );
+
+  // path("c:foo") / "/bar"; // yields "c:/bar"
+  compare_paths( append("c:foo", "/bar"),  "c:/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
+// <http://www.gnu.org/licenses/>.
+
+// C++17 30.10.8.4.3 path appends [fs.path.append]
+
+#include <filesystem>
+#include <string_view>
+#include <testsuite_fs.h>
+#include <testsuite_iterators.h>
+
+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<const char, __gnu_test::input_iterator_wrapper>
+    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
+// <http://www.gnu.org/licenses/>.
+
+// C++17 30.10.8.6 path non-member functions [fs.path.nonmember]
+
+#include <filesystem>
+#include <testsuite_fs.h>
+
+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();
+}