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

chore: Add a basic clang-tidy config #1192

Merged
merged 2 commits into from May 7, 2023
Merged

chore: Add a basic clang-tidy config #1192

merged 2 commits into from May 7, 2023

Conversation

royjacobson
Copy link
Contributor

@royjacobson royjacobson commented May 7, 2023

Tried to filter for low noise high value, but this is still much too noisy for something like a CI stage. Statistics of running on the whole DF + helio code base:

    265 [bugprone-narrowing-conversions]
    115 [clang-diagnostic-unqualified-std-cast-call]
     87 [readability-inconsistent-declaration-parameter-name]
     51 [performance-move-const-arg]
     45 [modernize-make-unique]
     30 [performance-unnecessary-value-param]
     25 [readability-make-member-function-const]
     24 [readability-container-size-empty]
     18 [google-explicit-constructor]
     16 [modernize-deprecated-headers]
     14 [modernize-return-braced-init-list]
     13 [readability-named-parameter]
     13 [google-default-arguments]
     10 [readability-static-definition-in-anonymous-namespace]
      8 [bugprone-unchecked-optional-access]
      7 [clang-diagnostic-unused-const-variable]
      6 [readability-redundant-member-init]
      5 [modernize-use-emplace]
      5 [misc-unused-alias-decls]
      5 [clang-diagnostic-unused-result]
      5 [bugprone-too-small-loop-variable]
      4 [performance-no-int-to-ptr]
      4 [modernize-use-equals-delete]
      4 [modernize-unary-static-assert]
      4 [clang-diagnostic-unused-but-set-variable]
      4 [bugprone-integer-division]
      3 [readability-non-const-parameter]
      3 [modernize-pass-by-value]
      3 [misc-misplaced-const]
      3 [clang-diagnostic-unused-variable]
      3 [bugprone-macro-parentheses]
      3 [bugprone-exception-escape]
      2 [readability-use-anyofallof]
      2 [readability-redundant-string-init]
      2 [performance-for-range-copy]
      2 [modernize-use-using]
      2 [misc-unused-using-decls]
      2 [misc-uniqueptr-reset-release]
      2 [clang-diagnostic-parentheses]
      2 [bugprone-unhandled-exception-at-new]
      2 [bugprone-suspicious-missing-comma]
      2 [bugprone-incorrect-roundings]
      1 [readability-redundant-string-cstr]
      1 [readability-redundant-smartptr-get]
      1 [readability-redundant-declaration]
      1 [readability-redundant-control-flow]
      1 [readability-const-return-type]
      1 [performance-unnecessary-copy-initialization]
      1 [performance-faster-string-find]
      1 [modernize-raw-string-literal]
      1 [misc-redundant-expression]
      1 [google-build-explicit-make-pair]
      1 [clang-diagnostic-unused-function]
      1 [clang-diagnostic-implicit-const-int-float-conversion]
      1 [clang-diagnostic-final-dtor-non-final-class]
      1 [bugprone-use-after-move]
      1 [bugprone-unused-raii]
      1 [bugprone-signed-char-misuse]
      1 [bugprone-assignment-in-if-condition]

Second commit fixes a few of the found issues.

@dranikpg
Copy link
Contributor

dranikpg commented May 7, 2023

Narrowing is a whole problem on its own. We use different types throughout the project for arg positions/sizes/etc and they're often casted back and forth

Maybe we can at first focus only df core parts (no helio, no tests)?

51 [performance-move-const-arg]

Wow 👀 We should look at those

@chakaz
Copy link
Collaborator

chakaz commented May 7, 2023

Yes please!! Great work!
Perhaps we can aim to enable the disabled ones in future fix-it weeks. Is there some tag for such tasks?

@chakaz chakaz enabled auto-merge (squash) May 7, 2023 19:46
@@ -700,7 +700,7 @@ error_code Replica::ConsumeDflyStream() {

// Iterate over map and cancle all blocking entities
{
lock_guard{multi_shard_exe_->map_mu};
lock_guard l{multi_shard_exe_->map_mu};
Copy link
Collaborator

Choose a reason for hiding this comment

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

Wow, nice catch!

@chakaz chakaz merged commit 912843d into main May 7, 2023
6 checks passed
@royjacobson
Copy link
Contributor Author

Narrowing is a whole problem on its own. We use different types throughout the project for arg positions/sizes/etc and they're often casted back and forth

Yeah, I disabled the narrowing warning at the end :) I don't thing that going over 260 warnings and fixing things like int64_t s = v.size() manually is not cost effective.

@romange romange deleted the clang-tidy branch May 13, 2023 11:25
romange pushed a commit that referenced this pull request Jun 1, 2023
* chore: Add .clang-tidy file

* Fix some problems and non-problems
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

3 participants