-
Notifications
You must be signed in to change notification settings - Fork 404
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
iox-#1823 Fix double move in emplace #1825
iox-#1823 Fix double move in emplace #1825
Conversation
Signed-off-by: Christian Eltzschig <[email protected]>
Signed-off-by: Christian Eltzschig <[email protected]>
Signed-off-by: Christian Eltzschig <[email protected]>
Codecov Report
@@ Coverage Diff @@
## master #1825 +/- ##
==========================================
+ Coverage 75.68% 75.70% +0.01%
==========================================
Files 375 375
Lines 14565 14566 +1
Branches 2067 2067
==========================================
+ Hits 11024 11027 +3
+ Misses 2910 2909 -1
+ Partials 631 630 -1
Flags with carried forward coverage won't be shown. Click here to find out more.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Function looks correct. Redundant parentheses, no semantic influence.
@@ -174,17 +174,18 @@ template <typename T, uint64_t Capacity> | |||
template <typename... Targs> | |||
inline bool vector<T, Capacity>::emplace(const uint64_t position, Targs&&... args) noexcept | |||
{ | |||
if ((m_size >= Capacity) || ((position >= Capacity) || (position > m_size))) | |||
const auto sizeBeforeEmplace = m_size; | |||
if ((m_size >= Capacity) || ((position >= Capacity) || (position > sizeBeforeEmplace))) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if ((m_size >= Capacity) || ((position >= Capacity) || (position > sizeBeforeEmplace))) | |
if ((m_size >= Capacity) || (position >= Capacity) || (position > sizeBeforeEmplace)) |
Even with redundant brace rules this is sufficient I think.
Pre-Review Checklist for the PR Author
iox-123-this-is-a-branch
)iox-#123 commit text
)task-list-completed
)iceoryx_hoofs
are added to./clang-tidy-diff-scans.txt
Notes for Reviewer
Checklist for the PR Reviewer
iceoryx_hoofs
have been added to./clang-tidy-diff-scans.txt
Post-review Checklist for the PR Author
References
vector::emplace
move element twice and causes undefined behavior #1823