Skip to content

Conversation

giacomofiorin
Copy link
Member

In draft for now: even with the default settings, the current code does not pass the test.

@giacomofiorin giacomofiorin added the testing Only affects CI; not listed in outside PRs label Sep 9, 2025
@giacomofiorin giacomofiorin force-pushed the clang-tidy branch 2 times, most recently from 47a2951 to af5e7f9 Compare September 9, 2025 20:46
@giacomofiorin
Copy link
Member Author

@HanatoK Continuing the conversation here, one of the advantages of using a container is to use a recent version of the Clang tools. The ubuntu-latest GitHub instance runs 18, but CentOS 9 comes with version 20.

@giacomofiorin
Copy link
Member Author

With the downgrade from Clang 20 to 18 and removal of the Lepton sources, we are down to two issues, one with colvarproxy_io::io_available() and one with the bitwise OR of values of Parse_Mode type.

I have a good idea about how to fix the former (requires some small refactoring) but I'm puzzled by the latter: I though that having added the clang::flag_enum annotation would have taken care of it, but was wrong. Any ideas?

@giacomofiorin
Copy link
Member Author

Looks like the checks are the same between 18 and 20, but good to have the latter back for the future (thanks @HanatoK).

I would like to add support for GNU Parallel in run_clang_tidy.sh. (The original command run-clang-tidy had the advantage of a -j flag.) What do you think?

@HanatoK
Copy link
Member

HanatoK commented Sep 10, 2025

Looks like the checks are the same between 18 and 20, but good to have the latter back for the future (thanks @HanatoK).

I would like to add support for GNU Parallel in run_clang_tidy.sh. (The original command run-clang-tidy had the advantage of a -j flag.) What do you think?

That's a good idea, but I don't know how to parallelize the script with GNU Parallel. It looks like there are other ways to parallelize the executions as well (see https://stackoverflow.com/questions/38774355/how-to-parallelize-for-loop-in-bash-limiting-number-of-processes).

@giacomofiorin
Copy link
Member Author

That's a good idea, but I don't know how to parallelize the script with GNU Parallel. It looks like there are other ways to parallelize the executions as well (see https://stackoverflow.com/questions/38774355/how-to-parallelize-for-loop-in-bash-limiting-number-of-processes).

I had forgotten about xargs -P: it's less powerful than Parallel, but more widely available. I added it to the script.

Also restored the -header-filter flag, so that we can see warnings from headers (there's a bunch in colvar_UIestimator.h

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
testing Only affects CI; not listed in outside PRs
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants