https://gcc.gnu.org/bugzilla/show_bug.cgi?id=71044

--- Comment #6 from Jonathan Wakely <redi at gcc dot gnu.org> ---
(In reply to Jonathan Wakely from comment #0)
> Before the code is moved from std::experimental::filesystem to
> std::filesystem (and into the shared library) there are a few improvements
> that should be made.
> 
> http://wg21.link/lwg2674 relaxes requirements on path::iterator so that a
> path is no longer required to embed a container of paths. The components
> could be stored as string_view objects, and only converted to path objects
> as needed (generally that means when a path::iterator is dereferenced).

I've decided against this. The current implementation avoid the need for
synchronization to make sure lazy init doesn't have data races, and avoids
lifetime problems due to paths being stashed in the iterators e.g.

  filesystem::path p = "foo/bar";
  basic_string_view s = std::next(p->begin())->native();

This creates a dangling string view if the path "bar" and its native string
only have the lifetime of the iterator.

> Parsing and splitting a path into components could be done lazily, so that
> it isn't done for intermediate temporary copies of a path, only the final
> value that is manipulated. Move semantics might mean this is unnecessary,
> since performance-sensitive code can usually move values instead of copying
> them.

Parsing and copying paths is now much faster (but still performs a lot of
allocations).

> Directory iterators currently construct a path object on every increment.
> This means that incrementing a directory iterator five times then
> dereferencing it constructs five paths which are never used (and each one is
> eagerly split into components, allocating a container of paths!).
> Construction of the path could be postponed so that it's only done on
> dereference, so that the five increments are cheap. This has thread-safety
> consequences, as the dereference is a const operation and could be performed
> concurrently, so the lazy construction of the path needs to be synchronized.

This has not been done.

(In reply to Jonathan Wakely from comment #2)
> Created attachment 38463 [details]
> Add assertions to directory iterator functions
> 
> Also consider replacing non-inline functions such as
> directory_iterator::increment() with inline ones with assertions.

Maybe not necessary. Building with --enable-libstdcxx-debug and then linking to
$libdir/debug/libstdc++s.a gives a version of the library with assertions
enabled.

> This would allow those functions to check preconditions at runtime, and more
> functions could be noexcept.

(In reply to Jonathan Wakely from comment #3)
> Several member functions of path construct a path from their argument, but
> could be done without any allocations, e.g. path::compare(string_view)

This has not been done, but would be trivial with the new path::_Parser helper
type, which splits a string into components with no allocations, so that each
component could be compared to [this->begin(),this->end()).

Reply via email to