[SERVER-54352] StringData should offer std::string += StringData overload. Created: 05/Feb/21 Updated: 29/Oct/23 Resolved: 18/Mar/22 |
|
| Status: | Closed |
| Project: | Core Server |
| Component/s: | Internal Code |
| Affects Version/s: | None |
| Fix Version/s: | 6.0.0-rc0 |
| Type: | Improvement | Priority: | Major - P3 |
| Reporter: | Mathias Stearn | Assignee: | Mathias Stearn |
| Resolution: | Fixed | Votes: | 0 |
| Labels: | mathias-sa-triage | ||
| Remaining Estimate: | Not Specified | ||
| Time Spent: | Not Specified | ||
| Original Estimate: | Not Specified | ||
| Issue Links: |
|
||||||||
| Backwards Compatibility: | Fully Compatible | ||||||||
| Participants: | |||||||||
| Description |
|
We already offer + overloads, but a += overload would be more natural and more efficient in some cases. |
| Comments |
| Comment by Githook User [ 18/Mar/22 ] | ||
|
Author: {'name': 'Mathias Stearn', 'email': 'redbeard0531@gmail.com', 'username': 'RedBeard0531'}Message: | ||
| Comment by Mathias Stearn [ 19/Mar/21 ] | ||
|
billy.donahue Given the | ||
| Comment by Billy Donahue [ 13/Mar/21 ] | ||
|
Some people recently got together and added implicit conversions in both directions to and from std::string_view to StringData, but this change wasn't reviewed or tested and the implications are subtle.
| ||
| Comment by Billy Donahue [ 13/Mar/21 ] | ||
|
Ok, Thanks. I don't know why I thought that. Maybe it was a style rule I'd internalized as a language rule. | ||
| Comment by Mathias Stearn [ 13/Mar/21 ] | ||
|
That is not true. The restriction you mention only applies to simple assignment: https://en.cppreference.com/w/cpp/language/operator_assignment. I have this overload in my branch and it works nicely https://github.com/10gen/mongo/blob/mathias/columns/src/mongo/base/string_data.h#L335. PS jira usually supports backslash escaping | ||
| Comment by Billy Donahue [ 13/Mar/21 ] | ||
(Jira is chewing up the C++ operators as markup because it's lame). But I think this sort of thing is a reason to move to std::string_view. |