[PATCH] D49774: [libc++] Use __int128_t to represent file_time_type.

[Louis Dionne via Phabricator via cfe-commits]

ldionne added a comment.

In https://reviews.llvm.org/D49774#1174588, @EricWF wrote:

> In https://reviews.llvm.org/D49774#1174565, @mclow.lists wrote:
>
> > I haven't reviewed this closely, but you might want to look at http://wg21.link/P0355, where we added a `file_clock` and `file_time` types.
>
>
> Thanks for the information. It doesn't look to have too much bearing on this from a design standpoint. Except to note that it seems to solve
>  the streaming problem by adding explicit streaming overload for time points.


It doesn't change anything from a design standpoint, but I don't think we can ship something deemed stable without taking P0355 into account. That's because it adds `file_clock` and changes `file_time_type` to use it, which is an ABI break. So if we take `filesystem` out of `experimental` and ship it before we've actually implemented the parts of P0355 that change the ABI, we'll have to take an ABI break in the future. That would suck because this ABI break would be necessary to implement C++20 properly in a fairly major way.

That's my understanding. Appart from that, I've read the design docs and while I'm not very familiar with the problems involved, I follow the reasoning and I think using `__int128_t` makes at least as much sense as the other potential solutions.

LGTM


Repository:
  rCXX libc++

https://reviews.llvm.org/D49774