[libcxx-commits] [PATCH] D134598: [libc++] Remove availability markup for std::format

[Louis Dionne via Phabricator via libcxx-commits]

ldionne marked an inline comment as done.
ldionne added inline comments.


================
Comment at: libcxx/include/__format/container_adaptor.h:40
 template <class _Adaptor, class _CharT>
-struct _LIBCPP_TEMPLATE_VIS _LIBCPP_AVAILABILITY_FORMAT __formatter_container_adaptor {
+struct _LIBCPP_TEMPLATE_VIS __formatter_container_adaptor {
 private:
----------------
Mordante wrote:
> ldionne wrote:
> > Mordante wrote:
> > > ldionne wrote:
> > > > Mordante wrote:
> > > > > These containers can contain `floating-point` values too.
> > > > As we discussed: yes, but in that case, the code will fail to compile due to the availability markup on the floating point formatter instantiated within. But using a container adaptor without any floating point in it should work, and it shouldn't fail to compile.
> > > I think the only thing that puzzles my then is why not remove the `_LIBCPP_AVAILABILITY_FORMAT` from `libcxx/include/__format/formatter_floating_point.h` and and let that use transitive depend on `to_char(floating-point)`.
> > That's actually a great point. I think I can go one step further and remove `_LIBCPP_AVAILABILITY_FORMAT` altogether. If you use something from `<format>` and it ends up using `std::to_chars(floating-point)`, then so be it, you'll get a compiler error. If not then no compiler error. I think that actually makes the most sense.
> > 
> > If we have a deployment requirement for `std::format` itself, then we can re-add markup.
> I agree, removing it altogether was on my mind too, but I wasn't sure whether it was possible, hence the question regarding the floating_point formatter. I assume you intend to remove them everywhere so I will not review this version further. Please let me know if you want me to review this version first.
> 
> I think I will move the `format_error` destructor to the header, then we won't need an availability macro. I'm not sure whether that will impact Apple users. But let's discuss that at another time.
In the current state of this patch, all `_LIBCPP_AVAILABILITY_FORMAT` should be gone. I think I would like to get this one reviewed and landed and then I will rebase the other patches in the stack on top of it. Does that make sense?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D134598/new/

https://reviews.llvm.org/D134598