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

Thu Aug 9 20:50:15 PDT 2018

Well, technically <experimental/filesystem> tracks one specification,
<filesystem> another. I normally advocate for libc++'s <experimental/foo>
to act like the specification for <foo>, but not all STLs do that.

Therefore, the purpose of the change was to allow users to write portable
code across multiple STL's using <filesystem> (minus the changes to the
build system needed to link it).

/Eric

On Thu, Aug 9, 2018 at 2:10 PM James Y Knight <jyknight at google.com> wrote:

> Why did you want the symbols moved out of libc++experimental, and for the
> header to be moved from <experimental/filesystem> to <filesystem>? It
> certainly seems like it'd be safer and clearer to move them back to the old
> locations, but it's not clear to me if that'd be trading off something else
> of value.
>
> Was there some other greater purpose served by the change in location,
> which will be hindered by moving them back where they were until
> ABI-stability can be promised?
>
>
>
>
> On Thu, Aug 9, 2018 at 1:54 PM Eric Fiselier via llvm-dev <
> llvm-dev at lists.llvm.org> wrote:
>
>> This isn't about C++20 and the <chrono> changes. I have no doubt any ABI
>> breaking changes W.R.T. that will be addressed.
>>
>> During my work finishing up filesystem, I discovered/filed a number of
>> issues with ABI breaking concerns, in particular around directory_entry,
>> but perhaps other places as well (recursive_directory_iterator, various
>> calls in [fs.op.funcs]). These issues have not yet been addressed in the
>> standard, and these spec should be addressed before we commit to an ABI.
>>
>> Note that we ofter put things into namespace std before they're
>> considered ABI stable, but up until now that has been limited to components
>> specified by a standard still in flight. Also note that libstdc++ also
>> hasn't committed to ABI stability for <filesystem> yet. This is the reason
>> we put the symbols in a separate static library.
>>
>> /Eric
>>
>> On Tue, Aug 7, 2018 at 2:15 PM Louis Dionne <ldionne at apple.com> wrote:
>>
>>> 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
>>> >>
>>>
>>> _______________________________________________
>> LLVM Developers mailing list
>> llvm-dev at lists.llvm.org
>> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-dev/attachments/20180809/d191ff89/attachment.html>