Re: [PATCH] PR libstdc++/83306 make filesystem_error no-throw copyable

[Jonathan Wakely]

On 28/11/18 16:52 +0000, Jonathan Wakely wrote:

On 28/11/18 15:27 +0000, Jonathan Wakely wrote:

The class API provides no way to modify the members, so we can share
them between copies of the same object. Copying becomes a simple
reference count update, which doesn't throw.

Also adjust the what() string to allow distinguishing between an empty
path passed to the constructor, and no path.

        PR libstdc++/83306
        * include/bits/fs_path.h (filesystem_error): Move data members into
        pimpl class owned by shared_ptr. Remove inline definitions of member
        functions.
        * src/filesystem/std-path.cc (filesystem_error::_Impl): Define.
        (filesystem_error): Define member functions.
        * testsuite/27_io/filesystem/filesystem_error/cons.cc: New test.
        * testsuite/27_io/filesystem/filesystem_error/copy.cc: New test.

Tested x86_64-linux, committed to trunk.

Ops, this broke he TS implementation:

FAIL: experimental/filesystem/operations/remove.cc (test for excess errors)
Excess errors:
/home/jwakely/src/gcc/libstdc++-v3/src/filesystem/path.cc:505: undefined reference to 
`std::filesystem::fs_err_concat(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, 
std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, std::__cxx11::basic_string<char, 
std::char_traits<char>, std::allocator<char> > const&)'

Fix coming up ...

Fixed by this patch, committed to trunk.


commit 76f7787e03bac06663a2df66b417cd49679bf847
Author: Jonathan Wakely <jwak...@redhat.com>
Date:   Wed Nov 28 17:05:08 2018 +0000

    Fix undefined references in libstdc++fs.a
    
    The recent patch for PR 83306 removed the fs_err_concat functions that
    were used by the experimental::filesystem::filesystem_error class as
    well. This fixes it by doing the string generation directly in
    filesystem_error::_M_gen_what() instead of using the removed function.
    
            PR libstdc++/83306
            * src/filesystem/path.cc (filesystem_error::_M_gen_what()): Create
            string directly, instead of calling fs_err_concat.

diff --git a/libstdc++-v3/src/filesystem/path.cc b/libstdc++-v3/src/filesystem/path.cc
index fb70d30fdca..63da684cf0a 100644
--- a/libstdc++-v3/src/filesystem/path.cc
+++ b/libstdc++-v3/src/filesystem/path.cc
@@ -485,28 +485,32 @@ fs::hash_value(const path& p) noexcept
   return seed;
 }
 
-namespace std
+#include <experimental/string_view>
+
+std::string
+fs::filesystem_error::_M_gen_what()
 {
-_GLIBCXX_BEGIN_NAMESPACE_VERSION
-namespace filesystem
-{
-  extern string
-  fs_err_concat(const string& __what, const string& __path1,
-		const string& __path2);
-} // namespace filesystem
-
-namespace experimental::filesystem::v1 {
-_GLIBCXX_BEGIN_NAMESPACE_CXX11
-
-  std::string filesystem_error::_M_gen_what()
-  {
-    using std::filesystem::fs_err_concat;
-    return fs_err_concat(system_error::what(), _M_path1.u8string(),
-			 _M_path2.u8string());
-  }
-
-_GLIBCXX_END_NAMESPACE_CXX11
-} // namespace experimental::filesystem::v1
-
-_GLIBCXX_END_NAMESPACE_VERSION
-} // namespace std
+  const std::string pstr1 = _M_path1.u8string();
+  const std::string pstr2 = _M_path2.u8string();
+  experimental::string_view s = this->system_error::what();
+  const size_t len = 18 + s.length()
+    + (pstr1.length() ? pstr1.length() + 3 : 0)
+    + (pstr2.length() ? pstr2.length() + 3 : 0);
+  std::string w;
+  w.reserve(len);
+  w = "filesystem error: ";
+  w.append(s.data(), s.length());
+  if (!pstr1.empty())
+    {
+      w += " [";
+      w += pstr1;
+      w += ']';
+    }
+  if (!pstr1.empty())
+    {
+      w += " [";
+      w += pstr2;
+      w += ']';
+    }
+  return w;
+}