[libcxx-commits] [PATCH] D116642: [libc++] LWG3171: implement operator<< for filesystem::directory_entry.

[Arthur O'Dwyer via Phabricator via libcxx-commits]

Quuxplusone requested changes to this revision.
Quuxplusone added a comment.
This revision now requires changes to proceed.

LGTM //except// for my comment on the test, which is major.



================
Comment at: libcxx/test/std/experimental/iterator/ostream.joiner/ostream.joiner.ops/ostream_joiner.op.assign.pass.cpp:93
+    // TODO(var-const): uncomment when it becomes possible to instantiate a `basic_ostream` object with a sized
+    // character type (see https://github.com/llvm/llvm-project/issues/53119).
 //  test<char, const char *, char16_t>('X', chars, chars+10, u"0X1X2X3X4X5X6X7X8X9");
----------------
var-const wrote:
> Quuxplusone wrote:
> > Please use https://llvm.org/PRxxxxx URLs.
> Can you please elaborate a little? First of all, which patch do I use? IIUC, libc++ bugs are reported on GitHub, so I'm not sure which patch should be used here. Second, why is a link to a patch preferable to a link to a GitHub issue?
https://llvm.org/PR53119 . In LLVM jargon, ["PR" stands for "problem report"](https://quuxplusone.github.io/blog/2019/08/02/the-tough-guide-to-cpp-acronyms/#pr) (or some such). The benefit of `llvm.org` URLs over `github.com` URLs is that we control the former, so they're less likely to bit-rot ten years from now.


================
Comment at: libcxx/test/std/input.output/filesystems/class.directory_entry/directory_entry.io/directory_entry.io.pass.cpp:39
+
+  auto& result = stream << p;
+  assert(stream.str() == expected_output);
----------------
Waaaaitaminute. This is `<<`ing a `path`, but aren't you supposed to be `<<`ing a `directory_entry` here?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116642