Skip to content

clang-tidy option in cmake an fix for cpp 14 #293

Merged
MFraters merged 6 commits intoGeodynamicWorldBuilder:masterfrom
MFraters:clang_tidy_cpp14
Jun 28, 2021
Merged

clang-tidy option in cmake an fix for cpp 14 #293
MFraters merged 6 commits intoGeodynamicWorldBuilder:masterfrom
MFraters:clang_tidy_cpp14

Conversation

@MFraters
Copy link
Member

This adds an option to run the clang-tidy check when compiling. It is off by default, because it makes compiling slower and is only useful when developing (could turn it on in debug mode in the future). I also am rerunning and fixing a select set of clang-tidy checks clang-tidy now that I am using cpp14. I am not sure whether all changes have to do with c++14 or I just missed them before.

I am also considering adding a test whether there are any issues.

@codecov
Copy link

codecov bot commented Jun 27, 2021

Codecov Report

Merging #293 (d515960) into master (be243ea) will not change coverage.
The diff coverage is 100.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #293   +/-   ##
=======================================
  Coverage   99.00%   99.00%           
=======================================
  Files          74       74           
  Lines        4228     4228           
=======================================
  Hits         4186     4186           
  Misses         42       42           
Impacted Files Coverage Δ
source/coordinate_systems/cartesian.cc 100.00% <ø> (ø)
source/coordinate_systems/interface.cc 100.00% <ø> (ø)
source/coordinate_systems/spherical.cc 100.00% <ø> (ø)
.../continental_plate_models/composition/interface.cc 100.00% <ø> (ø)
...es/continental_plate_models/composition/uniform.cc 100.00% <ø> (ø)
...tures/continental_plate_models/grains/interface.cc 100.00% <ø> (ø)
...eatures/continental_plate_models/grains/uniform.cc 100.00% <ø> (ø)
.../continental_plate_models/temperature/adiabatic.cc 94.11% <ø> (ø)
.../continental_plate_models/temperature/interface.cc 100.00% <ø> (ø)
...res/continental_plate_models/temperature/linear.cc 100.00% <ø> (ø)
... and 64 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update be243ea...d515960. Read the comment docs.

@github-actions
Copy link

github-actions bot commented Jun 27, 2021

Benchmark Master Feature Difference (99.9% CI)
Slab interpolation simple none 1.472 ± 0.089 (s=304) 1.457 ± 0.077 (s=313) -2.5% .. +0.5%
Slab interpolation curved simple none 1.400 ± 0.006 (s=314) 1.399 ± 0.003 (s=332) -0.1% .. +0.0%
Spherical slab interpolation simple none 2.032 ± 0.042 (s=213) 2.034 ± 0.045 (s=232) -0.5% .. +0.8%
Slab interpolation simple curved CMS 2.131 ± 0.017 (s=222) 2.126 ± 0.015 (s=203) -0.5% .. +0.0%
Spherical slab interpolation simple CMS 4.463 ± 0.163 (s=108) 4.457 ± 0.182 (s=96) -2.0% .. +1.7%
Spherical fault interpolation simple none 2.704 ± 0.047 (s=174) 2.692 ± 0.042 (s=162) -1.0% .. +0.2%

@MFraters MFraters force-pushed the clang_tidy_cpp14 branch 3 times, most recently from c7bdab7 to f167391 Compare June 27, 2021 06:07
@MFraters
Copy link
Member Author

I think these changes are sensible and considerably clean up the code. I also think they best to do right after the release, so that future development for new contributors will hopefully be easier. Although I do not think will make a real git merge conflict, I will wait on merging for #289 since it is nearly almost ready to merge and it includes some header files which will be changed. I don't think I need to wait for #264, since the changes there are not close the the changes in this pull request.

In summary, this modernizes the code for cpp14 in several areas and introduces a few readability improvements. They can be automatically checked and even fixed though cmake, but I don't think I will enforce this strictly for the time being. There is a will also be a follow up pull request to clean the code up even more (remove unused headers and fix some clang warnings (I will also need to add a non-mac clang tester, they are really different in their warnings apparently))

@MFraters MFraters added the ready to merge Pull request is ready to merge. May be waiting for tests to complete or other reviews. label Jun 27, 2021
@MFraters MFraters merged commit bdb5458 into GeodynamicWorldBuilder:master Jun 28, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ready to merge Pull request is ready to merge. May be waiting for tests to complete or other reviews.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant