llvm dev - Aug 2018 - [cfe-dev] Filesystem has Landed in Libc++

Hi All,

I recently committed <filesystem> to trunk. I wanted to bring attention to
some quirks it currently has.

First, it's been put in a separate library, libc++fs, for now. Users are
responsible for linking the library when they use filesystem.

Second, it should still not be considered ABI stable. Vendors should be
aware of this before shipping it. Hopefully all the standard and
implementation bugs can be resolved by the next release, and we can move it
into the main dylib.

Third, libc++experimental no longer contains the symbols for
<experimental/filesystem>, which is really just <filesystem> is
disguise.
If you've been using <experimental/filesystem> you now need to link
libc++fs instead.

Fourth, `<filesystem>` is technically available in C++11 and later. The
implementation lives in the std::__fs::filesystem namespace, which is
marked "inline" in C++17 but not before. We should consider
documenting
this as an extension to its use w/o C++17.

Happy coding,

/Eric

[1]
http://libcxx.llvm.org/docs/UsingLibcxx.html#using-filesystem-and-libc-fs
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20180726/74093f9e/attachment.html>

Eric,

I’m curious to know what the concerns are w.r.t. providing ABI stability for
filesystem right now. What do you envision may require changing the ABI in the
future?

I feel like taking filesystem out of experimental/ without providing the usual
guarantees provided by libc++ for non-experimental code may not be a good idea,
as we’ll be pretending that we support filesystem when we really only half
support it. In other words, I think the number of people that will start using
filesystem while _consciously_ knowing that it is ABI-unstable (and what that
means) is quite small, and that is making our users a disservice.

Would it be possible to instead ship the parts we’re not quite sure we can keep
ABI stable in the headers (with `_LIBCPP_HIDE_FROM_ABI`) for the time being, and
lower them to the dylib eventually as things stabilize? This would allow us to
ship filesystem with LLVM 7.0 without any compromise on the guarantees we make
our users.

I’m curious to know what you think of this suggestion.
Louis
> On Jul 27, 2018, at 00:20, Eric Fiselier via cfe-dev <cfe-dev at
lists.llvm.org> wrote:
> 
> Hi All,
> 
> I recently committed <filesystem> to trunk. I wanted to bring
attention to some quirks it currently has.
> 
> First, it's been put in a separate library, libc++fs, for now. Users
are responsible for linking the library when they use filesystem.
> 
> Second, it should still not be considered ABI stable. Vendors should be
aware of this before shipping it. Hopefully all the standard and implementation
bugs can be resolved by the next release, and we can move it into the main
dylib.
> 
> Third, libc++experimental no longer contains the symbols for
<experimental/filesystem>, which is really just <filesystem> is
disguise. If you've been using <experimental/filesystem> you now need
to link libc++fs instead.
> 
> Fourth, `<filesystem>` is technically available in C++11 and later.
The implementation lives in the std::__fs::filesystem namespace, which is marked
"inline" in C++17 but not before. We should consider documenting this
as an extension to its use w/o C++17.
> 
> Happy coding,
> 
> /Eric
> 
> [1]
http://libcxx.llvm.org/docs/UsingLibcxx.html#using-filesystem-and-libc-fs
<http://libcxx.llvm.org/docs/UsingLibcxx.html#using-filesystem-and-libc-fs>_______________________________________________
> cfe-dev mailing list
> cfe-dev at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20180727/b8966bf8/attachment.html>

FWIW, I’d like for us to come to an agreement before the branch for LLVM 7.0 is
cut. How do others feel about this? Am I wrong when I claim that shipping an
ABI-unstable feature in the std:: namespace is a deviation from normal practice?
Am I overcautious when I say it’s asking for trouble?

Eric, I know you’re busy and may not have time to do the work so I’m totally
willing to chime in, but I’d like to have your thoughts on my objection first.

