convert StringData to std::string_view
(SERVER-32422)
|
|
| 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. |
| Comments |
| Comment by Githook User [ 11/Jul/19 ] | ||||||||||||||
|
Author: {'name': 'Billy Donahue', 'email': 'billy.donahue@mongodb.com', 'username': 'BillyDonahue'}Message: use boost::string_view for operator<< | ||||||||||||||
| Comment by Billy Donahue [ 10/Jul/19 ] | ||||||||||||||
| Comment by Andrew Morrow (Inactive) [ 10/Jul/19 ] | ||||||||||||||
|
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. 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. | ||||||||||||||
| 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:
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:
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:
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:
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 ] | ||||||||||||||