[SERVER-55618] ItoA type should also have a named StringData getter Created: 30/Mar/21  Updated: 29/Oct/23  Resolved: 01/Aug/23

Status: Closed
Project: Core Server
Component/s: Internal Code
Affects Version/s: None
Fix Version/s: 7.1.0-rc0

Type: Improvement Priority: Major - P3
Reporter: Mathias Stearn Assignee: Billy Donahue
Resolution: Fixed Votes: 0
Labels: sa-remove-fv-backlog-22
Remaining Estimate: Not Specified
Time Spent: Not Specified
Original Estimate: Not Specified

Assigned Teams:
Service Arch
Backwards Compatibility: Fully Compatible
Sprint: Service Arch 2023-08-07
Participants:

 Description   

Only having a conversion operator makes it awkward to work with the string data in the same scope where you declare the ItoA object and makes it tempting to do the wrong thing:

auto str = ItoA(someNum);
invariant(str.size() < 4); // doesn't compile
invariant(str.operator StringData().size()); // compiles, but gross
invariant(str.sd().size() < 4); // nice, but doesn't compile... yet :)
 
// danger!
// We can't prevent this without also preventing the totally valid f(ItoA(someNum)),
// but we can make it less tempting.
auto str2 = StringData(ItoA(someNum)); 

As an alternative, we could make ItoA be a subclass of StringData so that it just directly presents the string-like API. It is a bit odd, since that would allow assigning through a mutable StringData& (but not through an ItoA& or an ItoA variable), but since that seems to be harmless, maybe its fine?



 Comments   
Comment by Githook User [ 02/Aug/23 ]

Author:

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

Message: SERVER-55618 add ItoA::toStringData
Branch: minh.luu-no_compile_sys-perf
https://github.com/mongodb/mongo/commit/7786c813ded45cb3d728d2c402c22565d2c565c5

Comment by Githook User [ 01/Aug/23 ]

Author:

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

Message: SERVER-55618 add ItoA::toStringData
Branch: master
https://github.com/mongodb/mongo/commit/7786c813ded45cb3d728d2c402c22565d2c565c5

Comment by Billy Donahue [ 01/Aug/23 ]

5 minute ticket

Comment by Billy Donahue [ 30/Mar/21 ]

Sounds fine to add a .toStringData() to supplement the class with explicit syntax for the accessor.
If you want to spend the few minutes writing it and send me a review that would be fine.
The way I'd write that invariant wasn't on the list.

invariant(StringData{str}.size());

Generated at Thu Feb 08 05:36:58 UTC 2024 using Jira 9.7.1#970001-sha1:2222b88b221c4928ef0de3161136cc90c8356a66.