cpp-docs icon indicating copy to clipboard operation
cpp-docs copied to clipboard

Modifies the description of `self-assignment` in the move assignment operator

Open Mq-b opened this issue 1 year ago • 6 comments

I think the description and handling of securing self-assignment is too simplistic.

Not all cases should use self-assignment checking to ensure self-assignment security, and in many cases it is not necessary.

MSVC STL std::unique_ptrstd::shared_ptr

template <class _Dx2 = _Dx, enable_if_t<is_move_assignable_v<_Dx2>, int> = 0>
_CONSTEXPR23 unique_ptr& operator=(unique_ptr&& _Right) noexcept {
    reset(_Right.release());
    _Mypair._Get_first() = _STD forward<_Dx>(_Right._Mypair._Get_first());
    return *this;
}
shared_ptr& operator=(shared_ptr&& _Right) noexcept { // take resource from _Right
    shared_ptr(_STD move(_Right)).swap(*this);
    return *this;
}

class Foo {
    std::string s;
    int i;
};

Foo is inherently self-assignment safe and does not require any additional processing.

  • In short: I don't think we should just describe a simple self-assignment detection method.

Mq-b avatar Feb 29 '24 14:02 Mq-b

@Mq-b : Thanks for your contribution! The author(s) have been notified to review your proposed change.

prmerger-automator[bot] avatar Feb 29 '24 14:02 prmerger-automator[bot]

Learn Build status updates of commit eb8b75e:

:white_check_mark: Validation status: passed

File Status Preview URL Details
docs/cpp/move-constructors-and-move-assignment-operators-cpp.md :white_check_mark:Succeeded

For more details, please refer to the build report.

For any questions, please:

@TylerMSFT

  • Can you review this PR?
  • IMPORTANT: When this content is ready to merge, you must add #sign-off in a comment or the approval may get overlooked.

#label:"aq-pr-triaged" @MicrosoftDocs/public-repo-pr-review-team

Jak-MS avatar Feb 29 '24 17:02 Jak-MS

@Mq-b : Thanks for your contribution! The author(s) have been notified to review your proposed change.

prmerger-automator[bot] avatar Feb 29 '24 17:02 prmerger-automator[bot]

It's curious (or rather old-fashioned) that the whole file mentions neither swap nor exchange. Perhaps it need to be substantially updated.

frederick-vs-ja avatar Mar 01 '24 02:03 frederick-vs-ja

It's curious (or rather old-fashioned) that the whole file mentions neither swap nor exchange. Perhaps it need to be substantially updated.

Yes!

Mq-b avatar Mar 01 '24 03:03 Mq-b