-
Notifications
You must be signed in to change notification settings - Fork 1
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
create_intervention_scenario: std::move risk_impacts to avoid copying #484
Conversation
cd6d813
to
c9ab7c3
Compare
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.
clang-tidy made some suggestions
PolicyDynamic dynamic) | ||
: active_period{period}, impacts{sorted_impacts}, dynamic{std::move(dynamic)} {} | ||
std::vector<PolicyImpact> sorted_impacts, PolicyDynamic dynamic) | ||
: active_period{period}, impacts{std::move(sorted_impacts)}, dynamic{std::move(dynamic)} {} |
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.
warning: std::move of the variable 'dynamic' of the trivially-copyable type 'PolicyDynamic' has no effect; remove std::move() [performance-move-const-arg]
: active_period{period}, impacts{std::move(sorted_impacts)}, dynamic{std::move(dynamic)} {} | |
: active_period{period}, impacts{std::move(sorted_impacts)}, dynamic{dynamic} {} |
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.
clang-tidy made some suggestions
PolicyDynamic dynamic) | ||
: active_period{period}, impacts{sorted_impacts}, dynamic{std::move(dynamic)} {} | ||
std::vector<PolicyImpact> sorted_impacts, PolicyDynamic dynamic) | ||
: active_period{period}, impacts{std::move(sorted_impacts)}, dynamic{std::move(dynamic)} {} |
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.
warning: std::move of the variable 'dynamic' of the trivially-copyable type 'PolicyDynamic' has no effect; remove std::move() [performance-move-const-arg]
: active_period{period}, impacts{std::move(sorted_impacts)}, dynamic{std::move(dynamic)} {} | |
: active_period{period}, impacts{std::move(sorted_impacts)}, dynamic{dynamic} {} |
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.
Looks fine. Go ahead 👍
2d23649
to
35919e5
Compare
While I was mucking about in the code base, I noticed that we were missing an opportunity to
std::move
something increate_intervention_scenario()
, so I've fixed it up.