-
Notifications
You must be signed in to change notification settings - Fork 63
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
Core library cleanup #1457
Core library cleanup #1457
Conversation
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.
These changes make sense to me!
@@ -97,17 +95,17 @@ maximum_number_of_previous_solutions() | |||
*/ | |||
inline unsigned int | |||
number_of_previous_solutions( | |||
Parameters::SimulationControl::TimeSteppingMethod method) | |||
const Parameters::SimulationControl::TimeSteppingMethod method) |
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.
Shouldn't the const
be absent here, given the link in the description?
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.
Very good question friend. Because this is an inline function the function is defined afterwards. So here the header does not contain the function prototype but the function itself. Consequently, the const has to be there because the function is defined straight up afterwards :)
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.
Oh that makes sense! Thank you for the explanation
9f00f03
to
2ac943a
Compare
Description
In the aims of deploying a CI that would check for warnings using the clang compiler, I am fixing some of these warnings ahead of time by cleaning the code file by file. This PR addresses this for only 4 files in the core library, but this has impact beyond the code library itself.
One important thing, I have learnt that according to clang we should not mark stuff const in the .h file, but on in the .cc file ( see https://stackoverflow.com/questions/38660537/why-is-const-unnecessary-in-function-declarations-in-header-files-for-parameters) and so I have applied these changes also here.
Testing
All unit tests should not change. They did not.
Miscellaneous (will be removed when merged)
Checklist (will be removed when merged)
See this page for more information about the pull request process.
Code related list:
Pull request related list: