[libc++] LWG3171: implement operator<< for filesystem::directory_entry.

[var-const]

libcxx/include/__filesystem/directory_entry.h

26

From what I've seen, other headers that implement operator<< include <iosfwd> -- at the very least, I couldn't find one that includes <ostream>. However, I'm not sure this is actually valid given that the code actually invokes a member function on the stream (operator<<) whereas <iosfwd> only contains a forward declaration of the class.

libcxx/test/std/input.output/filesystems/class.directory_entry/directory_entry.io/directory_entry.io.pass.cpp

15

This test is largely copied from test/std/input.output/filesystems/class.path/path.nonmember/path.io.pass.cpp.

51

See https://wandbox.org/permlink/hUEUX67Ejcgxz6UT.

[• Quuxplusone]

Please mention [LWG3171] in the commit subject line, and update Cxx2bIssues.csv. Otherwise, seems reasonable to me % comments.

libcxx/include/__filesystem/directory_entry.h

26

I believe it's technically OK, because operator<< is always a template: it's calling a member function on some basic_ostream<C, T>, but that's a dependent type, so the call doesn't need to be (can't be) checked beyond that.
I strongly recommend being consistent [;)] which means including <iosfwd> here, not <ostream>.

libcxx/include/filesystem

48–51 ↗

(On Diff #397454)

The extra // comment here was the red flag for me; but actually I think this entire diff is unnecessary. This operator<< is specified inside the body of class directory_entry; the body of directory_entry is not shown here; therefore this operator<< needn't be shown here.
So I believe no change is needed anywhere in this particular file.

libcxx/test/std/input.output/filesystems/class.directory_entry/directory_entry.io/directory_entry.io.pass.cpp

51

path.io.pass.cpp (c79795874adef) and ostream_joiner.op.assign.pass.cpp (e046bbdded2c6) seem to be the only places in the codebase where we bother with this kind of commented-out char{8,16,32}_t business. I'm not sure it's worth duplicating here... but, your example seems to indicate that libc++'s lack of support for stringstream<char16_t> is actually a bug? libstdc++ and MSVC support char16_t streams reasonably well AFAICT. So perhaps this should be filed as a bug against libc++, and then you can explicitly mention https://llvm.org/PRxxxxx both here and in those two other places?

[ldionne]

libcxx/include/filesystem

48–51 ↗

(On Diff #397454)

I'd agree with this, unless we want to detail all of class directory_entry in the synopsis, which might not be a bad idea. No strong preference here -- I don't think we have consistency about this in the library (but we seem to consistently *not* detail the classes in this header).

[var-const]

Address feedback.

[var-const]

In D116642#3225747, @Quuxplusone wrote:

Please mention [LWG3171] in the commit subject line, and update Cxx2bIssues.csv. Otherwise, seems reasonable to me % comments.

Thanks! The version is 14.0, right?

libcxx/include/__filesystem/directory_entry.h

26

Thanks for the explanation, changed to <iosfwd>.

libcxx/test/std/input.output/filesystems/class.directory_entry/directory_entry.io/directory_entry.io.pass.cpp

51

Yes, I think it's a bug (it happens with all sized character types and even unsigned char). Created a GitHub issue and referenced it in comments.

[• Quuxplusone]

libcxx/docs/Status/Cxx2bIssues.csv

20

libcxx/test/std/experimental/iterator/ostream.joiner/ostream.joiner.ops/ostream_joiner.op.assign.pass.cpp

93

Please use https://llvm.org/PRxxxxx URLs.

[var-const]

Address feedback

libcxx/docs/Status/Cxx2bIssues.csv

20

Sorry about that!

libcxx/test/std/experimental/iterator/ostream.joiner/ostream.joiner.ops/ostream_joiner.op.assign.pass.cpp

93

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?

[• Quuxplusone]

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

libcxx/test/std/experimental/iterator/ostream.joiner/ostream.joiner.ops/ostream_joiner.op.assign.pass.cpp

93

https://llvm.org/PR53119 . In LLVM jargon, "PR" stands for "problem report" (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.

libcxx/test/std/input.output/filesystems/class.directory_entry/directory_entry.io/directory_entry.io.pass.cpp

40

Waaaaitaminute. This is <<ing a path, but aren't you supposed to be <<ing a directory_entry here?

[var-const]

Fix the test.

[var-const]

libcxx/test/std/input.output/filesystems/class.directory_entry/directory_entry.io/directory_entry.io.pass.cpp

40

Thanks a lot!

[• Quuxplusone]

LGTM % comments; please wait for a second green light before landing.

libcxx/test/std/experimental/iterator/ostream.joiner/ostream.joiner.ops/ostream_joiner.op.assign.pass.cpp

93

This hasn't been done yet.

libcxx/test/std/input.output/filesystems/class.directory_entry/directory_entry.io/directory_entry.io.pass.cpp

37

Consider const-qualifying (just to prove that operator<<'s operand is const-qualified), and prefer parens-init to brace-init for non-aggregates:

const fs::directory_entry dir = fs::directory_entry(fs::path(input));

(I also think this is the only line that needs fs:: qualifiers, so if you adopt the above suggestion, you can remove lines 32-33 entirely.)

[var-const]

• change GitHub links to LLVM links;
• make the directory_entry used in tests const.

[var-const]

libcxx/test/std/experimental/iterator/ostream.joiner/ostream.joiner.ops/ostream_joiner.op.assign.pass.cpp

93

Sorry, I somehow missed this comment. Done now, and thanks for the explanation.

libcxx/docs/Status/Cxx2bIssues.csv

 "`3450 <https://wg21.link/LWG3450>`__","The const overloads of ``take_while_view::begin/end`` are underconstrained","November 2020","","","|ranges|"
 "`3464 <https://wg21.link/LWG3464>`__","``istream::gcount()`` can overflow","November 2020","",""
 "`2731 <https://wg21.link/LWG2731>`__","Existence of ``lock_guard<MutexTypes...>::mutex_type`` typedef unclear","November 2020","|Complete|","5.0"
 "`2743 <https://wg21.link/LWG2743>`__","P0083R3 ``node_handle`` private members missing ""exposition only"" comment","November 2020","",""
 "`2820 <https://wg21.link/LWG2820>`__","Clarify ``<cstdint>`` macros","November 2020","",""
 "`3120 <https://wg21.link/LWG3120>`__","Unclear behavior of ``monotonic_buffer_resource::release()``","November 2020","",""
 "`3170 <https://wg21.link/LWG3170>`__","``is_always_equal`` added to ``std::allocator`` makes the standard library treat derived types as always equal","November 2020","",""
 "`3036 <https://wg21.link/LWG3036>`__","``polymorphic_allocator::destroy`` is extraneous","November 2020","",""
-"`3171 <https://wg21.link/LWG3171>`__","LWG2989 breaks ``directory_entry`` stream insertion","November 2020","",""
+"`3171 <https://wg21.link/LWG3171>`__","LWG2989 breaks ``directory_entry`` stream insertion","November 2020","|Complete|","14.0"
 "`3306 <https://wg21.link/LWG3306>`__","``ranges::advance`` violates its preconditions","November 2020","","","|ranges|"
 "`3403 <https://wg21.link/LWG3403>`__","Domain of ``ranges::ssize(E)`` doesn't ``match ranges::size(E)``","November 2020","","","|ranges|"
 "`3404 <https://wg21.link/LWG3404>`__","Finish removing subrange's conversions from pair-like","November 2020","","","|ranges|"
 "`3405 <https://wg21.link/LWG3405>`__","``common_view``'s converting constructor is bad, too","November 2020","","","|ranges|"
 "`3406 <https://wg21.link/LWG3406>`__","``elements_view::begin()`` and ``elements_view::end()`` have incompatible constraints","November 2020","","","|ranges|"
 "`3419 <https://wg21.link/LWG3419>`__","[algorithms.requirements]/15 doesn't reserve as many rights as it intends to","November 2020","",""
 "`3420 <https://wg21.link/LWG3420>`__","cpp17-iterator should check that the type looks like an iterator first","November 2020","|Complete|","14.0","|ranges|"
 "`3421 <https://wg21.link/LWG3421>`__","Imperfect ADL emulation for boolean-testable","November 2020","","","|ranges|"

libcxx/include/__filesystem/directory_entry.h

 #include <__filesystem/file_type.h>
 #include <__filesystem/filesystem_error.h>
 #include <__filesystem/operations.h>
 #include <__filesystem/path.h>
 #include <__filesystem/perms.h>
 #include <chrono>
 #include <cstdint>
 #include <cstdlib>
+#include <iosfwd>
 #include <system_error>
 
 _LIBCPP_PUSH_MACROS
 #include <__undef_macros>
 
 #ifndef _LIBCPP_CXX03_LANG
 
 _LIBCPP_BEGIN_NAMESPACE_FILESYSTEM
bool operator>(directory_entry const& __rhs) const noexcept {
     return __p_ > __rhs.__p_;
   }
 
   _LIBCPP_INLINE_VISIBILITY
   bool operator>=(directory_entry const& __rhs) const noexcept {
     return __p_ >= __rhs.__p_;
   }
 
+  template <class _CharT, class _Traits>
+  _LIBCPP_INLINE_VISIBILITY
+  friend basic_ostream<_CharT, _Traits>& operator<<(basic_ostream<_CharT, _Traits>& __os, const directory_entry& __d) {
+    return __os << __d.path();
+  }
+
 private:
   friend class directory_iterator;
   friend class recursive_directory_iterator;
   friend class __dir_stream;
 
   enum _CacheType : unsigned char {
     _Empty,
     _IterSymlink,

libcxx/test/std/experimental/iterator/ostream.joiner/ostream.joiner.ops/ostream_joiner.op.assign.pass.cpp

int main(int, char**) {
 
     test("Z", chars, chars+10, "0Z1Z2Z3Z4Z5Z6Z7Z8Z9");
     test("z", ints,  ints+10,  "10z11z12z13z14z15z16z17z18z19");
 
 #ifndef TEST_HAS_NO_WIDE_CHARACTERS
     test<char, const char *, wchar_t> ('X', chars, chars+10, L"0X1X2X3X4X5X6X7X8X9");
     test<char, const int *,  wchar_t> ('x',  ints,  ints+10, L"10x11x12x13x14x15x16x17x18x19");
 #endif
+    // TODO(var-const): uncomment when it becomes possible to instantiate a `basic_ostream` object with a sized
+    // character type (see https://llvm.org/PR53119).
 //  test<char, const char *, char16_t>('X', chars, chars+10, u"0X1X2X3X4X5X6X7X8X9");
 //  test<char, const int *,  char16_t>('x',  ints,  ints+10, u"10x11x12x13x14x15x16x17x18x19");
 //  test<char, const char *, char32_t>('X', chars, chars+10, U"0X1X2X3X4X5X6X7X8X9");
 //  test<char, const int *,  char32_t>('x',  ints,  ints+10, U"10x11x12x13x14x15x16x17x18x19");
 
     test(mutating_delimiter(),  chars, chars+10, "0 1!2\"3#4$5%6&7'8(9");
     test(mutating_delimiter2(), chars, chars+10, "0 1!2\"3#4$5%6&7'8(9");
     }

libcxx/test/std/input.output/filesystems/class.directory_entry/directory_entry.io/directory_entry.io.pass.cpp

+//===----------------------------------------------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+// UNSUPPORTED: c++03
+// UNSUPPORTED: libcpp-has-no-localization
+
+// <filesystem>
+//
+// class directory_entry
+//
+// template<class charT, class traits>
+//   friend basic_ostream<charT, traits>&
+//     operator<<(basic_ostream<charT, traits>& os, const directory_entry& d);
+
+#include "filesystem_include.h"
+#include <cassert>
+#include <sstream>
+
+#include "test_macros.h"
+#include "make_string.h"
+
+MultiStringType InStr =  MKSTR("abcdefg/\"hijklmnop\"/qrstuvwxyz/123456789");
+MultiStringType OutStr = MKSTR("\"abcdefg/\\\"hijklmnop\\\"/qrstuvwxyz/123456789\"");
+
+template <class CharT>
+void TestOutput() {
+  const char* input = static_cast<const char*>(InStr);
+  const CharT* expected_output = static_cast<const CharT*>(OutStr);
+  const fs::directory_entry dir = fs::directory_entry(fs::path(input));
+  std::basic_stringstream<CharT> stream;
+
+  auto& result = stream << dir;
+  assert(stream.str() == expected_output);
+  assert(&result == &stream);
+}
+
+int main(int, char**) {
+  TestOutput<char>();
+#ifndef TEST_HAS_NO_WIDE_CHARACTERS
+  TestOutput<wchar_t>();
+#endif
+  // TODO(var-const): uncomment when it becomes possible to instantiate a `basic_ostream` object with a sized character
+  // type (see https://llvm.org/PR53119).
+  //TestOutput<char8_t>();
+  //TestOutput<char16_t>();
+  //TestOutput<char32_t>();
+
+  return 0;
+}

libcxx/test/std/input.output/filesystems/class.path/path.nonmember/path.io.pass.cpp

 #endif
 }
 
 int main(int, char**) {
   doIOTest<char>();
 #ifndef TEST_HAS_NO_WIDE_CHARACTERS
   doIOTest<wchar_t>();
 #endif
+  // TODO(var-const): uncomment when it becomes possible to instantiate a `basic_ostream` object with a sized character
+  // type (see https://llvm.org/PR53119).
   //doIOTest<char16_t>();
   //doIOTest<char32_t>();
   test_LWG2989();
 
   return 0;
 }