Skip to content
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

Use const& instead of const where applicable #5081

Merged
merged 1 commit into from Sep 14, 2017

Conversation

masterleinad
Copy link
Member

This results from a recent clang-tidy run. Use const & instead of const were applicable.

@@ -283,7 +283,7 @@ namespace LinearAlgebra
*/
void import(const distributed::Vector<Number> &vec,
VectorOperation::values operation,
std::shared_ptr<const CommunicationPatternBase> communication_pattern =
const std::shared_ptr<const CommunicationPatternBase> &communication_pattern =
Copy link
Member

Choose a reason for hiding this comment

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

Personally, I don't like this. Why would we want to pass a shared_ptr by reference? There is also no point in making the shared_ptr constant, what matters is what the shared_ptr points to.

Copy link
Member Author

Choose a reason for hiding this comment

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

This way we avoid increasing and decreasing the counter though.

@tjhei
Copy link
Member

tjhei commented Sep 14, 2017

This way we avoid increasing and decreasing the counter though.

Not that the performance of it matters here, but a shared_ptr needs to do refcounting in a threadsafe manner (mutex or atomic operations), so it is not "cheap". I agree that passing by reference here is cleaner.

@tjhei
Copy link
Member

tjhei commented Sep 14, 2017

/run-tests

@Rombur
Copy link
Member

Rombur commented Sep 14, 2017

Not that the performance of it matters here, but a shared_ptr needs to do refcounting in a threadsafe manner (mutex or atomic operations), so it is not "cheap"

It is cheap compared to the MPI communication that the function does.

@bangerth
Copy link
Member

Nice!
OK to merge once the tester is happy.

@bangerth bangerth merged commit e18088f into dealii:master Sep 14, 2017
@masterleinad masterleinad deleted the use_const_ref branch October 16, 2017 14:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants