Skip to content

[Clang-tidy] apply some auto generated fixes#1669

Merged
tdavidcl merged 8 commits intoShamrock-code:mainfrom
tdavidcl:patch-2026-02-21-12-10
Feb 22, 2026
Merged

[Clang-tidy] apply some auto generated fixes#1669
tdavidcl merged 8 commits intoShamrock-code:mainfrom
tdavidcl:patch-2026-02-21-12-10

Conversation

@tdavidcl
Copy link
Copy Markdown
Member

No description provided.

@github-actions
Copy link
Copy Markdown
Contributor

Thanks @tdavidcl for opening this PR!

You can do multiple things directly here:
1 - Comment pre-commit.ci run to run pre-commit checks.
2 - Comment pre-commit.ci autofix to apply fixes.
3 - Add label autofix.ci to fix authorship & pre-commit for every commit made.
4 - Add label light-ci to only trigger a reduced & faster version of the CI (need the full one before merge).
5 - Add label trigger-ci to create an empty commit to trigger the CI.

Once the workflow completes a message will appear displaying informations related to the run.

Also the PR gets automatically reviewed by gemini, you can:
1 - Comment /gemini review to trigger a review
2 - Comment /gemini summary for a summary
3 - Tag it using @gemini-code-assist either in the PR or in review comments on files

@gemini-code-assist
Copy link
Copy Markdown
Contributor

Summary of Changes

Hello @tdavidcl, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request integrates a series of automated clang-tidy fixes aimed at enhancing the overall quality, performance, and correctness of the C++ codebase. The changes primarily focus on optimizing parameter passing mechanisms to avoid redundant data copies, simplifying logical expressions, and ensuring proper resource management through move semantics. These modifications contribute to a more robust and efficient application without altering its core functionality.

Highlights

  • Const-correctness and efficiency improvements: Updated numerous function parameters across the codebase to be passed by const & (constant reference) instead of by value or non-const reference. This applies to std::string, std::shared_ptr, std::optional<std::string>, DeviceProperties, sycl::exception_list, sham::DeviceScheduler_ptr, and sycl::event types, reducing unnecessary copies and improving performance.
  • Range-based for loop optimization: Modified range-based for loops to iterate over const auto & or const sycl::event & to prevent accidental modification of elements and improve efficiency when elements are not intended to be changed.
  • Code simplification: Simplified boolean return statements in src/shambackends/src/DeviceContext.cpp and src/shamcmdopt/src/cmdopt.cpp from if/else blocks to direct return expressions.
  • Move semantics for optional strings: Introduced std::move when passing std::optional<std::string> parameters to other functions, ensuring efficient transfer of ownership for potentially expensive string objects.
  • Header includes: Added #include <utility> in src/shambackends/src/details/BufferEventHandler.cpp and src/shambase/src/profiling/profiling.cpp to support the use of std::move.
Changelog
  • src/shambackends/include/shambackends/details/internal_alloc.hpp
    • Updated internal_alloc and internal_free function signatures to pass std::shared_ptr<DeviceScheduler> by const &.
  • src/shambackends/src/Device.cpp
    • Modified fetch_mpi_properties function signature to pass DeviceProperties by const &.
  • src/shambackends/src/DeviceContext.cpp
    • Changed exception_handler lambda parameter exceptions to const sycl::exception_list &.
    • Simplified the is_help_mode function's boolean return logic.
  • src/shambackends/src/DeviceScheduler.cpp
    • Updated test_device_scheduler function signature to pass sham::DeviceScheduler_ptr by const &.
  • src/shambackends/src/details/BufferEventHandler.cpp
    • Added #include <utility>.
    • Modified range-based for loops to iterate over const sycl::event &.
    • Used std::move when constructing std::vector<sycl::event> from a single sycl::event.
  • src/shambackends/src/details/internal_alloc.cpp
    • Updated internal_free and internal_alloc function signatures to pass std::shared_ptr<DeviceScheduler> by const &.
    • Adjusted template instantiations for internal_free and internal_alloc to match the updated signatures.
  • src/shambase/include/shambase/fortran_io.hpp
    • Modified load_fortran_file function signature to pass std::string fname by const &.
  • src/shambase/include/shambase/logs/reformat_message.hpp
    • Updated reformat_all and reformat_simple function signatures to pass std::string parameters by const &.
  • src/shambase/include/shambase/print.hpp
    • Modified print and println function signatures to pass std::string by const &.
  • src/shambase/include/shambase/profiling/profiling.hpp
    • Updated stack_entry_start, stack_entry_end, stack_entry_start_no_time, and stack_entry_end_no_time function signatures to pass std::optional<std::string> parameters by const &.
  • src/shambase/include/shambase/stacktrace.hpp
    • Modified dump_profilings and dump_profilings_chrome function signatures to pass std::string process_prefix by const &.
  • src/shambase/src/logs/reformat_message.cpp
    • Updated reformat_all and reformat_simple function implementations to match the const & parameter changes.
  • src/shambase/src/print.cpp
    • Modified print and println function implementations to match the const & parameter changes.
  • src/shambase/src/profiling/profiling.cpp
    • Added #include <utility>.
    • Updated stack_entry_start, stack_entry_end, stack_entry_start_no_time, and stack_entry_end_no_time function implementations to match const & parameter changes.
    • Applied std::move when passing std::optional<std::string> arguments to nested profiling functions.
  • src/shambase/src/stacktrace.cpp
    • Modified dump_profilings_chrome and dump_profilings function implementations to match the const & parameter changes.
  • src/shambindings/src/locate_pylib.cpp
    • Updated range-based for loops to iterate over const auto & for std::string paths.
  • src/shamcmdopt/src/cmdopt.cpp
    • Added #include <utility>.
    • Modified range-based for loop in is_name_registered to iterate over const auto &.
    • Used std::move when pushing back args and description into registered_opts.
    • Simplified the is_help_mode function's boolean return logic.
  • src/shamcomm/include/shamcomm/io.hpp
    • Modified open_reset_file and open_read_only_file function signatures to pass std::string fname by const &.
  • src/shamcomm/src/io.cpp
    • Modified open_reset_file and open_read_only_file function implementations to match the const & parameter changes.
  • src/shamcomm/src/mpiInfo.cpp
    • Changed print_state lambda parameter log to const std::string &.
  • src/shamtree/include/shamtree/MortonKernels.hpp
    • Updated sycl_xyz_to_morton function signature to pass sham::DeviceScheduler_ptr by const &.
  • src/shamtree/src/MortonKernels.cpp
    • Updated sycl_xyz_to_morton function implementation to match the const & parameter change.
Activity
  • The pull request was created with an empty description, indicating it might be an automated or script-generated set of changes.
  • The title '[Clang-tidy] apply some auto generated fixes' confirms that the changes are derived from clang-tidy suggestions.
Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature Command Description
Code Review /gemini review Performs a code review for the current pull request in its current state.
Pull Request Summary /gemini summary Provides a summary of the current pull request in its current state.
Comment @gemini-code-assist Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help /gemini help Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Footnotes

  1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution.

Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request applies several auto-generated fixes from clang-tidy, which is great for improving code quality and performance. The changes primarily involve passing objects by const reference instead of by value to avoid unnecessary copies, and simplifying boolean returns. These are all positive changes.

I've found a couple of instances where std::move was applied to const references, which is ineffective and misleading. I've left specific comments with suggestions to correct these. Overall, good work on applying these automated improvements.

Comment thread src/shambase/src/profiling/profiling.cpp Outdated
Comment thread src/shambase/src/profiling/profiling.cpp Outdated
tdavidcl and others added 3 commits February 21, 2026 12:43
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
@tdavidcl
Copy link
Copy Markdown
Member Author

/gemini review

Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request applies a number of fixes automatically generated by clang-tidy. The changes primarily involve updating function parameters to use const& for non-trivial types like std::shared_ptr, std::string, and other structs, which is a good practice to avoid unnecessary copies and improve performance. Some stylistic improvements, like simplifying boolean returns, are also included. The changes are correct and beneficial. The existing comment provides a valid suggestion for further performance improvement and consistency, which aligns with general good practices and does not contradict any specific rules.

@tdavidcl tdavidcl added in-review and removed draft labels Feb 21, 2026
@tdavidcl tdavidcl marked this pull request as ready for review February 21, 2026 16:07
@github-actions
Copy link
Copy Markdown
Contributor

Workflow report

workflow report corresponding to commit 4d443f0
Commiter email is timothee.davidcleris@proton.me
GitHub page artifact URL GitHub page artifact link (can expire)

Pre-commit check report

Pre-commit check: ✅

trim trailing whitespace.................................................Passed
fix end of files.........................................................Passed
check for merge conflicts................................................Passed
check that executables have shebangs.....................................Passed
check that scripts with shebangs are executable..........................Passed
check for added large files..............................................Passed
check for case conflicts.................................................Passed
check for broken symlinks................................................Passed
check yaml...............................................................Passed
detect private key.......................................................Passed
No-tabs checker..........................................................Passed
Tabs remover.............................................................Passed
Validate GitHub Workflows................................................Passed
clang-format.............................................................Passed
ruff check...............................................................Passed
ruff format..............................................................Passed
Check doxygen headers....................................................Passed
Check license headers....................................................Passed
Check #pragma once.......................................................Passed
Check SYCL #include......................................................Passed
No ssh in git submodules remote..........................................Passed
No UTF-8 in files (except for authors)...................................Passed

Test pipeline can run.

Clang-tidy diff report

No relevant changes found.
Well done!

You should now go back to your normal life and enjoy a hopefully sunny day while waiting for the review.

Doxygen diff with main

Removed warnings : 13
New warnings : 13
Warnings count : 8183 → 8183 (0.0%)

