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

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>

I only saw this now after the llvm 7 branch was cut.

It would be good to get this all figured out before the release :-)

On Mon, Jul 30, 2018 at 10:19 PM, Louis Dionne via llvm-dev
<llvm-dev at lists.llvm.org> wrote:
> 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> 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
> _______________________________________________
> cfe-dev mailing list
> cfe-dev at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev
>
>
> _______________________________________________
> cfe-dev mailing list
> cfe-dev at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev
>
>
>
> _______________________________________________
> LLVM Developers mailing list
> llvm-dev at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
>

Hi,

My current understanding of the problem (based on
https://reviews.llvm.org/D49774) is that we have a type, file_time_type, which
is part of the ABI and is currently defined as
std::chrono::time_point<_FileSystemClock>, where _FileSystemClock is an
internal type represented using a __int128_t. However, C++20 will add a type
called file_clock and redefine file_time_type to be
std::chrono::time_point<std::chrono::file_clock> instead, which is an ABI
break.

Is this correct, and is this the only concern we have with respect to the ABI
stability of Filesystem as currently included in the release branch for LLVM 7?

If that is correct, then we can either
(1) define file_time_type in a way that is forward-ABI-compatible with the
definition that will be used in C++20 and patch LLVM 7 accordingly
(2) do nothing and live with an ABI break in C++20
(3) revert r338093 (and perhaps some patches that depend on it) in the LLVM 7
branch, and keep shipping Filesystem as being experimental

(1) is ideal, but we have to confirm that my understanding is correct AND agree
on a fix, soon.
(2) is a no-go.
(3) is the safest and simplest option.

If we can't get a solution for (1) going very soon, I would like to request
that we implement (3) and remove Filesystem from the non-experimental namespace
in the LLVM 7 branch. This is unfortunate, but we're currently running
towards (2), which is desirable for nobody.

I suggest a deadline of Friday August 10th to have settled on a solution for (1)
so as to give us time to implement it and review it in time for RC 2, on August
22nd.

Louis

> On Aug 2, 2018, at 03:46, Hans Wennborg <hans at chromium.org> wrote:
> 
> I only saw this now after the llvm 7 branch was cut.
> 
> It would be good to get this all figured out before the release :-)
> 
> On Mon, Jul 30, 2018 at 10:19 PM, Louis Dionne via llvm-dev
> <llvm-dev at lists.llvm.org> wrote:
>> 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> 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
>> _______________________________________________
>> cfe-dev mailing list
>> cfe-dev at lists.llvm.org
>> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev
>> 
>> 
>> _______________________________________________
>> cfe-dev mailing list
>> cfe-dev at lists.llvm.org
>> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev
>> 
>> 
>> 
>> _______________________________________________
>> LLVM Developers mailing list
>> llvm-dev at lists.llvm.org
>> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
>>