Cheers,
Louis
> On Jul 27, 2018, at 17:02, Louis Dionne via cfe-dev <cfe-dev at
lists.llvm.org> wrote:
> 
> Eric,
> 
> I’m curious to know what the concerns are w.r.t. providing ABI stability
for filesystem right now. What do you envision may require changing the ABI in
the future?
> 
> I feel like taking filesystem out of experimental/ without providing the
usual guarantees provided by libc++ for non-experimental code may not be a good
idea, as we’ll be pretending that we support filesystem when we really only half
support it. In other words, I think the number of people that will start using
filesystem while _consciously_ knowing that it is ABI-unstable (and what that
means) is quite small, and that is making our users a disservice.
> 
> Would it be possible to instead ship the parts we’re not quite sure we can
keep ABI stable in the headers (with `_LIBCPP_HIDE_FROM_ABI`) for the time
being, and lower them to the dylib eventually as things stabilize? This would
allow us to ship filesystem with LLVM 7.0 without any compromise on the
guarantees we make our users.
> 
> I’m curious to know what you think of this suggestion.
> Louis
> 
>> On Jul 27, 2018, at 00:20, Eric Fiselier via cfe-dev <cfe-dev at
lists.llvm.org <mailto:cfe-dev at lists.llvm.org>> wrote:
>> 
>> Hi All,
>> 
>> I recently committed <filesystem> to trunk. I wanted to bring
attention to some quirks it currently has.
>> 
>> First, it's been put in a separate library, libc++fs, for now.
Users are responsible for linking the library when they use filesystem.
>> 
>> Second, it should still not be considered ABI stable. Vendors should be
aware of this before shipping it. Hopefully all the standard and implementation
bugs can be resolved by the next release, and we can move it into the main
dylib.
>> 
>> Third, libc++experimental no longer contains the symbols for
<experimental/filesystem>, which is really just <filesystem> is
disguise. If you've been using <experimental/filesystem> you now need
to link libc++fs instead.
>> 
>> Fourth, `<filesystem>` is technically available in C++11 and
later. The implementation lives in the std::__fs::filesystem namespace, which is
marked "inline" in C++17 but not before. We should consider
documenting this as an extension to its use w/o C++17.
>> 
>> Happy coding,
>> 
>> /Eric
>> 
>> [1]
http://libcxx.llvm.org/docs/UsingLibcxx.html#using-filesystem-and-libc-fs
<http://libcxx.llvm.org/docs/UsingLibcxx.html#using-filesystem-and-libc-fs>_______________________________________________
>> cfe-dev mailing list
>> cfe-dev at lists.llvm.org <mailto:cfe-dev at lists.llvm.org>
>> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev
<http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev>
> 
> _______________________________________________
> cfe-dev mailing list
> cfe-dev at lists.llvm.org <mailto:cfe-dev at lists.llvm.org>
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev
<http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev>
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20180730/ed53f688/attachment.html>

On Thu, Jul 26, 2018 at 9:20 PM, Eric Fiselier via cfe-dev <
cfe-dev at lists.llvm.org> wrote:
> Hi All,
>
> I recently committed <filesystem> to trunk. I wanted to bring
attention to
> some quirks it currently has.
>
> First, it's been put in a separate library, libc++fs, for now. Users
are
> responsible for linking the library when they use filesystem.
>
> Second, it should still not be considered ABI stable. Vendors should be
> aware of this before shipping it. Hopefully all the standard and
> implementation bugs can be resolved by the next release, and we can move it
> into the main dylib.
>
[I have had discussions with several people, and I'm attempting to
summarize here]

Due to factors beyond our control, I do not believe that we can provide a
version of std::filesystem and promise future ABI stability at this time.

More information:

* The features for caching directory information were added late in the
C++17 cycle, and there have been some concerns about them.
LWG issue #2708 (https://wg21.link/lwg2708) is one of them, and there are a
couple of upcoming papers about the same part of the standard.

* The clock stuff being added in C++20 has already been discussed here.


We can:

1) Not ship std::filesystem, shipping only std::experimental::filesystem.
I think that this is a disservice to our users; because people are asking
for std::filesystem, and other vendors are providing it.
Note: experimental::filesystem is *different* from std::filesystem, and
they're only going to diverge further. In an ideal world, we would have two
implementations; one for experimental::filesystem, and the other for
std::filesystem, and they would behave differently (each according to their
specification)

2) Ship std::filesystem as it is, as part of libc++.dylib.
I don't think that this is a viable option, given that we are pretty sure
(certain) that ABI changes are coming down the pike.

3) We can ship std::filesystem as a static library; marked as "not ABI
stable"
We can put it into the libc++ dylib once we're confident that we can
provide a stable ABI.

