Skip to content

Remove unneeded check if (this != _STD addressof(_Right))#3540

Merged
StephanTLavavej merged 1 commit intomicrosoft:mainfrom
fsb4000:fix3538
Mar 7, 2023
Merged

Remove unneeded check if (this != _STD addressof(_Right))#3540
StephanTLavavej merged 1 commit intomicrosoft:mainfrom
fsb4000:fix3538

Conversation

@fsb4000
Copy link
Contributor

@fsb4000 fsb4000 commented Mar 6, 2023

Fixes #3538

@fsb4000 fsb4000 requested a review from a team as a code owner March 6, 2023 01:00
DenisYaroshevskiy
DenisYaroshevskiy reviewed Mar 6, 2023
View reviewed changes
Copy link

@DenisYaroshevskiy DenisYaroshevskiy left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What about a move assignment above the one you fixed. Can you delete it?

STL/stl/inc/memory

Line 3256 in 994f941

_CONSTEXPR23 unique_ptr& operator=(unique_ptr<_Ty2, _Dx2>&& _Right) noexcept {

@fsb4000
Copy link
Contributor Author

fsb4000 commented Mar 6, 2023 •
edited
Loading

Can you delete it?

No.

Look at https://eel.is/c++draft/unique.ptr.single.asgn

We must have 3 assignment operators.

constexpr unique_ptr& operator=(unique_ptr&& u) noexcept;

Constraints: is_­move_­assignable_­v is true.

STL/stl/inc/memory

Lines 3262 to 3269 in 994f941

template <class _Dx2 = _Dx, enable_if_t<is_move_assignable_v<_Dx2>, int> = 0>
_CONSTEXPR23 unique_ptr& operator=(unique_ptr&& _Right) noexcept {
if (this != _STD addressof(_Right)) {
reset(_Right.release());
_Mypair._Get_first() = _STD forward<_Dx>(_Right._Mypair._Get_first());
}
return *this;
} 

template<class U, class E> constexpr unique_ptr& operator=(unique_ptr<U, E>&& u) noexcept;

Constraints:

unique_­ptr<U, E>​::​pointer is implicitly convertible to pointer, and
U is not an array type, and
is_­assignable_­v<D&, E&&> is true.

STL/stl/inc/memory

Lines 3252 to 3260 in 994f941

template <class _Ty2, class _Dx2,
enable_if_t<conjunction_v<negation<is_array<_Ty2>>, is_assignable<_Dx&, _Dx2>,
is_convertible<typename unique_ptr<_Ty2, _Dx2>::pointer, pointer>>,
int> = 0>
_CONSTEXPR23 unique_ptr& operator=(unique_ptr<_Ty2, _Dx2>&& _Right) noexcept {
reset(_Right.release());
_Mypair._Get_first() = _STD forward<_Dx2>(_Right._Mypair._Get_first());
return *this;
} 

constexpr unique_ptr& operator=(nullptr_t) noexcept;

STL/stl/inc/memory

Lines 3212 to 3215 in 994f941

_CONSTEXPR23 unique_ptr& operator=(nullptr_t) noexcept {
reset();
return *this;
}

@DenisYaroshevskiy
Copy link

DenisYaroshevskiy commented Mar 6, 2023

I see, appreciate it

@StephanTLavavej StephanTLavavej added the performance Must go faster label Mar 6, 2023
@StephanTLavavej StephanTLavavej self-assigned this Mar 7, 2023
@StephanTLavavej
Copy link
Member

StephanTLavavej commented Mar 7, 2023

I'm mirroring this to the MSVC-internal repo - please notify me if any further changes are pushed.

@StephanTLavavej StephanTLavavej merged commit a5b46ba into microsoft:main Mar 7, 2023
@StephanTLavavej
Copy link
Member

StephanTLavavej commented Mar 7, 2023

Thanks for eliminating this branch in a widely used member function! 🌴 🪓 😸

@fsb4000 fsb4000 deleted the fix3538 branch March 8, 2023 01:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@StephanTLavavej StephanTLavavej StephanTLavavej approved these changes

+3 more reviewers

@DenisYaroshevskiy DenisYaroshevskiy DenisYaroshevskiy left review comments

@CaseyCarter CaseyCarter CaseyCarter approved these changes

@strega-nil-ms strega-nil-ms strega-nil-ms approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@StephanTLavavej StephanTLavavej

Labels

performance Must go faster

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

if (this != &rhs) in unique ptr

5 participants

@fsb4000 @DenisYaroshevskiy @StephanTLavavej @CaseyCarter @strega-nil-ms