convert StringData to std::string_view (SERVER-32422)

[SERVER-32434] operator<<(ostream,StringData) ignores stream state. Created: 21/Dec/17  Updated: 31/Oct/23  Resolved: 11/Jul/19

Status: Closed
Project: Core Server
Component/s: Portability
Affects Version/s: None
Fix Version/s: 4.3.1

Type: Sub-task Priority: Minor - P4
Reporter: Billy Donahue Assignee: Billy Donahue
Resolution: Fixed Votes: 0
Labels: None
Remaining Estimate: Not Specified
Time Spent: Not Specified
Original Estimate: Not Specified

Backwards Compatibility: Fully Compatible
Participants:

 Description   

ostream << std::setw(5) << expr

behaves differently depending on whether expr is a std::string or a StringData.

libc++'s operator<<(ostream, string_view) is complex, but accessible for reuse.
https://github.com/llvm-mirror/libcxx/blob/0f25cd9/include/ostream#L714



 Comments   
Comment by Githook User [ 11/Jul/19 ]

Author:

{'name': 'Billy Donahue', 'email': 'billy.donahue@mongodb.com', 'username': 'BillyDonahue'}

Message: SERVER-32434 operator<<(ostream,StringData)

use boost::string_view for operator<<
Branch: master
https://github.com/mongodb/mongo/commit/7f89b539052ba710714c2cd09fd7276a293dd6cc

Comment by Billy Donahue [ 10/Jul/19 ]

https://mongodbcr.appspot.com/500050001/

Comment by Andrew Morrow (Inactive) [ 10/Jul/19 ]

billy.donahue -

I think we may have a nice easy way to do this by leveraging the basic_string_view from boost. Please see my comment on https://mongodbcr.appspot.com/179290001/

Reassigning the ticket over to you to see if that works, since you have the unit test in your tree. If so I think we have a quick resolution for this, finally.

Comment by Billy Donahue [ 24/Jun/19 ]

It was discussed whether we could use fmt::basic_string_view instead of std::basic_string_view for streaming, as a workaround.
fmt::basic_string_view doesn't have an operator<< at all.

https://github.com/fmtlib/fmt/blob/master/include/fmt/core.h#L235

Comment by Billy Donahue [ 20/Jun/19 ]

We could also revisit my original implementation to hold us over.
https://mongodbcr.appspot.com/179290001/

Comment by Andrew Morrow (Inactive) [ 20/Jun/19 ]

jesse - Thanks for pursuing that, sorry it didn't work out. I think we should defer this ticket as you suggested. Can you put it back into 'needs scheduling' and we can find its new home at our next triage meeting.

Comment by A. Jesse Jiryu Davis [ 20/Jun/19 ]

I can reproduce the failure locally (macOS 10.13, XCode 10.1), and bumping SCons's -mmacosx-version-min argument from "10.12" to "10.13" has no effect.

In Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/include/c++ the v1/string_view header file has no operator<<. However, there's a v1/experimental/string_view file that does implement operator<<. So, something like this works for XCode 10.1:

// As of Clang 10, operator<<(ostream, string_view) is still in the experimental/string_view header.
// clang-format off
#ifdef __has_include
#if __has_include(<experimental/string_view>)
#include <experimental/string_view>
#define EXPERIMENTAL_STRING_VIEW
#endif
#endif
// clang-format on
 
#ifndef EXPERIMENTAL_STRING_VIEW
#include <string_view>
#endif

However, I patch-built that and it fails with XCode 10.2 on our build farm. The reason is that v1/string_view still has no operator<< and the contents of v1/experimental/string_view are now:

#error "<experimental/string_view> has been removed. Use <string_view> instead."

It seems like XCode was determined not to implement this operator, at least in the 10.x series.

Comment by Andrew Morrow (Inactive) [ 18/Jun/19 ]

jesse - Before we give up, I'd like to try something. Can you reproduce this link error locally? If so, I'd like to know if it still happens if we raise our macOS target minimum. It may be that the required function is missing from libc++ at the macOS 10.12 ABI target, but is present in one of the later ones. If so, we could consider bumping our target macOS minimum for 4.3+.

Comment by A. Jesse Jiryu Davis [ 17/Jun/19 ]

My solution was simple:

inline std::ostream& operator<<(std::ostream& stream, StringData value) {
    return stream << std::string_view(value.rawData(), value.size());
}

That works in GCC and MSVC, but as of clang 10.0, string_view.h doesn't have this operator although it's in the standard:

    template<class charT, class traits>
      basic_ostream<charT, traits>&
        operator<<(basic_ostream<charT, traits>& os,
                   basic_string_view<charT, traits> str);

So we get compilation errors on Mac. acm, should we defer this ticket again and wait longer for our toolchains to catch up to C++17?

Comment by A. Jesse Jiryu Davis [ 13/Jun/19 ]

The previous attempt was deferred until C++17. Now we can use string_view's implementation of I/O manipulators to support StringData very simply.

Comment by Billy Donahue [ 21/Dec/17 ]

https://mongodbcr.appspot.com/179290001/

Generated at Thu Feb 08 04:30:14 UTC 2024 using Jira 9.7.1#970001-sha1:2222b88b221c4928ef0de3161136cc90c8356a66.