Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

Fix assignment operator issues in ByteArray#51

Open
noSTALKER wants to merge 4 commits into
microsoft:mainmicrosoft/SPTAG:mainfrom
noSTALKER:bytearray_fixesnoSTALKER/SPTAG:bytearray_fixesCopy head branch name to clipboard
Open

Fix assignment operator issues in ByteArray#51
noSTALKER wants to merge 4 commits into
microsoft:mainmicrosoft/SPTAG:mainfrom
noSTALKER:bytearray_fixesnoSTALKER/SPTAG:bytearray_fixesCopy head branch name to clipboard

Conversation

@noSTALKER

Copy link
Copy Markdown
Contributor
  • Fix assignment operator issues in ByteArray
  • Add noexcept qualifier to functions

+ Fix assignment operator issues in ByteArray
+ Add noexcept qualifier to functions
m_length(0)
ByteArray::ByteArray() noexcept
: m_data(nullptr)
, m_length(0)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

please keep original formatting.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I have reverted the changes done in member initializer lists

m_data = p_right.m_data;
m_length = p_right.m_length;
m_dataHolder = p_right.m_dataHolder;
if (this != std::addressof(p_right))

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think shared_ptr has already handled such case? Not sure if this is necessary.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I haven't checked yet but it is just a recommended standard check in both copy constructor and move constructors

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I take some investigation and think this check is not necessary for std::shared_ptr and most other stl containers (it seems a popular implementation for shared_ptr assign function is to create a temp var and then swap it with this). So please remove this check.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I have reverted back the checks

AlphardWang
AlphardWang previously approved these changes Jun 12, 2019
@microsoft-github-updates
microsoft-github-updates Bot changed the base branch from master to main January 10, 2022 04:52
@microsoft-github-updates
microsoft-github-updates Bot dismissed AlphardWang’s stale review January 10, 2022 04:52

The base branch was changed.

ByteArray::operator= (ByteArray&& p_right)
ByteArray::operator=(ByteArray&& p_right) noexcept
{
m_data = p_right.m_data;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

shouldn't this move too?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

Morty Proxy This is a proxified and sanitized view of the page, visit original site.