Note: libstdc++ has done exactly this.
See
https://www.reddit.com/r/cpp/comments/789g99/gcc_lands_support_for_c_filesystem_ts/
for a discussion of this approach.

People can use std::filesystem, and include the object code it in their
executables, and when the ABI changes, they will be affected at build time,
not at run time. The downside is that they will have to re-build to get bug
fixes.

-- Marshall

P.S. I admit that this is not the best of all possible worlds.
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20180810/da296306/attachment.html>

> On Aug 10, 2018, at 13:28, Marshall Clow via cfe-dev <cfe-dev at
lists.llvm.org> wrote:
> 
> 
> 
> On Thu, Jul 26, 2018 at 9:20 PM, Eric Fiselier via cfe-dev <cfe-dev at
lists.llvm.org <mailto:cfe-dev at lists.llvm.org>> wrote:
> Hi All,
> 
> I recently committed <filesystem> to trunk. I wanted to bring
attention to some quirks it currently has.
> 
> First, it's been put in a separate library, libc++fs, for now. Users
are responsible for linking the library when they use filesystem.
> 
> Second, it should still not be considered ABI stable. Vendors should be
aware of this before shipping it. Hopefully all the standard and implementation
bugs can be resolved by the next release, and we can move it into the main
dylib.
> 
> [I have had discussions with several people, and I'm attempting to
summarize here]
> 
> Due to factors beyond our control, I do not believe that we can provide a
version of std::filesystem and promise future ABI stability at this time.
> 
> More information:
> 
> * The features for caching directory information were added late in the
C++17 cycle, and there have been some concerns about them.
> LWG issue #2708 (https://wg21.link/lwg2708
<https://wg21.link/lwg2708>) is one of them, and there are a couple of
upcoming papers about the same part of the standard.
> 
> * The clock stuff being added in C++20 has already been discussed here.
> 
> 
> We can:
> 
> 1) Not ship std::filesystem, shipping only std::experimental::filesystem.
> I think that this is a disservice to our users; because people are asking
for std::filesystem, and other vendors are providing it.
> Note: experimental::filesystem is *different* from std::filesystem, and
they're only going to diverge further. In an ideal world, we would have two
implementations; one for experimental::filesystem, and the other for
std::filesystem, and they would behave differently (each according to their
specification)
> 
> 2) Ship std::filesystem as it is, as part of libc++.dylib.
> I don't think that this is a viable option, given that we are pretty
sure (certain) that ABI changes are coming down the pike.
> 
> 3) We can ship std::filesystem as a static library; marked as "not ABI
stable"
> We can put it into the libc++ dylib once we’re confident that we can
provide a stable ABI.
After talking to Marshall and Eric, I believe 3) is OK and I take back my
objection to shipping a non-experimental filesystem in LLVM 7. The main reasons
are:

- We’re forcing users to link manually with -lc++fs. This forces them to read
the documentation, which contains a fat warning about ABI stability.
- This is what GCC and MS have done — it’s easier if we stay aligned with them.
- Since we’re shipping c++fs as a static library, the only potential problem is
that users will use ABI-unstable types in their own ABIs. We’re not going to
break the symbols exported from libc++.dylib at all.
- Filesystem is in an inline namespace __fs, so we can decide to bump that
inline namespace if we break the ABI. This will cause users that may have leaked
filesystem types in their ABIs to get clear link errors when we change the ABI.

So I think 3), which is the status quo, is fine.

Louis
> 
> Note: libstdc++ has done exactly this.
> See
https://www.reddit.com/r/cpp/comments/789g99/gcc_lands_support_for_c_filesystem_ts/
<https://www.reddit.com/r/cpp/comments/789g99/gcc_lands_support_for_c_filesystem_ts/>
for a discussion of this approach.
> 
> People can use std::filesystem, and include the object code it in their
executables, and when the ABI changes, they will be affected at build time, not
at run time. The downside is that they will have to re-build to get bug fixes.
> 
> -- Marshall
> 
> P.S. I admit that this is not the best of all possible worlds.
> 
> _______________________________________________
> cfe-dev mailing list
> cfe-dev at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20180810/f778009a/attachment.html>

