public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [patch] Implement ISO/IEC TS 18822 C++ File system TS
@ 2015-04-30 17:42 Jonathan Wakely
  2015-04-30 19:33 ` Jonathan Wakely
                   ` (2 more replies)
  0 siblings, 3 replies; 15+ messages in thread
From: Jonathan Wakely @ 2015-04-30 17:42 UTC (permalink / raw)
  To: libstdc++, gcc-patches

[-- Attachment #1: Type: text/plain, Size: 1743 bytes --]

This is the complete <experimental/filesystem> implementation I intend
to commit shortly. (It's also been pushed to the redi/filesystem-ts
branch in the git mirror).

As before, only a static library (libstdc++fs.a) is built, so there
are no symbols added to libstdc++.so and we can be a bit more risky
with regards to maintaining a stable ABI for this stuff.

Since the last updates to the branch I added the <codecvt> header,
which meant I could add the missing path conversion features to
filesystem. I've also put everything relevant in a nested __cxx11
namespace, to avoid problems with the dual std::string ABI.

At this time the objects in libstdc++fa.a are all built with the
default std::string ABI (as set by configure). It will probably be
necessary to compile all of src/filesystem/*.cc twice when the dual
ABI is enabled, so both sets of definitions go in the archive.

I've tested this on GNU/Linux and DragonFly BSD, but as it's probably
not going to build everywhere I've added the configure option
--enable-libstdcxx-filesystem-ts which defaults to enabled on GNU, BSD
and Solaris targets, and disabled elsewhere for now. If it fails to
build on any of those targets we can change the default while we fix
the problem.

There are still quite a few operations (see src/filesystem/ops.cc)
without proper implementations, which might need to use the Win32 API.
I've compiled the _GLIBCXX_FILESYSTEM_IS_WINDOWS code by overriding
the macro, but not tested it any more than that. Someone will have to
try using --enable-libstdcxx-filesystem-ts on mingw.org, mingw-w64 and
cygwin to see what happens there. It's possible that macro could be
replaced by the existing _GLIBCXX_HAVE_DOS_BASED_FILESYSTEM macro, but
I'm not sure yet.



[-- Attachment #2: patch.txt.bz2 --]
[-- Type: application/x-bzip2, Size: 29938 bytes --]

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [patch] Implement ISO/IEC TS 18822 C++ File system TS
  2015-04-30 17:42 [patch] Implement ISO/IEC TS 18822 C++ File system TS Jonathan Wakely
@ 2015-04-30 19:33 ` Jonathan Wakely
       [not found]   ` <CAFW6PZB9WvskzrKCsNKK4s-5oXWjcwkuZGMrWbrfgJ7fpJiCXQ@mail.gmail.com>
  2015-05-01 12:45 ` Rainer Orth
  2015-05-01 17:03 ` Daniel Krügler
  2 siblings, 1 reply; 15+ messages in thread
From: Jonathan Wakely @ 2015-04-30 19:33 UTC (permalink / raw)
  To: libstdc++, gcc-patches

On 30/04/15 18:32 +0100, Jonathan Wakely wrote:
>This is the complete <experimental/filesystem> implementation I intend
>to commit shortly. (It's also been pushed to the redi/filesystem-ts
>branch in the git mirror).

Committed to trunk at r222654.


    Implement N4100 File System TS
    
    	* acinclude.m4 (GLIBCXX_ENABLE_FILESYSTEM_TS): Define.
    	(GLIBCXX_CHECK_FILESYSTEM_DEPS): Define.
    	* config.h.in: Regenerate.
    	* configure: Regenerate.
    	* configure.ac: Enable filesystem TS and check its dependencies.
    	* include/Makefile.am: Add new headers.
    	* include/Makefile.in: Regenerate.
    	* include/bits/locale_conv.h (__do_str_code_cvt, __str_codecvt_in,
    	__str_codecvt_out): Move code conversion logic from wstring_convert
    	into new global functions.
    	(wstring_convert::to_bytes, wstring_convert::from_bytes): Use new
    	functions.
    	(wstring_convert::_M_conv): Remove.
    	* include/bits/quoted_string.h (_Quoted_string): Split out of iomanip.
    	* include/experimental/filesystem: New.
    	* include/experimental/fs_dir.h: New.
    	* include/experimental/fs_fwd.h: New.
    	* include/experimental/fs_ops.h: New.
    	* include/experimental/fs_path.h: New.
    	* include/std/iomanip (_Quoted_string): Move to bits/quoted_string.h.
    	* python/libstdcxx/v6/printers.py (StdExpPathPrinter): Add.
    	* src/Makefile.am (SUBDIRS): Add filesystem.
    	* src/Makefile.in: Regenerate.
    	* src/filesystem/Makefile.am: New.
    	* src/filesystem/Makefile.in: New.
    	* src/filesystem/dir.cc: New.
    	* src/filesystem/ops.cc: New.
    	* src/filesystem/path.cc: New.
    	* testsuite/experimental/filesystem/operations/absolute.cc: New.
    	* testsuite/experimental/filesystem/operations/copy.cc: New.
    	* testsuite/experimental/filesystem/operations/current_path.cc: New.
    	* testsuite/experimental/filesystem/path/append/path.cc: New.
    	* testsuite/experimental/filesystem/path/assign/assign.cc: New.
    	* testsuite/experimental/filesystem/path/assign/copy.cc: New.
    	* testsuite/experimental/filesystem/path/compare/compare.cc: New.
    	* testsuite/experimental/filesystem/path/compare/path.cc: New.
    	* testsuite/experimental/filesystem/path/compare/strings.cc: New.
    	* testsuite/experimental/filesystem/path/concat/path.cc: New.
    	* testsuite/experimental/filesystem/path/concat/strings.cc: New.
    	* testsuite/experimental/filesystem/path/construct/copy.cc: New.
    	* testsuite/experimental/filesystem/path/construct/default.cc: New.
    	* testsuite/experimental/filesystem/path/construct/locale.cc: New.
    	* testsuite/experimental/filesystem/path/construct/range.cc: New.
    	* testsuite/experimental/filesystem/path/decompose/extension.cc: New.
    	* testsuite/experimental/filesystem/path/decompose/filename.cc: New.
    	* testsuite/experimental/filesystem/path/decompose/parent_path.cc:
    	New.
    	* testsuite/experimental/filesystem/path/decompose/relative_path.cc:
    	New.
    	* testsuite/experimental/filesystem/path/decompose/root_directory.cc:
    	New.
    	* testsuite/experimental/filesystem/path/decompose/root_name.cc:
    	New.
    	* testsuite/experimental/filesystem/path/decompose/root_path.cc:
    	New.
    	* testsuite/experimental/filesystem/path/decompose/stem.cc: New.
    	* testsuite/experimental/filesystem/path/generic/generic_string.cc:
    	New.
    	* testsuite/experimental/filesystem/path/itr/traversal.cc: New.
    	* testsuite/experimental/filesystem/path/modifiers/clear.cc: New.
    	* testsuite/experimental/filesystem/path/modifiers/make_preferred.cc:
    	New.
    	* testsuite/experimental/filesystem/path/modifiers/remove_filename.cc:
    	New.
    	* testsuite/experimental/filesystem/path/modifiers/replace_extension.cc:
    	New.
    	* testsuite/experimental/filesystem/path/modifiers/replace_filename.cc:
    	New.
    	* testsuite/experimental/filesystem/path/modifiers/swap.cc: New.
    	* testsuite/experimental/filesystem/path/nonmember/hash_value.cc: New.
    	* testsuite/experimental/filesystem/path/query/empty.cc: New.
    	* testsuite/experimental/filesystem/path/query/has_extension.cc: New.
    	* testsuite/experimental/filesystem/path/query/has_filename.cc: New.
    	* testsuite/experimental/filesystem/path/query/has_parent_path.cc:
    	New.
    	* testsuite/experimental/filesystem/path/query/has_relative_path.cc:
    	New.
    	* testsuite/experimental/filesystem/path/query/has_root_directory.cc:
    	New.
    	* testsuite/experimental/filesystem/path/query/has_root_name.cc:
    	New.
    	* testsuite/experimental/filesystem/path/query/has_root_path.cc:
    	New.
    	* testsuite/experimental/filesystem/path/query/has_stem.cc: New.
    	* testsuite/experimental/filesystem/path/query/is_relative.cc: New.
    	* testsuite/util/testsuite_fs.h: New.
    

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [patch] Implement ISO/IEC TS 18822 C++ File system TS
  2015-04-30 17:42 [patch] Implement ISO/IEC TS 18822 C++ File system TS Jonathan Wakely
  2015-04-30 19:33 ` Jonathan Wakely
@ 2015-05-01 12:45 ` Rainer Orth
  2015-05-01 15:11   ` Jonathan Wakely
  2015-05-01 17:03 ` Daniel Krügler
  2 siblings, 1 reply; 15+ messages in thread
From: Rainer Orth @ 2015-05-01 12:45 UTC (permalink / raw)
  To: Jonathan Wakely; +Cc: libstdc++, gcc-patches

Jonathan Wakely <jwakely@redhat.com> writes:

> I've tested this on GNU/Linux and DragonFly BSD, but as it's probably
> not going to build everywhere I've added the configure option
> --enable-libstdcxx-filesystem-ts which defaults to enabled on GNU, BSD
> and Solaris targets, and disabled elsewhere for now. If it fails to
> build on any of those targets we can change the default while we fix
> the problem.

Unfortunately, the patch breaks Solaris 10 bootstrap, which lacks
fchmodat:

/vol/gcc/src/hg/trunk/local/libstdc++-v3/src/filesystem/ops.cc: In function 'void std::experimental::filesystem::v1::permissions(const std::experimental::filesystem::v1::__cxx11::path&, std::experimental::filesystem::v1::perms, std::error_code&)':
/vol/gcc/src/hg/trunk/local/libstdc++-v3/src/filesystem/ops.cc:890:17: error: '::fchmodat' has not been declared
   if (int err = ::fchmodat(AT_FDCWD, p.c_str(), static_cast<mode_t>(prms), 0))
                 ^
make[6]: *** [ops.lo] Error 1

	Rainer

-- 
-----------------------------------------------------------------------------
Rainer Orth, Center for Biotechnology, Bielefeld University

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [patch] Implement ISO/IEC TS 18822 C++ File system TS
  2015-05-01 12:45 ` Rainer Orth
@ 2015-05-01 15:11   ` Jonathan Wakely
  2015-05-01 15:14     ` Rainer Orth
  0 siblings, 1 reply; 15+ messages in thread
From: Jonathan Wakely @ 2015-05-01 15:11 UTC (permalink / raw)
  To: Rainer Orth; +Cc: libstdc++, gcc-patches

[-- Attachment #1: Type: text/plain, Size: 1154 bytes --]

On 01/05/15 14:45 +0200, Rainer Orth wrote:
>Jonathan Wakely <jwakely@redhat.com> writes:
>
>> I've tested this on GNU/Linux and DragonFly BSD, but as it's probably
>> not going to build everywhere I've added the configure option
>> --enable-libstdcxx-filesystem-ts which defaults to enabled on GNU, BSD
>> and Solaris targets, and disabled elsewhere for now. If it fails to
>> build on any of those targets we can change the default while we fix
>> the problem.
>
>Unfortunately, the patch breaks Solaris 10 bootstrap, which lacks
>fchmodat:
>
>/vol/gcc/src/hg/trunk/local/libstdc++-v3/src/filesystem/ops.cc: In function 'void std::experimental::filesystem::v1::permissions(const std::experimental::filesystem::v1::__cxx11::path&, std::experimental::filesystem::v1::perms, std::error_code&)':
>/vol/gcc/src/hg/trunk/local/libstdc++-v3/src/filesystem/ops.cc:890:17: error: '::fchmodat' has not been declared
>   if (int err = ::fchmodat(AT_FDCWD, p.c_str(), static_cast<mode_t>(prms), 0))
>                 ^
>make[6]: *** [ops.lo] Error 1

Sorry about that, I'll add a check for fchmodat, in the meantime this
will fix bootstrap. Committed as obvious.


[-- Attachment #2: patch.txt --]
[-- Type: text/x-patch, Size: 1157 bytes --]

commit f4768ebcfd68e2fa6e4763d0b681e8fe710c64c4
Author: Jonathan Wakely <jwakely@redhat.com>
Date:   Fri May 1 16:09:28 2015 +0100

    	* acinclude.m4 (GLIBCXX_ENABLE_FILESYSTEM_TS): Disable for solaris.
    	* configure: Regenerate.

diff --git a/libstdc++-v3/acinclude.m4 b/libstdc++-v3/acinclude.m4
index 537ca6f..9d8d96d 100644
--- a/libstdc++-v3/acinclude.m4
+++ b/libstdc++-v3/acinclude.m4
@@ -3923,7 +3923,7 @@ AC_DEFUN([GLIBCXX_ENABLE_FILESYSTEM_TS], [
         enable_libstdcxx_filesystem_ts=yes
         ;;
       solaris*)
-        enable_libstdcxx_filesystem_ts=yes
+        enable_libstdcxx_filesystem_ts=no
         ;;
       *)
         enable_libstdcxx_filesystem_ts=no
diff --git a/libstdc++-v3/configure b/libstdc++-v3/configure
index 0a059c6..5ecfdeb 100755
--- a/libstdc++-v3/configure
+++ b/libstdc++-v3/configure
@@ -79163,7 +79163,7 @@ $as_echo_n "checking whether to build Filesystem TS support... " >&6; }
         enable_libstdcxx_filesystem_ts=yes
         ;;
       solaris*)
-        enable_libstdcxx_filesystem_ts=yes
+        enable_libstdcxx_filesystem_ts=no
         ;;
       *)
         enable_libstdcxx_filesystem_ts=no

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [patch] Implement ISO/IEC TS 18822 C++ File system TS
  2015-05-01 15:11   ` Jonathan Wakely
@ 2015-05-01 15:14     ` Rainer Orth
  0 siblings, 0 replies; 15+ messages in thread
From: Rainer Orth @ 2015-05-01 15:14 UTC (permalink / raw)
  To: Jonathan Wakely; +Cc: libstdc++, gcc-patches

Jonathan Wakely <jwakely@redhat.com> writes:

> Sorry about that, I'll add a check for fchmodat, in the meantime this
> will fix bootstrap. Committed as obvious.
>
>
> commit f4768ebcfd68e2fa6e4763d0b681e8fe710c64c4
> Author: Jonathan Wakely <jwakely@redhat.com>
> Date:   Fri May 1 16:09:28 2015 +0100
>
>     	* acinclude.m4 (GLIBCXX_ENABLE_FILESYSTEM_TS): Disable for solaris.
>     	* configure: Regenerate.

Which is overkill given that Solaris > 10 does support fchmodat, but it's
just a workaround anyway.

Thanks.
        Rainer

-- 
-----------------------------------------------------------------------------
Rainer Orth, Center for Biotechnology, Bielefeld University

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [patch] Implement ISO/IEC TS 18822 C++ File system TS
  2015-04-30 17:42 [patch] Implement ISO/IEC TS 18822 C++ File system TS Jonathan Wakely
  2015-04-30 19:33 ` Jonathan Wakely
  2015-05-01 12:45 ` Rainer Orth
@ 2015-05-01 17:03 ` Daniel Krügler
  2015-05-01 17:55   ` Jonathan Wakely
  2015-05-01 18:22   ` Jonathan Wakely
  2 siblings, 2 replies; 15+ messages in thread
From: Daniel Krügler @ 2015-05-01 17:03 UTC (permalink / raw)
  To: Jonathan Wakely; +Cc: libstdc++, gcc-patches List

2015-04-30 19:32 GMT+02:00 Jonathan Wakely <jwakely@redhat.com>:
> This is the complete <experimental/filesystem> implementation I intend
> to commit shortly. (It's also been pushed to the redi/filesystem-ts
> branch in the git mirror).

- There are three places where you refer to std::__addressof and one
where you refer to std::addressof. Is this difference intentional?

- I found all together six non-uglified usages of parameter name "rhs"
in the synopsis of directory_iterator and
recursive_directory_iterator.

b/libstdc++-v3/include/experimental/fs_ops.h:

- non-uglified parameters names "ec", "base", "p", "options",
"new_time", "prms":

path canonical(const path& p, error_code& ec);
path canonical(const path& p, const path& base, error_code& ec);

void copy(const path& __from, const path& __to, copy_options options);
void copy(const path& __from, const path& __to, copy_options options,
error_code& __ec) noexcept;

void last_write_time(const path& __p, file_time_type new_time);
void last_write_time(const path& __p, file_time_type new_time,
error_code& __ec) noexcept;

void permissions(const path& __p, perms prms);
void permissions(const path& __p, perms prms, error_code& __ec) noexcept;

b/libstdc++-v3/include/experimental/fs_path.h:

- non-uglified parameters names "n", "pos", "equals":

void _M_add_root_name(size_t n);
void _M_add_root_dir(size_t pos);
void _M_add_filename(size_t pos, size_t n);

class path::iterator:
    bool equals(iterator) const;

b/libstdc++-v3/src/filesystem/dir.cc:

- Shouldn't the template bool is_set(Bitmask obj, Bitmask bits) be inline?

b/libstdc++-v3/src/filesystem/ops.cc:

- Shouldn't the template  bool is_set(Bitmask obj, Bitmask bits) be inline?

- fs::space(const path& p, error_code& ec) noexcept:

If _GLIBCXX_HAVE_SYS_STATVFS_H is not defined, there should be a #else
that performs

ec = std::make_error_code(std::errc::not_supported);

- fs::path fs::temp_directory_path(error_code& ec):

The code-branch

#ifdef _GLIBCXX_FILESYSTEM_IS_WINDOWS

should start with:

ec = std::make_error_code(std::errc::not_supported);

b/libstdc++-v3/src/filesystem/path.cc:

- path::compare(const path& p) const noexcept:

Shouldn't the implementation of this noexcept function not try to
create copies of path objects? Couldn't _Cmpt just hold references to
_M_pathname?

- Daniel

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [patch] Implement ISO/IEC TS 18822 C++ File system TS
  2015-05-01 17:03 ` Daniel Krügler
@ 2015-05-01 17:55   ` Jonathan Wakely
  2015-05-01 18:22   ` Jonathan Wakely
  1 sibling, 0 replies; 15+ messages in thread
From: Jonathan Wakely @ 2015-05-01 17:55 UTC (permalink / raw)
  To: Daniel Krügler; +Cc: libstdc++, gcc-patches List

[-- Attachment #1: Type: text/plain, Size: 731 bytes --]

On 01/05/15 19:03 +0200, Daniel Krügler wrote:
>- There are three places where you refer to std::__addressof and one
>where you refer to std::addressof. Is this difference intentional?

No, I'll make them all use __addressof.

>- I found all together six non-uglified usages of parameter name "rhs"
>in the synopsis of directory_iterator and
>recursive_directory_iterator.

All names should be uglified as of this patch, which also fixes the
dg-require-filesystem-ts check so the tests actually run again(!)

Tested x86_64-linux and powerpc64le-linux, committed to trunk.


I'll address the other issues ASAP (I'm making a number of changes to
the #else branches in ops.cc and dir.cc).

Thanks very much for the thorough review.


[-- Attachment #2: patch.txt --]
[-- Type: text/x-patch, Size: 6857 bytes --]

commit 503fb9b4fe01b0c34d47590ed2384db179e04a57
Author: Jonathan Wakely <jwakely@redhat.com>
Date:   Fri May 1 18:41:38 2015 +0100

    	* include/experimental/fs_dir.h: Fix use of non-reserved names.
    	* include/experimental/fs_ops.h: Likewise.
    	* include/experimental/fs_path.h: Likewise.
    	* testsuite/lib/libstdc++.exp (check_v3_target_filesystem_ts): Use
    	C++11 when checking for support.

diff --git a/libstdc++-v3/include/experimental/fs_dir.h b/libstdc++-v3/include/experimental/fs_dir.h
index 0538fd2..d46d41b 100644
--- a/libstdc++-v3/include/experimental/fs_dir.h
+++ b/libstdc++-v3/include/experimental/fs_dir.h
@@ -180,14 +180,14 @@ _GLIBCXX_BEGIN_NAMESPACE_CXX11
       directory_options __options, error_code& __ec) noexcept
     : directory_iterator(__p, __options, &__ec) { }
 
-    directory_iterator(const directory_iterator& rhs) = default;
+    directory_iterator(const directory_iterator& __rhs) = default;
 
-    directory_iterator(directory_iterator&& rhs) noexcept = default;
+    directory_iterator(directory_iterator&& __rhs) noexcept = default;
 
     ~directory_iterator() = default;
 
-    directory_iterator& operator=(const directory_iterator& rhs) = default;
-    directory_iterator& operator=(directory_iterator&& rhs) noexcept = default;
+    directory_iterator& operator=(const directory_iterator& __rhs) = default;
+    directory_iterator& operator=(directory_iterator&& __rhs) noexcept = default;
 
     const directory_entry& operator*() const;
     const directory_entry* operator->() const { return &**this; }
@@ -269,9 +269,9 @@ _GLIBCXX_BEGIN_NAMESPACE_CXX11
 
     // modifiers
     recursive_directory_iterator&
-      operator=(const recursive_directory_iterator& rhs) noexcept;
+      operator=(const recursive_directory_iterator& __rhs) noexcept;
     recursive_directory_iterator&
-      operator=(recursive_directory_iterator&& rhs) noexcept;
+      operator=(recursive_directory_iterator&& __rhs) noexcept;
 
     recursive_directory_iterator& operator++();
     recursive_directory_iterator& increment(error_code& __ec) noexcept;
diff --git a/libstdc++-v3/include/experimental/fs_ops.h b/libstdc++-v3/include/experimental/fs_ops.h
index 0878fbb..6b7d470 100644
--- a/libstdc++-v3/include/experimental/fs_ops.h
+++ b/libstdc++-v3/include/experimental/fs_ops.h
@@ -53,8 +53,8 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
   path absolute(const path& __p, const path& __base = current_path());
 
   path canonical(const path& __p, const path& __base = current_path());
-  path canonical(const path& p, error_code& ec);
-  path canonical(const path& p, const path& base, error_code& ec);
+  path canonical(const path& __p, error_code& __ec);
+  path canonical(const path& __p, const path& __base, error_code& __ec);
 
   inline void
   copy(const path& __from, const path& __to)
@@ -64,8 +64,8 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
   copy(const path& __from, const path& __to, error_code& __ec) noexcept
   { copy(__from, __to, copy_options::none, __ec); }
 
-  void copy(const path& __from, const path& __to, copy_options options);
-  void copy(const path& __from, const path& __to, copy_options options,
+  void copy(const path& __from, const path& __to, copy_options __options);
+  void copy(const path& __from, const path& __to, copy_options __options,
 	    error_code& __ec) noexcept;
 
   inline bool
@@ -239,12 +239,12 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
 
   file_time_type  last_write_time(const path& __p);
   file_time_type  last_write_time(const path& __p, error_code& __ec) noexcept;
-  void last_write_time(const path& __p, file_time_type new_time);
-  void last_write_time(const path& __p, file_time_type new_time,
+  void last_write_time(const path& __p, file_time_type __new_time);
+  void last_write_time(const path& __p, file_time_type __new_time,
 		       error_code& __ec) noexcept;
 
-  void permissions(const path& __p, perms prms);
-  void permissions(const path& __p, perms prms, error_code& __ec) noexcept;
+  void permissions(const path& __p, perms __prms);
+  void permissions(const path& __p, perms __prms, error_code& __ec) noexcept;
 
   path read_symlink(const path& __p);
   path read_symlink(const path& __p, error_code& __ec);
diff --git a/libstdc++-v3/include/experimental/fs_path.h b/libstdc++-v3/include/experimental/fs_path.h
index ffc4926..11b0561 100644
--- a/libstdc++-v3/include/experimental/fs_path.h
+++ b/libstdc++-v3/include/experimental/fs_path.h
@@ -423,9 +423,9 @@ _GLIBCXX_BEGIN_NAMESPACE_CXX11
 
     void _M_split_cmpts();
     void _M_trim();
-    void _M_add_root_name(size_t n);
-    void _M_add_root_dir(size_t pos);
-    void _M_add_filename(size_t pos, size_t n);
+    void _M_add_root_name(size_t __n);
+    void _M_add_root_dir(size_t __pos);
+    void _M_add_filename(size_t __pos, size_t __n);
 
     string_type _M_pathname;
 
@@ -681,10 +681,10 @@ _GLIBCXX_BEGIN_NAMESPACE_CXX11
     iterator  operator--(int) { auto __tmp = *this; --_M_cur; return __tmp; }
 
     friend bool operator==(const iterator& __lhs, const iterator& __rhs)
-    { return __lhs.equals(__rhs); }
+    { return __lhs._M_equals(__rhs); }
 
     friend bool operator!=(const iterator& __lhs, const iterator& __rhs)
-    { return !__lhs.equals(__rhs); }
+    { return !__lhs._M_equals(__rhs); }
 
   private:
     friend class path;
@@ -697,7 +697,7 @@ _GLIBCXX_BEGIN_NAMESPACE_CXX11
     : _M_path(__path), _M_cur(), _M_at_end(__at_end)
     { }
 
-    bool equals(iterator) const;
+    bool _M_equals(iterator) const;
 
     const path* 		_M_path;
     path::_List::const_iterator _M_cur;
@@ -749,7 +749,7 @@ _GLIBCXX_BEGIN_NAMESPACE_CXX11
     inline path::_Path<_CharT*, _CharT*>&
     path::operator+=(_CharT __x)
     {
-      auto* __addr = std::addressof(__x);
+      auto* __addr = std::__addressof(__x);
       return concat(__addr, __addr + 1);
     }
 
@@ -990,7 +990,7 @@ _GLIBCXX_BEGIN_NAMESPACE_CXX11
   }
 
   inline bool
-  path::iterator::equals(iterator __rhs) const
+  path::iterator::_M_equals(iterator __rhs) const
   {
     if (_M_path != __rhs._M_path)
       return false;
diff --git a/libstdc++-v3/testsuite/lib/libstdc++.exp b/libstdc++-v3/testsuite/lib/libstdc++.exp
index 7ae4f3f..7d8f282 100644
--- a/libstdc++-v3/testsuite/lib/libstdc++.exp
+++ b/libstdc++-v3/testsuite/lib/libstdc++.exp
@@ -1896,6 +1896,8 @@ proc check_v3_target_little_endian { } {
 }
 
 proc check_v3_target_filesystem_ts { } {
+    global cxxflags
+    global DEFAULT_CXXFLAGS
     global et_filesystem_ts
     global tool
 
@@ -1930,7 +1932,11 @@ proc check_v3_target_filesystem_ts { } {
 	puts $f "#endif"
 	close $f
 
+	set cxxflags_saved $cxxflags
+	set cxxflags "$cxxflags $DEFAULT_CXXFLAGS -Werror -std=gnu++11"
+
 	set lines [v3_target_compile $src /dev/null preprocess ""]
+	set cxxflags $cxxflags_saved
 	file delete $src
 
 	if [string match "" $lines] {

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [patch] Implement ISO/IEC TS 18822 C++ File system TS
  2015-05-01 17:03 ` Daniel Krügler
  2015-05-01 17:55   ` Jonathan Wakely
@ 2015-05-01 18:22   ` Jonathan Wakely
  2015-05-01 19:28     ` Daniel Krügler
  1 sibling, 1 reply; 15+ messages in thread
From: Jonathan Wakely @ 2015-05-01 18:22 UTC (permalink / raw)
  To: Daniel Krügler; +Cc: libstdc++, gcc-patches List

On 01/05/15 19:03 +0200, Daniel Krügler wrote:
>b/libstdc++-v3/src/filesystem/path.cc:
>
>- path::compare(const path& p) const noexcept:
>
>Shouldn't the implementation of this noexcept function not try to
>create copies of path objects? Couldn't _Cmpt just hold references to
>_M_pathname?

All your other comments are definitely correct and I'll make the
relevant fixes soon, but I'm not sure what you mean here, could you
clarify?

I think it would be possible to implement a path like:

  class path
  {
    using value_type = ...;
    using string_type = basic_string<value_type>;
    struct _Cmpt;
    using composite_path = pair<string_type, list<_Cmpt>>;
    using sub_path = basic_string_view<value_type>;

    variant<composite_path, sub_path> _M_data;
    enum _Type { ... } _M_type;
  };

  struct _Cmpt : path
  {
    // ...
  };

With the invariant that for all objects of type _Cmpt the variant
member holds a string_view that refers to the string_type object in
some other path.

This would reduce the memory footprint of a path object (but for the
new std::string ABI, only if there are path components longer than the
SSO small string buffer). Even with that design, paths would still
be sizeof(string_type) + sizeof(std::list<_Cmpt>) + sizeof(_Type).

Alternatively, it could be done without a variant, just adding a
string_view member to the path type, which would make it even larger,
but would simplify the implementation.

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [patch] Implement ISO/IEC TS 18822 C++ File system TS
  2015-05-01 18:22   ` Jonathan Wakely
@ 2015-05-01 19:28     ` Daniel Krügler
  2015-05-01 19:38       ` Jonathan Wakely
  0 siblings, 1 reply; 15+ messages in thread
From: Daniel Krügler @ 2015-05-01 19:28 UTC (permalink / raw)
  To: Jonathan Wakely; +Cc: libstdc++, gcc-patches List

2015-05-01 20:22 GMT+02:00 Jonathan Wakely <jwakely@redhat.com>:
> On 01/05/15 19:03 +0200, Daniel Krügler wrote:
>>
>> b/libstdc++-v3/src/filesystem/path.cc:
>>
>> - path::compare(const path& p) const noexcept:
>>
>> Shouldn't the implementation of this noexcept function not try to
>> create copies of path objects? Couldn't _Cmpt just hold references to
>> _M_pathname?
>
> All your other comments are definitely correct and I'll make the
> relevant fixes soon, but I'm not sure what you mean here, could you
> clarify?

My remembrance of the difference in noexcept qualifications of

int  compare(const path& p) const noexcept;
int  compare(const string_type& s) const;
int  compare(const value_type* s) const;

is that the latter are allowed to allocate memory to be implemented as
the standard writes the corresponding functions for std::basic_string:

int compare(const basic_string& str) const noexcept;
int compare(const charT* s) const;

Returns: compare(basic_string(s)).

But if I read your implementation of path::compare(const path& p)
correctly it *also* may allocate memory by copying _M_pathname into a
_Cmpt object. I was wondering whether for this comparison there exists
real need to *copy* _M_pathname (potentially exceeding the memory
limits). Wouldn't it be possible to define a _CmptRef type that for
the purpose of implementing compare(const path& p) just refers to
references of the _M_pathname and therefore doesn't need to allocate
any dynamic storage? (I should have spoken of a new type _CmptRef and
not of your existing _Cmpt).

> I think it would be possible to implement a path like:

Actually I was not (yet) suggesting to change your API of path, I was
only wondering whether given the existing ABI compare(const path& p)
could be implementable without need of additional dynamic memory. This
was more a kind of louder thinking, I have not fully thought whether
this not possible without changing your currently suggested ABI.

- Daniel

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [patch] Implement ISO/IEC TS 18822 C++ File system TS
  2015-05-01 19:28     ` Daniel Krügler
@ 2015-05-01 19:38       ` Jonathan Wakely
  2015-05-01 20:05         ` Jonathan Wakely
  0 siblings, 1 reply; 15+ messages in thread
From: Jonathan Wakely @ 2015-05-01 19:38 UTC (permalink / raw)
  To: Daniel Krügler; +Cc: libstdc++, gcc-patches List

On 01/05/15 21:28 +0200, Daniel Krügler wrote:
>2015-05-01 20:22 GMT+02:00 Jonathan Wakely <jwakely@redhat.com>:
>> On 01/05/15 19:03 +0200, Daniel Krügler wrote:
>>>
>>> b/libstdc++-v3/src/filesystem/path.cc:
>>>
>>> - path::compare(const path& p) const noexcept:
>>>
>>> Shouldn't the implementation of this noexcept function not try to
>>> create copies of path objects? Couldn't _Cmpt just hold references to
>>> _M_pathname?
>>
>> All your other comments are definitely correct and I'll make the
>> relevant fixes soon, but I'm not sure what you mean here, could you
>> clarify?
>
>My remembrance of the difference in noexcept qualifications of
>
>int  compare(const path& p) const noexcept;
>int  compare(const string_type& s) const;
>int  compare(const value_type* s) const;
>
>is that the latter are allowed to allocate memory to be implemented as
>the standard writes the corresponding functions for std::basic_string:
>
>int compare(const basic_string& str) const noexcept;
>int compare(const charT* s) const;
>
>Returns: compare(basic_string(s)).
>
>But if I read your implementation of path::compare(const path& p)
>correctly it *also* may allocate memory by copying _M_pathname into a
>_Cmpt object.

Yes, I agree that there's a bug here that could cause it to
std::terminate.

>I was wondering whether for this comparison there exists
>real need to *copy* _M_pathname (potentially exceeding the memory
>limits). Wouldn't it be possible to define a _CmptRef type that for
>the purpose of implementing compare(const path& p) just refers to
>references of the _M_pathname and therefore doesn't need to allocate
>any dynamic storage? (I should have spoken of a new type _CmptRef and
>not of your existing _Cmpt).


Ah, I see what you mean (I thought you were suggesting some
improvements to _Cmpt itself ... which might be possible so I'm glad
you made me think about it :-)

I think I wrote compare() like that because it was easier, and when I
first implemented this we had COW strings so it wouldn't throw when
copying. That isn't true now, so I need to change it.

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [patch] Implement ISO/IEC TS 18822 C++ File system TS
       [not found]     ` <20150501105351.GY3618@redhat.com>
@ 2015-05-01 19:49       ` Jonathan Wakely
  2015-05-02  9:52         ` Jonathan Wakely
  2015-05-08 11:43         ` Rainer Orth
  0 siblings, 2 replies; 15+ messages in thread
From: Jonathan Wakely @ 2015-05-01 19:49 UTC (permalink / raw)
  To: Luke Allardyce; +Cc: libstdc++, gcc-patches, Daniel Krügler, Rainer Orth

[-- Attachment #1: Type: text/plain, Size: 2774 bytes --]

On 01/05/15 11:53 +0100, Jonathan Wakely wrote:
>On 01/05/15 10:05 +0900, Luke Allardyce wrote:
>>This fails on mingw-w64 4.02 due to
>
>Yay, thanks for trying it!
>
>>an extra typename:
>>
>>/mnt/build/native/gcc/x86_64-w64-mingw32/libstdc++-v3/include/experimental/fs_path.h:784:35:
>>error: expected nested-name-specifier
>>      using _CharAlloc = typename __alloc_rebind<_Allocator, char>;
>
>Oops, forgetting how to use my own alias.
>
>
>>readdir_r not available:
>>
>>/mnt/src/gcc/libstdc++-v3/src/filesystem/dir.cc:169:46: error:
>>'readdir_r' was not declared in this scope
>>  if (int err = readdir_r(dirp, &ent, &result))
>>
>>
>>and calling opendir with a wchar_t*
>>
>>/mnt/src/gcc/libstdc++-v3/src/filesystem/dir.cc:108:40: error: cannot
>>convert 'const value_type* {aka const wchar_t*}' to 'const char*' for
>>argument '1' to 'DIR* opendir(const char*)'
>>    if (DIR* dirp = ::opendir(p.c_str()))
>>
>>/mnt/src/gcc/libstdc++-v3/src/filesystem/dir.cc:256:38: error: cannot
>>convert 'const value_type* {aka const wchar_t*}' to 'const char*' for
>>argument '1' to 'DIR* opendir(const char*)'
>>  if (DIR* dirp = ::opendir(p.c_str()))
>
>Ah interesting, this means mingw supports <dirent.h>, I was expecting
>it to use the dummy definitions at the top of src/filesystem/dir.cc
>(where I'd made opendir take wchar_t).
>
>I've done some reading and am going to commit some fixes and
>improvements for mingw in an hour or two. It would be great if you
>could try it again after that.

I've committed the two changes attached (only tested on linux again).

patch2.txt should fix the mingw-w64 errors above, as well as the
issues Daniel reported, and should fix the error on Solaris 10

Rainer, would you be able to test with
--enable-libstdcxx-filesystem-ts before we re-enable it to build by
default on solaris* ?


patch1.txt changes path::_M_cmpts from std::list to std::vector (as
Marc suggested months ago) and fixes the pretty printer, so paths are
shown like this in GDB:

(gdb) print p
$1 = filesystem::path ""
(gdb) print c
$2 = filesystem::path "/home/jwakely" = {[root-directory] = "/", [1] = "home", [2] = "jwakely"}
(gdb) print r
$3 = filesystem::path "/" [root-directory]
(gdb) print h
$4 = filesystem::path "home"
(gdb) print l
$5 = filesystem::path "//host/foo/bar///baz/.//xyzzy/meta/syntactic/variable" = {[root-name] = "//host", [root-directory] = "/", [2] = "foo", [3] = "bar", [4] = "baz", [5] = ".", [6] = "xyzzy", [7] = "meta", [8] = "syntactic", [9] = "variable"}

That output came from debugging this code:

  path p;
  path c = current_path();
  path r = *c.begin();
  path h = *std::next(c.begin());
  path l = "//host/foo/bar///baz/.//xyzzy/meta/syntactic/variable";


Thanks everyone for testing and feedback, it's been very helpful.

[-- Attachment #2: patch1.txt --]
[-- Type: text/plain, Size: 6075 bytes --]

commit 44990ad8ae2cafab954b9e64556fc2ca0db97d7e
Author: Jonathan Wakely <jwakely@redhat.com>
Date:   Fri May 1 13:41:42 2015 +0100

    	* include/experimental/fs_path.h (path::_List): Use vector instead of
    	list.
    	* python/libstdcxx/v6/printers.py (StdExpPathPrinter): Adapt.
    	* src/filesystem/path.cc: Use std::prev instead of decrementing
    	rvalues. Fix whitespace.
    	* testsuite/experimental/filesystem/path/decompose/parent_path.cc:
    	Do not decrement iterators before begin.

diff --git a/libstdc++-v3/include/experimental/fs_path.h b/libstdc++-v3/include/experimental/fs_path.h
index 11b0561..e57a08b 100644
--- a/libstdc++-v3/include/experimental/fs_path.h
+++ b/libstdc++-v3/include/experimental/fs_path.h
@@ -36,7 +36,7 @@
 
 #include <utility>
 #include <type_traits>
-#include <list>
+#include <vector>
 #include <locale>
 #include <iosfwd>
 #include <codecvt>
@@ -430,7 +430,7 @@ _GLIBCXX_BEGIN_NAMESPACE_CXX11
     string_type _M_pathname;
 
     struct _Cmpt;
-    using _List = std::list<_Cmpt>;
+    using _List = _GLIBCXX_STD_C::vector<_Cmpt>;
     _List _M_cmpts; // empty unless _M_type == _Type::_Multi
     _Type _M_type = _Type::_Multi;
   };
diff --git a/libstdc++-v3/python/libstdcxx/v6/printers.py b/libstdc++-v3/python/libstdcxx/v6/printers.py
index 37c3b9b..c6f96d7 100644
--- a/libstdc++-v3/python/libstdcxx/v6/printers.py
+++ b/libstdc++-v3/python/libstdcxx/v6/printers.py
@@ -984,16 +984,51 @@ class StdExpPathPrinter:
 
     def __init__ (self, typename, val):
         self.val = val
-        self.list_visualizer = gdb.default_visualizer(val['_M_cmpts'])
+        start = self.val['_M_cmpts']['_M_impl']['_M_start']
+        finish = self.val['_M_cmpts']['_M_impl']['_M_finish']
+        self.num_cmpts = int (finish - start)
+
+    def _path_type(self):
+        t = str(self.val['_M_type'])
+        if t[-9:] == '_Root_dir':
+            return "root-directory"
+        if t[-10:] == '_Root_name':
+            return "root-name"
+        return None
 
     def to_string (self):
-        path = self.val ['_M_pathname']
-        if self.list_visualizer:
-            list_head = self.val['_M_cmpts']['_M_impl']['_M_node']
-            if list_head.address != list_head['_M_next']:
-                cmpts = self.list_visualizer.to_string()
-                path = "%s [Components %s]" % (path, cmpts)
-        return path
+        path = "%s" % self.val ['_M_pathname']
+        if self.num_cmpts == 0:
+            t = self._path_type()
+            if t:
+                path = '%s [%s]' % (path, t)
+        return "filesystem::path %s" % path
+
+    class _iterator(Iterator):
+        def __init__(self, cmpts):
+            self.item = cmpts['_M_impl']['_M_start']
+            self.finish = cmpts['_M_impl']['_M_finish']
+            self.count = 0
+
+        def __iter__(self):
+            return self
+
+        def __next__(self):
+            if self.item == self.finish:
+                raise StopIteration
+            item = self.item.dereference()
+            count = self.count
+            self.count = self.count + 1
+            self.item = self.item + 1
+            path = item['_M_pathname']
+            t = StdExpPathPrinter(item.type.name, item)._path_type()
+            if not t:
+                t = count
+            return ('[%s]' % t, path)
+
+    def children(self):
+        return self._iterator(self.val['_M_cmpts'])
+
 
 # A "regular expression" printer which conforms to the
 # "SubPrettyPrinter" protocol from gdb.printing.
@@ -1383,7 +1418,7 @@ def build_libstdcxx_dictionary ():
     # Filesystem TS components
     libstdcxx_printer.add_version('std::experimental::filesystem::v1::',
                                   'path', StdExpPathPrinter)
-    libstdcxx_printer.add_version('std::experimental::filesystem::v1::__cxx11',
+    libstdcxx_printer.add_version('std::experimental::filesystem::v1::__cxx11::',
                                   'path', StdExpPathPrinter)
 
     # Extensions.
diff --git a/libstdc++-v3/src/filesystem/path.cc b/libstdc++-v3/src/filesystem/path.cc
index db58f3b..cc5780f 100644
--- a/libstdc++-v3/src/filesystem/path.cc
+++ b/libstdc++-v3/src/filesystem/path.cc
@@ -35,7 +35,7 @@ path::remove_filename()
     {
       if (!_M_cmpts.empty())
 	{
-	  auto cmpt = --_M_cmpts.end();
+	  auto cmpt = std::prev(_M_cmpts.end());
 	  _M_pathname.erase(cmpt->_M_pos);
 	  _M_cmpts.erase(cmpt);
 	  _M_trim();
@@ -109,7 +109,7 @@ path::compare(const path& p) const noexcept
 {
   if (_M_type == _Type::_Multi && p._M_type == _Type::_Multi)
     return do_compare(_M_cmpts.begin(), _M_cmpts.end(),
-		   p._M_cmpts.begin(), p._M_cmpts.end());
+		      p._M_cmpts.begin(), p._M_cmpts.end());
   else if (_M_type == _Type::_Multi)
     {
       _Cmpt c[1] = { { p._M_pathname, p._M_type, 0 } };
@@ -130,8 +130,7 @@ path::root_name() const
   path __ret;
   if (_M_type == _Type::_Root_name)
     __ret = *this;
-  else if (_M_cmpts.size()
-      && _M_cmpts.begin()->_M_type == _Type::_Root_name)
+  else if (_M_cmpts.size() && _M_cmpts.begin()->_M_type == _Type::_Root_name)
     __ret = *_M_cmpts.begin();
   return __ret;
 }
@@ -203,8 +202,8 @@ path::parent_path() const
   path __ret;
   if (_M_cmpts.size() < 2)
     return __ret;
-  for (auto __it = _M_cmpts.begin(), __end = --_M_cmpts.end();
-      __it != __end; ++__it)
+  for (auto __it = _M_cmpts.begin(), __end = std::prev(_M_cmpts.end());
+       __it != __end; ++__it)
     {
       __ret /= *__it;
     }
diff --git a/libstdc++-v3/testsuite/experimental/filesystem/path/decompose/parent_path.cc b/libstdc++-v3/testsuite/experimental/filesystem/path/decompose/parent_path.cc
index 2c21f6f..41df1bf 100644
--- a/libstdc++-v3/testsuite/experimental/filesystem/path/decompose/parent_path.cc
+++ b/libstdc++-v3/testsuite/experimental/filesystem/path/decompose/parent_path.cc
@@ -44,6 +44,8 @@ test02()
 {
   for (const path& p : __gnu_test::test_paths)
   {
+    if (p.begin() == p.end())
+      continue;
     path pp;
     for (auto i = p.begin(), end = --p.end(); i != end; ++i)
     {

[-- Attachment #3: patch2.txt --]
[-- Type: text/plain, Size: 7672 bytes --]

commit e132913c9b82a9d57c4af3df2560dc89279a78a5
Author: Jonathan Wakely <jwakely@redhat.com>
Date:   Fri May 1 12:13:06 2015 +0100

    	* acinclude.m4 (GLIBCXX_ENABLE_FILESYSTEM_TS): Disable when <dirent.h>
    	is not available.
    	(GLIBCXX_CHECK_FILESYSTEM_DEPS): Check for fchmodat.
    	* configure: Regenerate.
    	* config.h.in: Regenerate.
    	* configure.ac: Check for utime.h
    	* include/experimental/fs_path.h (path::string<>)
    	[_GLIBCXX_FILESYSTEM_IS_WINDOWS]: Remove stray typename keyword.
    	* src/filesystem/dir.cc [!_GLIBCXX_HAVE_DIRENT_H] (DIR, opendir,
    	closedir, dirent, readdir_r): Replace dummy functions with #error.
    	(native_readdir, _Dir::advance): Use readdir when readdir_r is missing.
    	* src/filesystem/ops.cc (do_stat, is_set): Make inline.
    	(last_write_time) [!_GLIBCXX_USE_UTIMENSAT]: Use utime.
    	(permissions) [!_GLIBCXX_USE_FCHMODAT]: Use chmod.
    	(space, temp_directory_path) [_GLIBCXX_FILESYSTEM_IS_WINDOWS]: Set
    	error_code.

diff --git a/libstdc++-v3/acinclude.m4 b/libstdc++-v3/acinclude.m4
index 9d8d96d..07b5bd7 100644
--- a/libstdc++-v3/acinclude.m4
+++ b/libstdc++-v3/acinclude.m4
@@ -3914,6 +3914,9 @@ AC_DEFUN([GLIBCXX_ENABLE_FILESYSTEM_TS], [
     [permit yes|no|auto])
 
   AC_MSG_CHECKING([whether to build Filesystem TS support])
+  if test x"$ac_cv_header_dirent_h" != x"yes"; then
+    enable_libstdcxx_filesystem_ts=no
+  fi
   if test x"$enable_libstdcxx_filesystem_ts" = x"auto"; then
     case "${target_os}" in
       freebsd*|netbsd*|openbsd*|dragonfly*|darwin*)
@@ -3993,6 +3996,22 @@ dnl
   fi
   AC_MSG_RESULT($glibcxx_cv_st_mtim)
 dnl
+  AC_MSG_CHECKING([for fchmodat])
+  AC_CACHE_VAL(glibcxx_cv_fchmodat, [dnl
+    GCC_TRY_COMPILE_OR_LINK(
+      [
+        #include <fcntl.h>
+        #include <sys/stat.h>
+      ],
+      [fchmodat(AT_FDCWD, "", 0, AT_SYMLINK_NOFOLLOW);],
+      [glibcxx_cv_fchmodat=yes],
+      [glibcxx_cv_fchmodat=no])
+  ])
+  if test $glibcxx_cv_fchmodat = yes; then
+    AC_DEFINE(_GLIBCXX_USE_FCHMODAT, 1, [Define if fchmodat is available in <sys/stat.h>.])
+  fi
+  AC_MSG_RESULT($glibcxx_cv_fchmodat)
+dnl
   CXXFLAGS="$ac_save_CXXFLAGS"
   AC_LANG_RESTORE
 ])
diff --git a/libstdc++-v3/configure.ac b/libstdc++-v3/configure.ac
index 4b39bfa..311baa5 100644
--- a/libstdc++-v3/configure.ac
+++ b/libstdc++-v3/configure.ac
@@ -403,7 +403,7 @@ GLIBCXX_CONFIGURE_TESTSUITE
 GLIBCXX_CHECK_GTHREADS
 
 # For Filesystem TS.
-AC_CHECK_HEADERS([fcntl.h dirent.h sys/statvfs.h])
+AC_CHECK_HEADERS([fcntl.h dirent.h sys/statvfs.h utime.h])
 AC_STRUCT_DIRENT_D_TYPE
 GLIBCXX_ENABLE_FILESYSTEM_TS
 GLIBCXX_CHECK_FILESYSTEM_DEPS
diff --git a/libstdc++-v3/include/experimental/fs_path.h b/libstdc++-v3/include/experimental/fs_path.h
index e57a08b..33a16db 100644
--- a/libstdc++-v3/include/experimental/fs_path.h
+++ b/libstdc++-v3/include/experimental/fs_path.h
@@ -781,7 +781,7 @@ _GLIBCXX_BEGIN_NAMESPACE_CXX11
       const value_type* __last = __first + _M_pathname.size();
 
 #ifdef _GLIBCXX_FILESYSTEM_IS_WINDOWS
-      using _CharAlloc = typename __alloc_rebind<_Allocator, char>;
+      using _CharAlloc = __alloc_rebind<_Allocator, char>;
       using _String = basic_string<char, char_traits<char>, _CharAlloc>;
       using _WString = basic_string<_CharT, _Traits, _Allocator>;
 
diff --git a/libstdc++-v3/src/filesystem/dir.cc b/libstdc++-v3/src/filesystem/dir.cc
index 4ed869e..016a78d 100644
--- a/libstdc++-v3/src/filesystem/dir.cc
+++ b/libstdc++-v3/src/filesystem/dir.cc
@@ -25,7 +25,6 @@
 #include <experimental/filesystem>
 #include <utility>
 #include <stack>
-#include <tuple>
 #include <string.h>
 #include <errno.h>
 #ifdef _GLIBCXX_HAVE_DIRENT_H
@@ -34,17 +33,12 @@
 # endif
 # include <dirent.h>
 #else
-// TODO: replace dummy definitions with suitable Win32 code
-#ifndef EACCES
-# define EACCES static_cast<int>(std::errc::permission_denied)
+# error "the <dirent.h> header is needed to build the Filesystem TS"
 #endif
-using DIR = void;
-using P = std::experimental::filesystem::path;
-static DIR* opendir(const P::value_type*) { return nullptr; }
-static void closedir(DIR*) { }
-struct dirent { const char* d_name; };
-static inline int readdir_r(DIR*, dirent*, dirent**)
-{ return static_cast<int>(std::errc::not_supported); }
+
+#ifdef _GLIBCXX_FILESYSTEM_IS_WINDOWS
+# undef opendir
+# define opendir _wopendir
 #endif
 
 namespace fs = std::experimental::filesystem;
@@ -97,7 +91,7 @@ struct fs::_Dir
 namespace
 {
   template<typename Bitmask>
-    bool is_set(Bitmask obj, Bitmask bits)
+    inline bool is_set(Bitmask obj, Bitmask bits)
     {
       return (obj & bits) != Bitmask::none;
     }
@@ -159,14 +153,27 @@ namespace
     return fs::file_type::none;
 #endif
   }
+
+  int
+  native_readdir(DIR* dirp, ::dirent*& entryp)
+  {
+#ifdef _GLIBCXX_FILESYSTEM_IS_WINDOWS
+    errno = 0;
+    if ((entryp = ::readdir(dirp)))
+      return 0;
+    return errno;
+#else
+    return ::readdir_r(dirp, entryp, &entryp);
+#endif
+  }
 }
 
 bool
 fs::_Dir::advance(ErrorCode ec)
 {
   ::dirent ent;
-  ::dirent* result;
-  if (int err = readdir_r(dirp, &ent, &result))
+  ::dirent* result = &ent;
+  if (int err = native_readdir(dirp, result))
     {
       if (!ec)
 	_GLIBCXX_THROW_OR_ABORT(filesystem_error(
diff --git a/libstdc++-v3/src/filesystem/ops.cc b/libstdc++-v3/src/filesystem/ops.cc
index 091ca72..c7e3960 100644
--- a/libstdc++-v3/src/filesystem/ops.cc
+++ b/libstdc++-v3/src/filesystem/ops.cc
@@ -47,6 +47,16 @@
 # include <ext/stdio_filebuf.h>
 # include <ostream>
 #endif
+#if _GLIBCXX_HAVE_UTIME_H
+# include <utime.h>
+#endif
+
+#ifdef _GLIBCXX_FILESYSTEM_IS_WINDOWS
+# undef utime
+# define utime _wutime
+# undef chmod
+# define chmod _wchmod
+#endif
 
 namespace fs = std::experimental::filesystem;
 
@@ -131,7 +141,7 @@ fs::copy(const path& from, const path& to, copy_options options)
 namespace
 {
   template<typename Bitmask>
-    bool is_set(Bitmask obj, Bitmask bits)
+    inline bool is_set(Bitmask obj, Bitmask bits)
     {
       return (obj & bits) != Bitmask::none;
     }
@@ -767,7 +777,7 @@ fs::file_size(const path& p)
 namespace
 {
   template<typename Accessor, typename T>
-    T
+    inline T
     do_stat(const fs::path& p, std::error_code& ec, Accessor f, T deflt)
     {
 #ifdef _GLIBCXX_HAVE_SYS_STAT_H
@@ -871,6 +881,14 @@ fs::last_write_time(const path& p __attribute__((__unused__)),
     ec.assign(errno, std::generic_category());
   else
     ec.clear();
+#elif _GLIBCXX_HAVE_UTIME_H
+  ::utimbuf times;
+  times.modtime = s.count();
+  times.actime = do_stat(p, ec, std::mem_fn(&stat::st_atime), times.modtime);
+  if (::utime(p.c_str(), &times))
+    ec.assign(errno, std::generic_category());
+  else
+    ec.clear();
 #else
   ec = std::make_error_code(std::errc::not_supported);
 #endif
@@ -887,7 +905,11 @@ fs::permissions(const path& p, perms prms)
 
 void fs::permissions(const path& p, perms prms, error_code& ec) noexcept
 {
+#if _GLIBCXX_USE_FCHMODAT
   if (int err = ::fchmodat(AT_FDCWD, p.c_str(), static_cast<mode_t>(prms), 0))
+#else
+  if (int err = ::chmod(p.c_str(), static_cast<mode_t>(prms)))
+#endif
     ec.assign(err, std::generic_category());
   else
     ec.clear();
@@ -1051,6 +1073,8 @@ fs::space(const path& p, error_code& ec) noexcept
       };
       ec.clear();
     }
+#else
+  ec = std::make_error_code(std::errc::not_supported);
 #endif
   return info;
 }
@@ -1157,6 +1181,7 @@ fs::path fs::temp_directory_path()
 fs::path fs::temp_directory_path(error_code& ec)
 {
 #ifdef _GLIBCXX_FILESYSTEM_IS_WINDOWS
+  ec = std::make_error_code(std::errc::not_supported);
   return {}; // TODO
 #else
   const char* tmpdir = ::getenv("TMPDIR");



^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [patch] Implement ISO/IEC TS 18822 C++ File system TS
  2015-05-01 19:38       ` Jonathan Wakely
@ 2015-05-01 20:05         ` Jonathan Wakely
  0 siblings, 0 replies; 15+ messages in thread
From: Jonathan Wakely @ 2015-05-01 20:05 UTC (permalink / raw)
  To: Daniel Krügler; +Cc: libstdc++, gcc-patches List

[-- Attachment #1: Type: text/plain, Size: 1283 bytes --]

On 01/05/15 20:38 +0100, Jonathan Wakely wrote:
>On 01/05/15 21:28 +0200, Daniel Krügler wrote:
>>But if I read your implementation of path::compare(const path& p)
>>correctly it *also* may allocate memory by copying _M_pathname into a
>>_Cmpt object.
>
>Yes, I agree that there's a bug here that could cause it to
>std::terminate.
>
>>I was wondering whether for this comparison there exists
>>real need to *copy* _M_pathname (potentially exceeding the memory
>>limits). Wouldn't it be possible to define a _CmptRef type that for
>>the purpose of implementing compare(const path& p) just refers to
>>references of the _M_pathname and therefore doesn't need to allocate
>>any dynamic storage? (I should have spoken of a new type _CmptRef and
>>not of your existing _Cmpt).
>
>
>Ah, I see what you mean (I thought you were suggesting some
>improvements to _Cmpt itself ... which might be possible so I'm glad
>you made me think about it :-)
>
>I think I wrote compare() like that because it was easier, and when I
>first implemented this we had COW strings so it wouldn't throw when
>copying. That isn't true now, so I need to change it.

And here's the fix for this noexcept issue, it was very simple in the
end, thanks for the idea.

Tested powerpc64le-linux, committed to trunk.


[-- Attachment #2: patch.txt --]
[-- Type: text/x-patch, Size: 1216 bytes --]

commit 9c2f4abea2096bcc5e3568facf12fb3550358c6b
Author: Jonathan Wakely <jwakely@redhat.com>
Date:   Fri May 1 20:57:21 2015 +0100

    	* src/filesystem/path.cc (path::compare): Do not copy strings.

diff --git a/libstdc++-v3/src/filesystem/path.cc b/libstdc++-v3/src/filesystem/path.cc
index cc5780f..7924732 100644
--- a/libstdc++-v3/src/filesystem/path.cc
+++ b/libstdc++-v3/src/filesystem/path.cc
@@ -107,17 +107,23 @@ namespace
 int
 path::compare(const path& p) const noexcept
 {
+  struct CmptRef
+  {
+    const path* ptr;
+    const string_type& native() const noexcept { return ptr->native(); }
+  };
+
   if (_M_type == _Type::_Multi && p._M_type == _Type::_Multi)
     return do_compare(_M_cmpts.begin(), _M_cmpts.end(),
 		      p._M_cmpts.begin(), p._M_cmpts.end());
   else if (_M_type == _Type::_Multi)
     {
-      _Cmpt c[1] = { { p._M_pathname, p._M_type, 0 } };
+      CmptRef c[1] = { { &p } };
       return do_compare(_M_cmpts.begin(), _M_cmpts.end(), c, c+1);
     }
   else if (p._M_type == _Type::_Multi)
     {
-      _Cmpt c[1] = { { _M_pathname, _M_type, 0 } };
+      CmptRef c[1] = { { this } };
       return do_compare(c, c+1, p._M_cmpts.begin(), p._M_cmpts.end());
     }
   else

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [patch] Implement ISO/IEC TS 18822 C++ File system TS
  2015-05-01 19:49       ` Jonathan Wakely
@ 2015-05-02  9:52         ` Jonathan Wakely
  2015-05-08 11:43         ` Rainer Orth
  1 sibling, 0 replies; 15+ messages in thread
From: Jonathan Wakely @ 2015-05-02  9:52 UTC (permalink / raw)
  To: libstdc++, gcc-patches; +Cc: Daniel Krügler, Rainer Orth, Luke Allardyce

[-- Attachment #1: Type: text/plain, Size: 327 bytes --]

Another portability fix, to avoid using &stat::st_atime because
st_atime might be a macro, and &stat::st_atim.tv_sec is not valid.

Also POSIX doesn't actually require any particular structure for
timespec, so set the fields explicitly instead of using braces.

Tested powerpc64le-linux and powerpc-aix7, committed to trunk.



[-- Attachment #2: patch.txt --]
[-- Type: text/x-patch, Size: 1694 bytes --]

commit 2a89701d33798b1b5425e56e3050024327879b5f
Author: Jonathan Wakely <jwakely@redhat.com>
Date:   Fri May 1 23:46:21 2015 +0100

    	* src/filesystem/ops.cc (last_write_time) [_GLIBCXX_USE_UTIMENSAT]:
    	Set timespec members explicitly instead of with a braced-init-list.
    	[_GLIBCXX_HAVE_UTIME_H]: Use lambda to handle st_atime being a macro.

diff --git a/libstdc++-v3/src/filesystem/ops.cc b/libstdc++-v3/src/filesystem/ops.cc
index c7e3960..aa1ab04 100644
--- a/libstdc++-v3/src/filesystem/ops.cc
+++ b/libstdc++-v3/src/filesystem/ops.cc
@@ -871,20 +871,22 @@ fs::last_write_time(const path& p __attribute__((__unused__)),
 {
   auto d = new_time.time_since_epoch();
   auto s = chrono::duration_cast<chrono::seconds>(d);
+#if _GLIBCXX_USE_UTIMENSAT
   auto ns = chrono::duration_cast<chrono::nanoseconds>(d - s);
-#ifdef _GLIBCXX_USE_UTIMENSAT
-  struct ::timespec ts[2] = {
-    { 0, UTIME_OMIT },
-    { static_cast<std::time_t>(s.count()), static_cast<long>(ns.count()) }
-  };
-  if (utimensat(AT_FDCWD, p.c_str(), ts, 0))
+  struct ::timespec ts[2];
+  ts[0].tv_sec = 0;
+  ts[0].tv_nsec = UTIME_OMIT;
+  ts[1].tv_sec = static_cast<std::time_t>(s.count());
+  ts[1].tv_nsec = static_cast<long>(ns.count());
+  if (::utimensat(AT_FDCWD, p.c_str(), ts, 0))
     ec.assign(errno, std::generic_category());
   else
     ec.clear();
 #elif _GLIBCXX_HAVE_UTIME_H
   ::utimbuf times;
   times.modtime = s.count();
-  times.actime = do_stat(p, ec, std::mem_fn(&stat::st_atime), times.modtime);
+  times.actime = do_stat(p, ec, [](const auto& st) { return st.st_atime; },
+			 times.modtime);
   if (::utime(p.c_str(), &times))
     ec.assign(errno, std::generic_category());
   else

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [patch] Implement ISO/IEC TS 18822 C++ File system TS
  2015-05-01 19:49       ` Jonathan Wakely
  2015-05-02  9:52         ` Jonathan Wakely
@ 2015-05-08 11:43         ` Rainer Orth
  2015-05-13 10:56           ` Jonathan Wakely
  1 sibling, 1 reply; 15+ messages in thread
From: Rainer Orth @ 2015-05-08 11:43 UTC (permalink / raw)
  To: Jonathan Wakely
  Cc: Luke Allardyce, libstdc++, gcc-patches, Daniel Krügler

Jonathan Wakely <jwakely@redhat.com> writes:

> I've committed the two changes attached (only tested on linux again).
>
> patch2.txt should fix the mingw-w64 errors above, as well as the
> issues Daniel reported, and should fix the error on Solaris 10
>
> Rainer, would you be able to test with
> --enable-libstdcxx-filesystem-ts before we re-enable it to build by
> default on solaris* ?

I just did: a bootstrap on i386-pc-solaris2.10 completed successfully
and the experimental/filesystem tests all PASSed.  Given that Solaris 11
was fine even before, I think it's safe to re-enable it by default on
Solaris again.

Thanks.
        Rainer

-- 
-----------------------------------------------------------------------------
Rainer Orth, Center for Biotechnology, Bielefeld University

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [patch] Implement ISO/IEC TS 18822 C++ File system TS
  2015-05-08 11:43         ` Rainer Orth
@ 2015-05-13 10:56           ` Jonathan Wakely
  0 siblings, 0 replies; 15+ messages in thread
From: Jonathan Wakely @ 2015-05-13 10:56 UTC (permalink / raw)
  To: Rainer Orth; +Cc: Luke Allardyce, libstdc++, gcc-patches, Daniel Krügler

[-- Attachment #1: Type: text/plain, Size: 734 bytes --]

On 08/05/15 13:43 +0200, Rainer Orth wrote:
>Jonathan Wakely <jwakely@redhat.com> writes:
>
>> I've committed the two changes attached (only tested on linux again).
>>
>> patch2.txt should fix the mingw-w64 errors above, as well as the
>> issues Daniel reported, and should fix the error on Solaris 10
>>
>> Rainer, would you be able to test with
>> --enable-libstdcxx-filesystem-ts before we re-enable it to build by
>> default on solaris* ?
>
>I just did: a bootstrap on i386-pc-solaris2.10 completed successfully
>and the experimental/filesystem tests all PASSed.  Given that Solaris 11
>was fine even before, I think it's safe to re-enable it by default on
>Solaris again.

Thanks. Re-enabled with this patch, committed to trunk.

[-- Attachment #2: patch.txt --]
[-- Type: text/x-patch, Size: 1159 bytes --]

commit 38715a0b0a5afcb980570dfb65c591e3a7eb8cb9
Author: Jonathan Wakely <jwakely@redhat.com>
Date:   Sun May 10 19:46:25 2015 +0100

    	* acinclude.m4 (GLIBCXX_ENABLE_FILESYSTEM_TS): Re-enable on solaris.
    	* configure: Regenerate.

diff --git a/libstdc++-v3/acinclude.m4 b/libstdc++-v3/acinclude.m4
index 07b5bd7..6f67774 100644
--- a/libstdc++-v3/acinclude.m4
+++ b/libstdc++-v3/acinclude.m4
@@ -3926,7 +3926,7 @@ AC_DEFUN([GLIBCXX_ENABLE_FILESYSTEM_TS], [
         enable_libstdcxx_filesystem_ts=yes
         ;;
       solaris*)
-        enable_libstdcxx_filesystem_ts=no
+        enable_libstdcxx_filesystem_ts=yes
         ;;
       *)
         enable_libstdcxx_filesystem_ts=no
diff --git a/libstdc++-v3/configure b/libstdc++-v3/configure
index 90a5192..39fc28c 100755
--- a/libstdc++-v3/configure
+++ b/libstdc++-v3/configure
@@ -79166,7 +79166,7 @@ $as_echo_n "checking whether to build Filesystem TS support... " >&6; }
         enable_libstdcxx_filesystem_ts=yes
         ;;
       solaris*)
-        enable_libstdcxx_filesystem_ts=no
+        enable_libstdcxx_filesystem_ts=yes
         ;;
       *)
         enable_libstdcxx_filesystem_ts=no

^ permalink raw reply	[flat|nested] 15+ messages in thread

end of thread, other threads:[~2015-05-13 10:54 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-04-30 17:42 [patch] Implement ISO/IEC TS 18822 C++ File system TS Jonathan Wakely
2015-04-30 19:33 ` Jonathan Wakely
     [not found]   ` <CAFW6PZB9WvskzrKCsNKK4s-5oXWjcwkuZGMrWbrfgJ7fpJiCXQ@mail.gmail.com>
     [not found]     ` <20150501105351.GY3618@redhat.com>
2015-05-01 19:49       ` Jonathan Wakely
2015-05-02  9:52         ` Jonathan Wakely
2015-05-08 11:43         ` Rainer Orth
2015-05-13 10:56           ` Jonathan Wakely
2015-05-01 12:45 ` Rainer Orth
2015-05-01 15:11   ` Jonathan Wakely
2015-05-01 15:14     ` Rainer Orth
2015-05-01 17:03 ` Daniel Krügler
2015-05-01 17:55   ` Jonathan Wakely
2015-05-01 18:22   ` Jonathan Wakely
2015-05-01 19:28     ` Daniel Krügler
2015-05-01 19:38       ` Jonathan Wakely
2015-05-01 20:05         ` Jonathan Wakely

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).