Detailed changes :
- src/shambackends/src/DeviceScheduler.cpp:73: warning: Member test_device_scheduler(DeviceScheduler_ptr dev_sched) (function) of namespace sham is not documented.
- src/shambackends/src/DeviceScheduler.cpp:73: warning: Member test_device_scheduler(DeviceScheduler_ptr dev_sched) (function) of namespace sham is not documented.
- src/shambackends/src/DeviceScheduler.cpp:73: warning: Member test_device_scheduler(DeviceScheduler_ptr dev_sched) (function) of namespace sham is not documented.
+ src/shambackends/src/DeviceScheduler.cpp:73: warning: Member test_device_scheduler(const DeviceScheduler_ptr &dev_sched) (function) of namespace sham is not documented.
+ src/shambackends/src/DeviceScheduler.cpp:73: warning: Member test_device_scheduler(const DeviceScheduler_ptr &dev_sched) (function) of namespace sham is not documented.
+ src/shambackends/src/DeviceScheduler.cpp:73: warning: Member test_device_scheduler(const DeviceScheduler_ptr &dev_sched) (function) of namespace sham is not documented.
- src/shambase/src/profiling/profiling.cpp:28: warning: Member src_loc_to_name(const SourceLocation &loc) (function) of file profiling.cpp is not documented.
+ src/shambase/src/profiling/profiling.cpp:29: warning: Member src_loc_to_name(const SourceLocation &loc) (function) of file profiling.cpp is not documented.
- src/shambase/src/profiling/profiling.cpp:37: warning: Member get_profiling (variable) of file profiling.cpp is not documented.
+ src/shambase/src/profiling/profiling.cpp:38: warning: Member get_profiling (variable) of file profiling.cpp is not documented.
- src/shambase/src/profiling/profiling.cpp:49: warning: Member get_nvtx (variable) of file profiling.cpp is not documented.
+ src/shambase/src/profiling/profiling.cpp:50: warning: Member get_nvtx (variable) of file profiling.cpp is not documented.
- src/shambase/src/profiling/profiling.cpp:68: warning: Member get_complete_event (variable) of file profiling.cpp is not documented.
+ src/shambase/src/profiling/profiling.cpp:69: warning: Member get_complete_event (variable) of file profiling.cpp is not documented.
- src/shambase/src/profiling/profiling.cpp:80: warning: Member get_threshold (variable) of file profiling.cpp is not documented.
+ src/shambase/src/profiling/profiling.cpp:81: warning: Member get_threshold (variable) of file profiling.cpp is not documented.
- src/shambase/src/profiling/profiling.cpp:88: warning: Member enable_profiling (variable) of file profiling.cpp is not documented.
+ src/shambase/src/profiling/profiling.cpp:89: warning: Member enable_profiling (variable) of file profiling.cpp is not documented.
- src/shambase/src/profiling/profiling.cpp:89: warning: Member use_complete_event (variable) of file profiling.cpp is not documented.
- src/shambase/src/profiling/profiling.cpp:90: warning: Member threshold (variable) of file profiling.cpp is not documented.
+ src/shambase/src/profiling/profiling.cpp:90: warning: Member use_complete_event (variable) of file profiling.cpp is not documented.
- src/shambase/src/profiling/profiling.cpp:91: warning: Member enable_nvtx (variable) of file profiling.cpp is not documented.
+ src/shambase/src/profiling/profiling.cpp:91: warning: Member threshold (variable) of file profiling.cpp is not documented.
+ src/shambase/src/profiling/profiling.cpp:92: warning: Member enable_nvtx (variable) of file profiling.cpp is not documented.
+ src/shamtree/include/shamtree/MortonKernels.hpp:86: warning: Member sycl_xyz_to_morton(const sham::DeviceScheduler_ptr &dev_sched, u32 pos_count, sham::DeviceBuffer< pos_t > &in_positions, pos_t bounding_box_min, pos_t bounding_box_max, std::unique_ptr< sycl::buffer< morton_t > > &out_morton) (function) of class shamrock::sfc::MortonKernels is not documented.
- src/shamtree/include/shamtree/MortonKernels.hpp:86: warning: Member sycl_xyz_to_morton(sham::DeviceScheduler_ptr dev_sched, u32 pos_count, sham::DeviceBuffer< pos_t > &in_positions, pos_t bounding_box_min, pos_t bounding_box_max, std::unique_ptr< sycl::buffer< morton_t > > &out_morton) (function) of class shamrock::sfc::MortonKernels is not documented.

@tdavidcl tdavidcl merged commit fb9fa72 into Shamrock-code:main Feb 22, 2026
143 of 147 checks passed
@tdavidcl tdavidcl deleted the patch-2026-02-21-12-10 branch February 22, 2026 21:29
aserhani pushed a commit to aserhani/Shamrock that referenced this pull request Feb 23, 2026
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.

1 participant