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

Tracking issue: Fix clang-tidy reports in kvrocks #1029

Closed
17 of 18 tasks
PragmaTwice opened this issue Oct 23, 2022 · 0 comments
Closed
17 of 18 tasks

Tracking issue: Fix clang-tidy reports in kvrocks #1029

PragmaTwice opened this issue Oct 23, 2022 · 0 comments
Labels
enhancement type enhancement good first issue Good for newcomers help wanted Good for newcomers

Comments

@PragmaTwice
Copy link
Member

PragmaTwice commented Oct 23, 2022

Search before asking

  • I had searched in the issues and found no similar issues.

Motivation

    clang-analyzer-core.CallAndMessage
    clang-analyzer-core.CallAndMessageModeling
    clang-analyzer-core.DivideZero
    clang-analyzer-core.DynamicTypePropagation
    clang-analyzer-core.NonNullParamChecker
    clang-analyzer-core.NonnilStringConstants
    clang-analyzer-core.NullDereference
    clang-analyzer-core.StackAddrEscapeBase
    clang-analyzer-core.StackAddressEscape
    clang-analyzer-core.UndefinedBinaryOperatorResult
    clang-analyzer-core.VLASize
    clang-analyzer-core.builtin.BuiltinFunctions
    clang-analyzer-core.builtin.NoReturnFunctions
    clang-analyzer-core.uninitialized.ArraySubscript
    clang-analyzer-core.uninitialized.Assign
    clang-analyzer-core.uninitialized.Branch
    clang-analyzer-core.uninitialized.CapturedBlockVariable
    clang-analyzer-core.uninitialized.UndefReturn
    clang-analyzer-cplusplus.InnerPointer
    clang-analyzer-cplusplus.Move
    clang-analyzer-cplusplus.NewDelete
    clang-analyzer-cplusplus.NewDeleteLeaks
    clang-analyzer-cplusplus.PlacementNew
    clang-analyzer-cplusplus.PureVirtualCall
    clang-analyzer-cplusplus.SelfAssignment
    clang-analyzer-cplusplus.SmartPtrModeling
    clang-analyzer-cplusplus.VirtualCallModeling
    clang-analyzer-deadcode.DeadStores
    clang-analyzer-nullability.NullPassedToNonnull
    clang-analyzer-nullability.NullReturnedFromNonnull
    clang-analyzer-nullability.NullabilityBase
    clang-analyzer-nullability.NullableDereferenced
    clang-analyzer-nullability.NullablePassedToNonnull
    clang-analyzer-nullability.NullableReturnedFromNonnull
    clang-analyzer-security.FloatLoopCounter
    clang-analyzer-security.insecureAPI.DeprecatedOrUnsafeBufferHandling
    clang-analyzer-security.insecureAPI.SecuritySyntaxChecker
    clang-analyzer-security.insecureAPI.UncheckedReturn
    clang-analyzer-security.insecureAPI.bcmp
    clang-analyzer-security.insecureAPI.bcopy
    clang-analyzer-security.insecureAPI.bzero
    clang-analyzer-security.insecureAPI.decodeValueOfObjCType
    clang-analyzer-security.insecureAPI.getpw
    clang-analyzer-security.insecureAPI.gets
    clang-analyzer-security.insecureAPI.mkstemp
    clang-analyzer-security.insecureAPI.mktemp
    clang-analyzer-security.insecureAPI.rand
    clang-analyzer-security.insecureAPI.strcpy
    clang-analyzer-security.insecureAPI.vfork
    clang-analyzer-unix.API
    clang-analyzer-unix.DynamicMemoryModeling
    clang-analyzer-unix.Malloc
    clang-analyzer-unix.MallocSizeof
    clang-analyzer-unix.MismatchedDeallocator
    clang-analyzer-unix.Vfork
    clang-analyzer-unix.cstring.BadSizeArg
    clang-analyzer-unix.cstring.CStringModeling
    clang-analyzer-unix.cstring.NullArg
    clang-analyzer-valist.CopyToSelf
    clang-analyzer-valist.Uninitialized
    clang-analyzer-valist.Unterminated
    clang-analyzer-valist.ValistBase
    cppcoreguidelines-init-variables 197
    cppcoreguidelines-interfaces-global-init 0
    cppcoreguidelines-macro-usage 21
    cppcoreguidelines-narrowing-conversions 122
    cppcoreguidelines-no-malloc 0
    cppcoreguidelines-prefer-member-initializer 0
    cppcoreguidelines-slicing 0
    cppcoreguidelines-special-member-functions 3
    google-build-explicit-make-pair 0
    google-default-arguments 0
    google-explicit-constructor 0
    modernize-avoid-bind 3
    modernize-loop-convert 7
    modernize-make-shared 0
    modernize-make-unique 2
    modernize-pass-by-value 0
    modernize-redundant-void-arg 5
    modernize-return-braced-init-list 0
    modernize-use-auto 9
    modernize-use-bool-literals 1
    modernize-use-emplace 0
    modernize-use-equals-default 0
    modernize-use-equals-delete 0
    modernize-use-nullptr 5
    modernize-use-override 0
    modernize-use-using 1
    performance-faster-string-find 3
    performance-for-range-copy 2
    performance-implicit-conversion-in-loop 0
    performance-inefficient-algorithm 0
    performance-inefficient-vector-operation 6
    performance-move-const-arg 0
    performance-move-constructor-init 0
    performance-no-automatic-move 0
    performance-trivially-destructible 0
    performance-type-promotion-in-math-fn 0
    performance-unnecessary-copy-initialization 0
    performance-unnecessary-value-param 16

Currently we have enabled lots of clang-tidy checks, but there are so many reports in current kvrocks code, so we cannot treat these report as errors to block future PR with some clang-tidy reported warnings.

We can solve these report and enable warning-as-error: https://github.com/apache/incubator-kvrocks/actions/runs/3399280291/jobs/5652921393#step:8:899

For example:

/home/runner/work/incubator-kvrocks/incubator-kvrocks/src/cluster/redis_slot.cc:72:29: warning: 'find' called with a string literal consisting of a single character; consider using the more effective overload accepting a character [performance-faster-string-find]
  auto right_pos = key.find("}", left_pos + 1);
                            ^~~
                            '}'

For this report (categorized as performance-faster-string-find), we can replace "}" with '}', and submit a patch. If all performance-faster-string-find tagged report are solved within this patch, we can add performance-faster-string-find to WarningsAsErrors in .clang-tidy. Then, if performance-faster-string-find tagged report appears in a future PR, the CI will fail.

Solution

Tasks:

Are you willing to submit a PR?

  • I'm willing to submit a PR!
@PragmaTwice PragmaTwice added enhancement type enhancement help wanted Good for newcomers good first issue Good for newcomers labels Oct 23, 2022
@PragmaTwice PragmaTwice changed the title Fix clang-tidy reports in kvrocks Tracking issue: Fix clang-tidy reports in kvrocks Nov 12, 2022
@tanruixiang tanruixiang pinned this issue Nov 12, 2022
@tanruixiang tanruixiang unpinned this issue Dec 13, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement type enhancement good first issue Good for newcomers help wanted Good for newcomers
Projects
None yet
Development

No branches or pull requests

1 participant