[SERVER-32422] convert StringData to std::string_view Created: 20/Dec/17  Updated: 02/Nov/23

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

Type: Improvement Priority: Minor - P4
Reporter: Billy Donahue Assignee: Backlog - Service Architecture
Resolution: Unresolved Votes: 0
Labels: sa-remove-fv-backlog-22, techdebt
Σ Remaining Estimate: Not Specified Remaining Estimate: Not Specified
Σ Time Spent: Not Specified Time Spent: Not Specified
Σ Original Estimate: Not Specified Original Estimate: Not Specified

Issue Links:
Related
related to SERVER-55180 Remove StringData <=> std::string_vie... Closed
is related to SERVER-48429 Remove StringData::ComparatorInterfac... Closed
Sub-Tasks:
Key
Summary
Type
Status
Assignee
SERVER-48429 Remove StringData::ComparatorInterfac... Sub-task Closed Billy Donahue  
SERVER-32434 operator<<(ostream,StringData) ignore... Sub-task Closed Billy Donahue  
SERVER-82604 Add missing std::string_view members ... Sub-task Closed Billy Donahue  
Assigned Teams:
Service Arch
Participants:

 Description   

I want to collect some notes on differences in StringData that could complicate a migration to std::string_view.

The BIG one: as of its introduction in C++17, std::string_view(cp) is UB for null-valued cp, but this is tolerated for StringData. This will be very difficult to audit for, as this is an implicit constructor.

Renames:

  • rawData() should be called data().
  • startsWith,endsWith (slated for C++20 as starts_with and ends_with).

Several public members must be brought outside the class:

  • The range constructor.
  • toString.
  • (SERVER-48429) StringData::ComparatorInterface.
  • equalCaseInsensitive
  • copyTo

Behavior:
Exception behavior should match. std::string_view throws std::out_of_range.
Noexcept and constexpr annotations should be aligned with string_view.

  • char operator[](unsigned) should take size_type and return const_reference.

Needs more members:

  • Several typedefs need to be added.
  • many more find/rfind variants, and these need offset parameters.
  • remove_prefix / remove_suffix
  • string_view has its own npos
  • (SERVER-38248) explicit operator std::string instead of .toString, to approximate the string(string_view) ctor.
  • member swap
  • at, front, back, max_size, length
  • (c?r?)(begin|end) iterator accessors.
  • `explicit operator std::string() const`

Apparatus:



 Comments   
Comment by Billy Donahue [ 02/Nov/23 ]

Our macOS builds are using XCode 13.2.1 (13 Dec 2021) which doesn't have a great std::string_view.
It's missing some C++20 features:

  • No range constructors
  • No operator<=>
Comment by Billy Donahue [ 01/Nov/23 ]

Another complication. On Windows, the std::string_view::iterator type is a class, and it is NOT const char*.

Unsurprisingly we have some code that breaks from this.

[2023/11/01 04:59:10.179] C:\data\mci\60fe99ce1a7121b33f58df8d31ceb8af\src\src\mongo/bson/bsonelementvalue.h(151,42): error C2672: 'mongo::OID::from': no matching overloaded function found
[2023/11/01 04:59:10.179]         return BSONDBRef(ns, mongo::OID::from(ns.end() + kStringTerminatorBytes));
[2023/11/01 04:59:10.179]                                          ^

The simplest fix is to replace
s.begin() => s.data()
and replace
s.end() => s.data() + s.size()

But this is giving up on some of the pretty intense debug-mode instrumentation provided by this iterator class.
( See https://github.com/microsoft/STL/blob/main/stl/inc/xstring#L900 )

Of course, we're not getting any of that instrumentation with StringData today but it would be nice to get it as a side effect of the std::string_view migration.

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