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

Tidy headers #1534

Merged
merged 14 commits into from
Dec 4, 2024
Merged

Tidy headers #1534

merged 14 commits into from
Dec 4, 2024

Conversation

esseivaju
Copy link
Contributor

@sethrj That's what it takes to tidy the headers, 90% of it is just the rule of 5, missing std::forward on function objects passed as universal reference, and override/final annotations.

@esseivaju esseivaju added the documentation Documentation, examples, tests, and CI label Dec 4, 2024
@esseivaju esseivaju requested a review from sethrj December 4, 2024 16:29
Copy link

github-actions bot commented Dec 4, 2024

Test summary

 3 882 files   5 991 suites   4m 12s ⏱️
 1 609 tests  1 580 ✅ 29 💤 0 ❌
19 907 runs  19 832 ✅ 75 💤 0 ❌

Results for commit 00ccf65.

♻️ This comment has been updated with latest results.

src/accel/HepMC3PrimaryGenerator.hh Outdated Show resolved Hide resolved
src/celeritas/Types.hh Outdated Show resolved Hide resolved
src/celeritas/em/data/AtomicRelaxationData.hh Outdated Show resolved Hide resolved
src/celeritas/ext/detail/AllElementReader.hh Outdated Show resolved Hide resolved
src/celeritas/field/MakeMagFieldPropagator.hh Outdated Show resolved Hide resolved
src/celeritas/grid/GridIdFinder.hh Outdated Show resolved Hide resolved
@sethrj
Copy link
Member

sethrj commented Dec 4, 2024

@esseivaju This is great, but before you go any further can you turn off the "operator= signature" and "missing std::forward" since those are always going to generate new warnings for correct code?

@esseivaju
Copy link
Contributor Author

@esseivaju This is great, but before you go any further can you turn off the "operator= signature" and "missing std::forward" since those are always going to generate new warnings for correct code?

I'll turn these off, out of curiosity, what's the advantage of passing a function object by forwarding reference instead of constructing referenced if we need to call it multiple times?

@sethrj
Copy link
Member

sethrj commented Dec 4, 2024

@esseivaju The problem is that if you've got a functor that has a mutable operator() then you have to pass a mutable reference; but if you're passing a const function object then you have to use a const reference. Passing by value or rvalue reference is I think the only way to do it.

@esseivaju esseivaju marked this pull request as ready for review December 4, 2024 22:11
Copy link
Member

@sethrj sethrj left a comment

Choose a reason for hiding this comment

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

Thanks for all that (hopefully not too tedious) work!

@sethrj sethrj enabled auto-merge (squash) December 4, 2024 22:28
@sethrj sethrj merged commit 6f376a2 into celeritas-project:develop Dec 4, 2024
33 checks passed
@esseivaju esseivaju deleted the clang-tidy-headers branch December 5, 2024 00:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Documentation, examples, tests, and CI
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants