ci: run clang-tidy as a part of CI #4666
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,315 @@ | ||
| --- | ||
| # This file defines what clang-tidy checks and parameter. It is experimental, and may let | ||
|
Member
Author
There was a problem hiding this comment. @envoyproxy/maintainers I am open to any suggestion to this file. With this
Member
There was a problem hiding this comment. Hmm. Don't we need to fail if clang-tidy fails? So don't we need to be warning clean? Or we only fail on errors? In general it seems like we should be warning clean also.
Member
Author
There was a problem hiding this comment. No I don't think we should mark clang-tidy required, at least in a meanwhile. We can make some of checks error with
Member
There was a problem hiding this comment. Then what's the point of using a CI slot for this? No one will look at it.
Member
Author
There was a problem hiding this comment. Just like mac CI that is not required (on GitHub) but we still check? At this point, there are too many warnings to fix at once so I am hesitating to make them error. cc @jsedgwick @rlazarus @PiotrSikora on #921, any suggestion to enable some checks as
Member
There was a problem hiding this comment. How does Google deal with this internally? Since Google fixes bugs raised by clang-tidy presumably some internal CI is catching them? Can we just start with Google's config? cc @htuch @alyssawilk @PiotrSikora
Member
There was a problem hiding this comment. Internally clang-tidy is largely advisory I believe, but most reviewers will take its suggestions as actionable. For Envoy, only some importers (we have a weekly rotation) actually pay attention as it's largely mechanical. I think we have a noise problem with a purely advisory clang-tidy, if the report is as noisy as in https://gist.github.com/lizan/115f05ace9ac7dbae4c042f6fbdb3596, reviewers will never both to look at it. Inside Google, our review system highlights issues only in the delta diff. Maybe we can turn off all warnings, make clang-tidy compulsory and only turns on the ones that actually are low on the false positive side?
Member
Author
There was a problem hiding this comment. Yes running clang-tidy on diff (especially for PRs) is definitely the plan, considering how clang-tidy is heavy (it takes an hour to run on whole source code). There is a tool to run clang-tidy against diff. If we do have a better .clang-tidy to start with, I'm happy to start with that rather than this.
Member
There was a problem hiding this comment. +1, IMO we need to make this actionable or we shouldn't bother running it in CI. |
||
| # clang-tidy issue warnings more than expected. | ||
| # Generated with `clang-tidy --checks='*' --dump-config` with following changes: | ||
| # | ||
| # -cppcoreguidelines-pro-type-vararg, -hicpp-va-arg: | ||
| # Raises warning on every ASSERT/RELEASE_ASSERT. | ||
| # -llvm-header-guard,-clang-diagnostic-pragma-once-outside-header: | ||
| # Allow `#pragma once` header guard. | ||
| # -google-runtime-references: | ||
| # Allow non-const refereces as function parameter or member variable per Envoy style. | ||
| Checks: 'clang-diagnostic-*,clang-analyzer-*,*,-google-runtime-references,-llvm-header-guard,-clang-diagnostic-pragma-once-outside-header,-fuchsia-*,-android-*,-cppcoreguidelines-pro-type-vararg,-hicpp-va-arg' | ||
| WarningsAsErrors: '' | ||
| HeaderFilterRegex: '' | ||
| AnalyzeTemporaryDtors: false | ||
| FormatStyle: none | ||
| CheckOptions: | ||
| - key: abseil-string-find-startswith.AbseilStringsMatchHeader | ||
| value: absl/strings/match.h | ||
| - key: abseil-string-find-startswith.IncludeStyle | ||
| value: llvm | ||
| - key: abseil-string-find-startswith.StringLikeClasses | ||
| value: '::std::basic_string' | ||
| - key: bugprone-argument-comment.StrictMode | ||
| value: '0' | ||
| - key: bugprone-assert-side-effect.AssertMacros | ||
| value: assert | ||
| - key: bugprone-assert-side-effect.CheckFunctionCalls | ||
| value: '0' | ||
| - key: bugprone-dangling-handle.HandleClasses | ||
| value: 'std::basic_string_view;std::experimental::basic_string_view' | ||
| - key: bugprone-exception-escape.FunctionsThatShouldNotThrow | ||
| value: '' | ||
| - key: bugprone-exception-escape.IgnoredExceptions | ||
| value: '' | ||
| - key: bugprone-misplaced-widening-cast.CheckImplicitCasts | ||
| value: '0' | ||
| - key: bugprone-sizeof-expression.WarnOnSizeOfCompareToConstant | ||
| value: '1' | ||
| - key: bugprone-sizeof-expression.WarnOnSizeOfConstant | ||
| value: '1' | ||
| - key: bugprone-sizeof-expression.WarnOnSizeOfIntegerExpression | ||
| value: '0' | ||
| - key: bugprone-sizeof-expression.WarnOnSizeOfThis | ||
| value: '1' | ||
| - key: bugprone-string-constructor.LargeLengthThreshold | ||
| value: '8388608' | ||
| - key: bugprone-string-constructor.WarnOnLargeLength | ||
| value: '1' | ||
| - key: bugprone-suspicious-enum-usage.StrictMode | ||
| value: '0' | ||
| - key: bugprone-suspicious-missing-comma.MaxConcatenatedTokens | ||
| value: '5' | ||
| - key: bugprone-suspicious-missing-comma.RatioThreshold | ||
| value: '0.200000' | ||
| - key: bugprone-suspicious-missing-comma.SizeThreshold | ||
| value: '5' | ||
| - key: bugprone-suspicious-string-compare.StringCompareLikeFunctions | ||
| value: '' | ||
| - key: bugprone-suspicious-string-compare.WarnOnImplicitComparison | ||
| value: '1' | ||
| - key: bugprone-suspicious-string-compare.WarnOnLogicalNotComparison | ||
| value: '0' | ||
| - key: bugprone-unused-return-value.CheckedFunctions | ||
| value: '::std::async;::std::launder;::std::remove;::std::remove_if;::std::unique;::std::unique_ptr::release;::std::basic_string::empty;::std::vector::empty' | ||
| - key: cert-dcl59-cpp.HeaderFileExtensions | ||
| value: ',h,hh,hpp,hxx' | ||
| - key: cert-err09-cpp.CheckThrowTemporaries | ||
| value: '1' | ||
| - key: cert-err61-cpp.CheckThrowTemporaries | ||
| value: '1' | ||
| - key: cert-msc32-c.DisallowedSeedTypes | ||
| value: 'time_t,std::time_t' | ||
| - key: cert-msc51-cpp.DisallowedSeedTypes | ||
| value: 'time_t,std::time_t' | ||
| - key: cert-oop11-cpp.IncludeStyle | ||
| value: llvm | ||
| - key: cppcoreguidelines-no-malloc.Allocations | ||
| value: '::malloc;::calloc' | ||
| - key: cppcoreguidelines-no-malloc.Deallocations | ||
| value: '::free' | ||
| - key: cppcoreguidelines-no-malloc.Reallocations | ||
| value: '::realloc' | ||
| - key: cppcoreguidelines-owning-memory.LegacyResourceConsumers | ||
| value: '::free;::realloc;::freopen;::fclose' | ||
| - key: cppcoreguidelines-owning-memory.LegacyResourceProducers | ||
| value: '::malloc;::aligned_alloc;::realloc;::calloc;::fopen;::freopen;::tmpfile' | ||
| - key: cppcoreguidelines-pro-bounds-constant-array-index.GslHeader | ||
| value: '' | ||
| - key: cppcoreguidelines-pro-bounds-constant-array-index.IncludeStyle | ||
| value: '0' | ||
| - key: cppcoreguidelines-pro-type-member-init.IgnoreArrays | ||
| value: '0' | ||
| - key: cppcoreguidelines-special-member-functions.AllowMissingMoveFunctions | ||
| value: '0' | ||
| - key: cppcoreguidelines-special-member-functions.AllowSoleDefaultDtor | ||
| value: '0' | ||
| - key: fuchsia-header-anon-namespaces.HeaderFileExtensions | ||
| value: ',h,hh,hpp,hxx' | ||
| - key: fuchsia-restrict-system-includes.Includes | ||
| value: '*' | ||
| - key: google-build-namespaces.HeaderFileExtensions | ||
| value: ',h,hh,hpp,hxx' | ||
| - key: google-global-names-in-headers.HeaderFileExtensions | ||
| value: ',h,hh,hpp,hxx' | ||
| - key: google-readability-braces-around-statements.ShortStatementLines | ||
| value: '1' | ||
| - key: google-readability-function-size.BranchThreshold | ||
| value: '4294967295' | ||
| - key: google-readability-function-size.LineThreshold | ||
| value: '4294967295' | ||
| - key: google-readability-function-size.NestingThreshold | ||
| value: '4294967295' | ||
| - key: google-readability-function-size.ParameterThreshold | ||
| value: '4294967295' | ||
| - key: google-readability-function-size.StatementThreshold | ||
| value: '800' | ||
| - key: google-readability-function-size.VariableThreshold | ||
| value: '4294967295' | ||
| - key: google-readability-namespace-comments.ShortNamespaceLines | ||
| value: '10' | ||
| - key: google-readability-namespace-comments.SpacesBeforeComments | ||
| value: '2' | ||
| - key: google-runtime-int.SignedTypePrefix | ||
| value: int | ||
| - key: google-runtime-int.TypeSuffix | ||
| value: '' | ||
| - key: google-runtime-int.UnsignedTypePrefix | ||
| value: uint | ||
| - key: hicpp-braces-around-statements.ShortStatementLines | ||
| value: '0' | ||
| - key: hicpp-function-size.BranchThreshold | ||
| value: '4294967295' | ||
| - key: hicpp-function-size.LineThreshold | ||
| value: '4294967295' | ||
| - key: hicpp-function-size.NestingThreshold | ||
| value: '4294967295' | ||
| - key: hicpp-function-size.ParameterThreshold | ||
| value: '4294967295' | ||
| - key: hicpp-function-size.StatementThreshold | ||
| value: '800' | ||
| - key: hicpp-function-size.VariableThreshold | ||
| value: '4294967295' | ||
| - key: hicpp-member-init.IgnoreArrays | ||
| value: '0' | ||
| - key: hicpp-move-const-arg.CheckTriviallyCopyableMove | ||
| value: '1' | ||
| - key: hicpp-multiway-paths-covered.WarnOnMissingElse | ||
| value: '0' | ||
| - key: hicpp-named-parameter.IgnoreFailedSplit | ||
| value: '0' | ||
| - key: hicpp-no-malloc.Allocations | ||
| value: '::malloc;::calloc' | ||
| - key: hicpp-no-malloc.Deallocations | ||
| value: '::free' | ||
| - key: hicpp-no-malloc.Reallocations | ||
| value: '::realloc' | ||
| - key: hicpp-special-member-functions.AllowMissingMoveFunctions | ||
| value: '0' | ||
| - key: hicpp-special-member-functions.AllowSoleDefaultDtor | ||
| value: '0' | ||
| - key: hicpp-use-auto.MinTypeNameLength | ||
| value: '5' | ||
| - key: hicpp-use-auto.RemoveStars | ||
| value: '0' | ||
| - key: hicpp-use-emplace.ContainersWithPushBack | ||
| value: '::std::vector;::std::list;::std::deque' | ||
| - key: hicpp-use-emplace.SmartPointers | ||
| value: '::std::shared_ptr;::std::unique_ptr;::std::auto_ptr;::std::weak_ptr' | ||
| - key: hicpp-use-emplace.TupleMakeFunctions | ||
| value: '::std::make_pair;::std::make_tuple' | ||
| - key: hicpp-use-emplace.TupleTypes | ||
| value: '::std::pair;::std::tuple' | ||
| - key: hicpp-use-equals-default.IgnoreMacros | ||
| value: '1' | ||
| - key: hicpp-use-noexcept.ReplacementString | ||
| value: '' | ||
| - key: hicpp-use-noexcept.UseNoexceptFalse | ||
| value: '1' | ||
| - key: hicpp-use-nullptr.NullMacros | ||
| value: '' | ||
| - key: llvm-namespace-comment.ShortNamespaceLines | ||
| value: '1' | ||
| - key: llvm-namespace-comment.SpacesBeforeComments | ||
| value: '1' | ||
| - key: misc-definitions-in-headers.HeaderFileExtensions | ||
| value: ',h,hh,hpp,hxx' | ||
| - key: misc-definitions-in-headers.UseHeaderFileExtension | ||
| value: '1' | ||
| - key: misc-throw-by-value-catch-by-reference.CheckThrowTemporaries | ||
| value: '1' | ||
| - key: misc-unused-parameters.StrictMode | ||
| value: '0' | ||
| - key: modernize-loop-convert.MaxCopySize | ||
| value: '16' | ||
| - key: modernize-loop-convert.MinConfidence | ||
| value: reasonable | ||
| - key: modernize-loop-convert.NamingStyle | ||
| value: CamelCase | ||
| - key: modernize-make-shared.IgnoreMacros | ||
| value: '1' | ||
| - key: modernize-make-shared.IncludeStyle | ||
| value: '0' | ||
| - key: modernize-make-shared.MakeSmartPtrFunction | ||
| value: 'std::make_shared' | ||
| - key: modernize-make-shared.MakeSmartPtrFunctionHeader | ||
| value: memory | ||
| - key: modernize-make-unique.IgnoreMacros | ||
| value: '1' | ||
| - key: modernize-make-unique.IncludeStyle | ||
| value: '0' | ||
| - key: modernize-make-unique.MakeSmartPtrFunction | ||
| value: 'std::make_unique' | ||
| - key: modernize-make-unique.MakeSmartPtrFunctionHeader | ||
| value: memory | ||
| - key: modernize-pass-by-value.IncludeStyle | ||
| value: llvm | ||
| - key: modernize-pass-by-value.ValuesOnly | ||
| value: '0' | ||
| - key: modernize-raw-string-literal.ReplaceShorterLiterals | ||
| value: '0' | ||
| - key: modernize-replace-auto-ptr.IncludeStyle | ||
| value: llvm | ||
| - key: modernize-replace-random-shuffle.IncludeStyle | ||
| value: llvm | ||
| - key: modernize-use-auto.MinTypeNameLength | ||
| value: '5' | ||
| - key: modernize-use-auto.RemoveStars | ||
| value: '0' | ||
| - key: modernize-use-default-member-init.IgnoreMacros | ||
| value: '1' | ||
| - key: modernize-use-default-member-init.UseAssignment | ||
| value: '0' | ||
| - key: modernize-use-emplace.ContainersWithPushBack | ||
| value: '::std::vector;::std::list;::std::deque' | ||
| - key: modernize-use-emplace.SmartPointers | ||
| value: '::std::shared_ptr;::std::unique_ptr;::std::auto_ptr;::std::weak_ptr' | ||
| - key: modernize-use-emplace.TupleMakeFunctions | ||
| value: '::std::make_pair;::std::make_tuple' | ||
| - key: modernize-use-emplace.TupleTypes | ||
| value: '::std::pair;::std::tuple' | ||
| - key: modernize-use-equals-default.IgnoreMacros | ||
| value: '1' | ||
| - key: modernize-use-noexcept.ReplacementString | ||
| value: '' | ||
| - key: modernize-use-noexcept.UseNoexceptFalse | ||
| value: '1' | ||
| - key: modernize-use-nullptr.NullMacros | ||
| value: 'NULL' | ||
| - key: modernize-use-transparent-functors.SafeMode | ||
| value: '0' | ||
| - key: modernize-use-using.IgnoreMacros | ||
| value: '1' | ||
| - key: objc-forbidden-subclassing.ForbiddenSuperClassNames | ||
| value: 'ABNewPersonViewController;ABPeoplePickerNavigationController;ABPersonViewController;ABUnknownPersonViewController;NSHashTable;NSMapTable;NSPointerArray;NSPointerFunctions;NSTimer;UIActionSheet;UIAlertView;UIImagePickerController;UITextInputMode;UIWebView' | ||
| - key: objc-property-declaration.Acronyms | ||
| value: '' | ||
| - key: objc-property-declaration.IncludeDefaultAcronyms | ||
| value: '1' | ||
| - key: performance-faster-string-find.StringLikeClasses | ||
| value: 'std::basic_string' | ||
| - key: performance-for-range-copy.WarnOnAllAutoCopies | ||
| value: '0' | ||
| - key: performance-inefficient-string-concatenation.StrictMode | ||
| value: '0' | ||
| - key: performance-inefficient-vector-operation.VectorLikeClasses | ||
| value: '::std::vector' | ||
| - key: performance-move-const-arg.CheckTriviallyCopyableMove | ||
| value: '1' | ||
| - key: performance-move-constructor-init.IncludeStyle | ||
| value: llvm | ||
| - key: performance-type-promotion-in-math-fn.IncludeStyle | ||
| value: llvm | ||
| - key: performance-unnecessary-value-param.IncludeStyle | ||
| value: llvm | ||
| - key: portability-simd-intrinsics.Std | ||
| value: '' | ||
| - key: portability-simd-intrinsics.Suggest | ||
| value: '0' | ||
| - key: readability-braces-around-statements.ShortStatementLines | ||
| value: '0' | ||
| - key: readability-function-size.BranchThreshold | ||
| value: '4294967295' | ||
| - key: readability-function-size.LineThreshold | ||
| value: '4294967295' | ||
| - key: readability-function-size.NestingThreshold | ||
| value: '4294967295' | ||
| - key: readability-function-size.ParameterThreshold | ||
| value: '4294967295' | ||
| - key: readability-function-size.StatementThreshold | ||
| value: '800' | ||
| - key: readability-function-size.VariableThreshold | ||
| value: '4294967295' | ||
| - key: readability-identifier-naming.IgnoreFailedSplit | ||
| value: '0' | ||
| - key: readability-implicit-bool-conversion.AllowIntegerConditions | ||
| value: '0' | ||
| - key: readability-implicit-bool-conversion.AllowPointerConditions | ||
| value: '0' | ||
| - key: readability-inconsistent-declaration-parameter-name.IgnoreMacros | ||
| value: '1' | ||
| - key: readability-inconsistent-declaration-parameter-name.Strict | ||
| value: '0' | ||
| - key: readability-simplify-boolean-expr.ChainedConditionalAssignment | ||
| value: '0' | ||
| - key: readability-simplify-boolean-expr.ChainedConditionalReturn | ||
| value: '0' | ||
| - key: readability-simplify-subscript-expr.Types | ||
| value: '::std::basic_string;::std::basic_string_view;::std::vector;::std::array' | ||
| - key: readability-static-accessed-through-instance.NameSpecifierNestingThreshold | ||
| value: '3' | ||
| - key: zircon-temporary-objects.Names | ||
| value: '' | ||
| ... | ||
|
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.