On Aug 10, 2018, at 1:28 PM, Marshall Clow via cfe-dev <cfe-dev at
lists.llvm.org> wrote:
> 
> * The clock stuff being added in C++20 has already been discussed here.
I’ve missed the discussions on file_time_type, however I thought I should throw
in my opinion here before it is too late to do anything about it.

I believe it is a mistake to model file_time_type with 128 bits.  It would be
acceptable if this was absolutely necessary to get the job done, but it isn’t. 
The 16 byte integer is unnecessarily expensive to get the job done.

file_time_type does not need to model the full range and precision of timespec
(which on 64 bit platforms is a 128 bit type).  All file_time_type needs to
model is the full range and precision of what the underlying file system
libraries are capable of producing.

The latest Linux file system is ext4 (https://en.wikipedia.org/wiki/Ext4) and is
capable of nanosecond resolution. However its timestamp is only 64 bits.  It has
a range of approximately [1901-12-14, 2446-05-10].  Modeling ext4 would be a
good design decision for libc++.  libc++ could also model other file systems
(Windows, macOS).  All of these are based on 64 bit timestamps.

Here is a file_clock, quickly thrown together, lightly tested, that models ext4:

    #include "date/tz.h"
    #include <ostream>
    #include <istream>

    namespace filesystem
    {

    struct file_clock
    {
        using duration                  = std::chrono::nanoseconds;
        using rep                       = duration::rep;
        using period                    = duration::period;
        using time_point                =
std::chrono::time_point<file_clock>;
        static constexpr bool is_steady = false;

        static time_point now();

        template<typename Duration>
        static
        std::chrono::time_point<std::chrono::system_clock, Duration>
        to_sys(const std::chrono::time_point<file_clock, Duration>& t)
noexcept;

        template<typename Duration>
        static
        std::chrono::time_point<file_clock, Duration>
        from_sys(const std::chrono::time_point<std::chrono::system_clock,
Duration>& t) noexcept;

        template<typename Duration>
        static
        std::chrono::time_point<date::local_t, Duration>
        to_local(const std::chrono::time_point<file_clock, Duration>&
t) noexcept;

        template<typename Duration>
        static
        std::chrono::time_point<file_clock, Duration>
        from_local(const std::chrono::time_point<date::local_t,
Duration>& t) noexcept;

        // private helpers

        static
        timespec
        to_timespec(const time_point& t) noexcept;

        static
        time_point
        from_timespec(const timespec& t) noexcept;
    };

    template <class Duration>
        using file_time = std::chrono::time_point<file_clock, Duration>;

    using file_time_type = file_clock::time_point;

    template <class Duration>
    inline
    std::chrono::time_point<std::chrono::system_clock, Duration>
    file_clock::to_sys(const std::chrono::time_point<file_clock,
Duration>& t) noexcept
    {
        using namespace date;
        return sys_time<Duration>{t.time_since_epoch()} +
                                 (sys_days{2174_y/1/1} - sys_days{1970_y/1/1});
    }

    template <class Duration>
    inline
    std::chrono::time_point<file_clock, Duration>
    file_clock::from_sys(const
std::chrono::time_point<std::chrono::system_clock, Duration>& t)
noexcept
    {
        using namespace date;
        return file_time<Duration>{t.time_since_epoch()} -
                                  (sys_days{2174_y/1/1} - sys_days{1970_y/1/1});
    }

    template <class Duration>
    inline
    std::chrono::time_point<date::local_t, Duration>
    file_clock::to_local(const std::chrono::time_point<file_clock,
Duration>& t) noexcept
    {
        using namespace date;
        return local_time<Duration>{to_sys(t).time_since_epoch()};
    }

    template <class Duration>
    inline
    std::chrono::time_point<file_clock, Duration>
    file_clock::from_local(const std::chrono::time_point<date::local_t,
Duration>& t) noexcept
    {
        using namespace date;
        return
file_time<Duration>{from_sys(sys_time<Duration>{t.time_since_epoch()})};
    }

    file_clock::time_point
    file_clock::now()
    {
        return from_sys(std::chrono::system_clock::now());
    }

    template <class CharT, class Traits, class Duration>
    std::basic_ostream<CharT, Traits>&
    to_stream(std::basic_ostream<CharT, Traits>& os, const CharT* fmt,
              const file_time<Duration>& t)
    {
        using namespace std::chrono;
        const std::string abbrev("UTC");
        constexpr std::chrono::seconds offset{0};
        using D128 = duration<__int128, typename Duration::period>;
        return date::to_stream(os, fmt,
file_clock::to_local(time_point_cast<D128>(t)),
                               &abbrev, &offset);
    }

    template <class Duration, class CharT, class Traits, class Alloc =
std::allocator<CharT>>
    std::basic_istream<CharT, Traits>&
    from_stream(std::basic_istream<CharT, Traits>& is, const CharT*
fmt,
                file_time<Duration>& tp,
                std::basic_string<CharT, Traits, Alloc>* abbrev = nullptr,
                std::chrono::minutes* offset = nullptr)
    {
        using namespace date;
        using namespace std::chrono;
        using D128 = duration<__int128, typename Duration::period>;
        local_time<D128> lp;
        from_stream(is, fmt, lp, abbrev, offset);
        if (!is.fail())
            tp = file_clock::from_local(lp);
        return is;
    }

    template <class CharT, class Traits, class Duration>
    std::basic_ostream<CharT, Traits>&
    operator<<(std::basic_ostream<CharT, Traits>& os, const
file_time<Duration>& t)
    {
        const CharT fmt[] = {'%', 'F', ' ', '%',
'T', CharT{}};
        return to_stream(os, fmt, t);
    }

    inline
    timespec
    file_clock::to_timespec(const time_point& t) noexcept
    {
        using namespace date;
        using namespace std::chrono;
        auto tp = to_sys(time_point_cast<std::chrono::duration<__int128,
std::nano>>(t));
        auto s = floor<seconds>(tp);
        timespec ts;
        ts.tv_sec =
static_cast<decltype(ts.tv_sec)>(s.time_since_epoch().count());
        ts.tv_nsec = static_cast<decltype(ts.tv_nsec)>((tp - s).count());
        return ts;
    }

    inline
    file_clock::time_point
    file_clock::from_timespec(const timespec& t) noexcept
    {
        using namespace date;
        using namespace std::chrono;
        auto d = std::chrono::duration<__int128>{t.tv_sec} +
nanoseconds{t.tv_nsec};
        return
time_point_cast<duration>(from_sys(sys_time<decltype(d)>{d}));
    }

    }  // namespace filesystem

    #include <iostream>
    #include <sstream>

    int
    main()
    {
        using namespace std;
        using namespace date;
        std::cout << filesystem::file_clock::time_point::min() <<
'\n';
        std::cout << filesystem::file_clock::now() << '\n';
        std::cout << filesystem::file_clock::time_point::max() <<
'\n';
        std::istringstream in{"2466-04-11 23:47:16.854775807"};
        filesystem::file_clock::time_point tp;
        in >> date::parse("%F %T", tp);
        cout << tp << '\n';
        in.clear();
        in.str("1881-09-22 00:12:43.145224192");
        in >> date::parse("%F %T", tp);
        cout << tp << '\n';
        timespec ts = {15661036036, 854775807};  // or {-2785708037, 145224192}
        tp = filesystem::file_clock::from_timespec(ts);
        cout << tp << '\n';
        ts = filesystem::file_clock::to_timespec(tp);
        cout << "{" << ts.tv_sec << ", "
<< ts.tv_nsec << "}\n";
        using s32 = chrono::duration<int>;
        using ns64 = chrono::duration<long, nano>;
        using uns64 = chrono::duration<unsigned long, nano>;
        using ns128 = chrono::duration<__int128, nano>;
        cout << date::sys_time<s32>::min() + ns128{uns64::max()}
<< '\n';
        cout << date::sys_time<s32>::min() + ns128{ns64::max()} +
1ns << '\n';
    }

It is a 64bit timestamp with nanosecond resolution that is capable of
representing a superset of ext4 (about +/- 20 years on either side of the limits
of ext4).  It _does_ internally use __int128 for a few intermediate computations
such as converting to/from a timespec.  This allows it avoid overflow out near
min()/max().  However, the most common operations users will encounter are
simply the arithmetic involving time_point and duration, and these are strictly
64 bit (and already provided by chrono).

This is what I advise for the libc++ filesystem library.  I have also sent this
code to the gcc developers.  Consider it public domain.

Howard

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: Message signed with OpenPGP
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20180810/e87f8564/attachment.sig>