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

Enable clang-tidy checks #309

Merged
merged 11 commits into from Sep 22, 2023

Conversation

bcaddy
Copy link
Collaborator

@bcaddy bcaddy commented Jul 3, 2023

hicpp-signed-bitwise

This check only failed in a few spots and it was with string literals operating with an unsigned int types so I set the
IgnorePositiveIntegerLiterals option to true.

misc-confusable-identifiers check

The only thing that triggered this was a1 and al in the Roe solver. It turns out the expression for al was short and only used one place so I copied and pasted it there, eliminating al entirely. I did the same for ar to maintain symmetry.

readability-identifier-naming

Fixed and enabled name case checking for:

  • namespaces
  • enums
  • macro definitions
  • typdefs
  • type aliases

cppcoreguidelines-pro-bounds-constant-array-index

Disabled. This would be good to have on but it's just not feasible in GPU code since it pretty much requires heavy use of the STL.

@bcaddy bcaddy changed the title Enable hicpp-signed-bitwise clang-tidy check Enable 2 clang-tidy checks Jul 3, 2023
@bcaddy bcaddy changed the title Enable 2 clang-tidy checks Enable clang-tidy checks Jul 10, 2023
@bcaddy bcaddy force-pushed the dev-hackday-2023-07-03 branch 4 times, most recently from a63d527 to 4698d26 Compare July 10, 2023 21:04
@bcaddy bcaddy marked this pull request as draft July 12, 2023 19:32
@bcaddy
Copy link
Collaborator Author

bcaddy commented Aug 22, 2023

This should be ready to merge. I would like someone to take a look at the changes in commit 45ace1b since I changed the logic a bit. It should yield the same results, be clearer, and provide warning on a failure but I want a second set of eyes just in case.

Pretty sure it's just the stuff in mpi_routines.cpp

@bcaddy bcaddy marked this pull request as ready for review August 22, 2023 21:16
@bcaddy bcaddy force-pushed the dev-hackday-2023-07-03 branch 6 times, most recently from 626817b to 9550e28 Compare September 14, 2023 17:40
This check only failed in a few spots and it was with string literals
operating with an unsigned int type so I set the
IgnorePositiveIntegerLiterals option to true.
Enabled checks for
- macro definitions
- typdefs
- aliases
- enums
The ClassCase does cover structs so I removed the struct case line
While this would be a good check to have on it's just not feasible in
GPU code.
@evaneschneider evaneschneider merged commit 8c555b6 into cholla-hydro:dev Sep 22, 2023
8 of 9 checks passed
@bcaddy bcaddy deleted the dev-hackday-2023-07-03 branch November 30, 2023 16:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants