Cleanups and enhancements for basic_string#3984
Cleanups and enhancements for basic_string#3984StephanTLavavej merged 6 commits intomicrosoft:mainfrom
basic_string#3984Conversation
|
|
||
| _Elem* const _Old_ptr = _My_data._Myptr(); | ||
| size_type _New_capacity = _Calculate_growth(_My_data._Mysize); | ||
| size_type _New_capacity = _Calculate_growth(_My_data._Mysize + 1); |
There was a problem hiding this comment.
This is not practically behavior-changing (won't affect resulting value), but I think it will be clearer to +1, as this is the only place that use _Mysize as input, and _Calculate_growth accept the first argument as _Requested, and this will be in line with push_back (using _Reallocate_grow_by(1, ...) which does _Calculate_growth(_My_data._Mysize + 1,...).
I'm neutral about +1 change, as it might slightly affect codegen; a comment about why to use _My_data._Mysize (just to do x1.5 evaluation) might be better.
There was a problem hiding this comment.
I think + 1 makes sense here.
|
|
|
||
| const bool _My_large = _My_data._Large_mode_engaged(); | ||
| const bool _Right_large = _Right_data._Large_mode_engaged(); | ||
|
|
There was a problem hiding this comment.
Line 4272 in 60f1885
Is it encouraged to move this
using _STD swap into the if block where it is used? I'm not sure as I find using _STD swap is usually defined at the beginning of functions.
There was a problem hiding this comment.
Making it as local as possible seems like an improvement.
Not sure whether we should remove the strengthening, since
See #1035. I think we should say |
|
|
||
| const bool _My_large = _My_data._Large_mode_engaged(); | ||
| const bool _Right_large = _Right_data._Large_mode_engaged(); | ||
|
|
There was a problem hiding this comment.
Making it as local as possible seems like an improvement.
|
|
||
| _Elem* const _Old_ptr = _My_data._Myptr(); | ||
| size_type _New_capacity = _Calculate_growth(_My_data._Mysize); | ||
| size_type _New_capacity = _Calculate_growth(_My_data._Mysize + 1); |
There was a problem hiding this comment.
I think + 1 makes sense here.
|
All of these cleanups look great - thanks! |
|
I have enough confidence in these cleanups to merge them without a second maintainer approval. |
|
I'm mirroring this to the MSVC-internal repo - please notify me if any further changes are pushed. |
🧵 🧶 🧹 |
_Memcpy_val_fromto its only caller.basic_string(initializer_list)basic_string(&&, off). The rvalue will be unchanged if exception is thrown from_Alloc_proxy().noexceptenhancements, mainly for comparisons with c-string.