[libcxx-commits] [PATCH] D96664: [libc++][format] Implement formatters.

[Louis Dionne via Phabricator via libcxx-commits]

ldionne accepted this revision.
ldionne added inline comments.
This revision is now accepted and ready to land.


================
Comment at: libcxx/test/std/utilities/format/format.functions/format_to_n.pass.cpp:11
+// UNSUPPORTED: libcpp-has-no-incomplete-format
+// UNSUPPORTED: gcc-10, gcc-11
+
----------------
Mordante wrote:
> ldionne wrote:
> > You can remove mentions of `gcc-10` everywhere now.
> > 
> > Also, it would be great to have a comment explaining why this is unsupported on `gcc-11` (in all places where you do that).
> I need to investigate why gcc-11 doesn't work. I've added a TODO to look at it later. I'll need to update one of my systems so I have gcc-11 locally which makes testing easier.
FWIW, you can use our Docker image to match the CI environment if you'd like:

```
./libcxx/utils/ci/run-buildbot-container # this will run libc++'s Docker image that we use in CI, which contains GCC 11 amongst other things
$ ./libcxx/utils/ci/run-buildbot generic-gcc # run this from the Docker image
```


================
Comment at: libcxx/test/std/utilities/format/format.functions/tests.inc:1
+//===----------------------------------------------------------------------===//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
----------------
Mordante wrote:
> This file is obsolete and should be removed. I prefer to keep this file a little bit longer. This will make it a lot easier to migrate the other patches in the series to the new `format_tests.h`. Once that migration is done this file can be removed.
It's very unusual to approve patches with these sorts of artifacts, let's make this an exception.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96664