[Overview] Modernization code with clang-tidy

[SunBlack]

This issue is just to have an overview about clang-tidy modernizations.

Full list of clang-tidy checks.

Checks which redirect to another test are removed from following list.

Android:

• [x] android-cloexec-accept (Nothing found)
• [x] android-cloexec-accept4 (Nothing found)
• [x] android-cloexec-creat (Nothing found)
• [x] android-cloexec-dup (Nothing found)
• [x] android-cloexec-epoll-create (Nothing found)
• [x] android-cloexec-epoll-create1 (Nothing found)
• [x] android-cloexec-fopen (discussion: #2748, PR: #2755, #3223)
• [x] android-cloexec-inotify-init (Nothing found)
• [x] android-cloexec-inotify-init1 (Nothing found)
• [x] android-cloexec-memfd-create (Nothing found)
• [x] android-cloexec-open (discussion: #2748, PR: #2755)
• [x] android-cloexec-pipe (Nothing found)
• [x] android-cloexec-pipe2 (Nothing found)
• [x] android-cloexec-socket (Nothing found)
• [x] android-comparison-in-temp-failure-retry (Nothing found)

Boost:

• [x] boost-use-to-string (PR: #2785)

Bugprone:

• [x] bugprone-argument-comment (Nothing found)
• [x] bugprone-assert-side-effect (Nothing found)
• [ ] bugprone-bool-pointer-implicit-conversion
• [x] bugprone-branch-clone (Nothing found)
• [ ] bugprone-copy-constructor-init
• [x] bugprone-dangling-handle (Nothing found)
• [ ] bugprone-exception-escape
• [x] bugprone-fold-init-type (Nothing found)
• [ ] bugprone-forward-declaration-namespace
• [x] bugprone-forwarding-reference-overload (Nothing found)
• [x] bugprone-inaccurate-erase (Nothing found)
• [ ] bugprone-incorrect-roundings
• [ ] bugprone-integer-division
• [x] bugprone-lambda-function-name (Nothing found)
• [ ] bugprone-macro-parentheses
• [x] bugprone-macro-repeated-side-effects (Nothing found)
• [x] bugprone-misplaced-operator-in-strlen-in-alloc (Nothing found)
• [ ] bugprone-misplaced-widening-cast
• [x] bugprone-move-forwarding-reference (Nothing found)
• [ ] bugprone-multiple-statement-macro
• [ ] bugprone-narrowing-conversions
• [ ] bugprone-parent-virtual-call
• [x] bugprone-posix-return (Nothing found)
• [x] bugprone-sizeof-container (Nothing found)
• [ ] bugprone-sizeof-expression
• [x] bugprone-string-constructor (Nothing found)
• [x] bugprone-string-integer-assignment (Nothing found)
• [x] bugprone-string-literal-with-embedded-nul (Nothing found)
• [ ] bugprone-suspicious-enum-usage
• [x] bugprone-suspicious-memset-usage (Nothing found)
• [x] bugprone-suspicious-missing-comma (Nothing found)
• [x] bugprone-suspicious-semicolon (Nothing found)
• [ ] bugprone-suspicious-string-compare
• [x] bugprone-swapped-arguments (Nothing found)
• [x] bugprone-terminating-continue (Nothing found)
• [x] bugprone-throw-keyword-missing (Nothing found)
• [x] bugprone-too-small-loop-variable (PR: #2829)
• [ ] bugprone-undefined-memory-manipulation
• [x] bugprone-undelegated-constructor (Nothing found)
• [x] bugprone-unhandled-self-assignment (Nothing found)
• [x] bugprone-unused-raii (Nothing found)
• [x] bugprone-unused-return-value (Nothing found)
• [x] bugprone-use-after-move (Nothing found)
• [ ] bugprone-virtual-near-miss

Cert:

• [ ] cert-dcl21-cpp
• [ ] cert-dcl50-cpp
• [x] cert-dcl58-cpp (Nothing found)
• [x] cert-env33-c (Nothing found)
• [ ] cert-err34-c
• [ ] cert-err52-cpp
• [ ] cert-err58-cpp
• [ ] cert-err60-cpp
• [ ] cert-flp30-c
• [ ] cert-msc50-cpp
• [ ] cert-msc51-cpp
• [x] cert-oop11-cp (Nothing found)

Clang-Analyzer:

• [x] clang-analyzer-apiModeling.StdCLibraryFunctions (Nothing found)
• [x] clang-analyzer-apiModeling.TrustNonnull (Nothing found)
• [x] clang-analyzer-apiModeling.google.GTest (Nothing found)
• [ ] clang-analyzer-core.CallAndMessage
• [ ] clang-analyzer-core.DivideZero
• [x] clang-analyzer-core.DynamicTypePropagation (Nothing found)
• [ ] clang-analyzer-core.NonNullParamChecker
• [x] clang-analyzer-core.NonnilStringConstants (Nothing found)
• [ ] clang-analyzer-core.NullDereference
• [x] clang-analyzer-core.StackAddressEscape (Nothing found)
• [x] clang-analyzer-core.UndefinedBinaryOperatorResult (Nothing found)
• [x] clang-analyzer-core.VLASize (Nothing found)
• [x] clang-analyzer-core.builtin.BuiltinFunctions (Nothing found)
• [x] clang-analyzer-core.builtin.NoReturnFunctions (Nothing found)
• [ ] clang-analyzer-core.uninitialized.ArraySubscript
• [ ] clang-analyzer-core.uninitialized.Assign
• [ ] clang-analyzer-core.uninitialized.Branch
• [x] clang-analyzer-core.uninitialized.CapturedBlockVariable (Nothing found)
• [ ] clang-analyzer-core.uninitialized.UndefReturn
• [x] clang-analyzer-cplusplus.InnerPointer (Nothing found)
• [x] clang-analyzer-cplusplus.Move (Nothing found)
• [ ] clang-analyzer-cplusplus.NewDelete
• [ ] clang-analyzer-cplusplus.NewDeleteLeaks
• [x] clang-analyzer-cplusplus.SelfAssignment (Nothing found)
• [x] clang-analyzer-deadcode.DeadStores (PR: #3095)
• clang-analyzer-llvm.Conventions (check not released in a stable version of clang-tidy)
• [x] clang-analyzer-nullability.NullPassedToNonnull (Nothing found)
• [x] clang-analyzer-nullability.NullReturnedFromNonnull (Nothing found)
• [x] clang-analyzer-nullability.NullableDereferenced (Nothing found)
• [x] clang-analyzer-nullability.NullablePassedToNonnull (Nothing found)
• [x] clang-analyzer-nullability.NullableReturnedFromNonnull (Nothing found)
• [ ] clang-analyzer-optin.cplusplus.VirtualCall
• [x] clang-analyzer-optin.mpi.MPI-Checker (Nothing found)
• [x] clang-analyzer-optin.osx.cocoa.localizability.EmptyLocalizationContextChecker (Nothing found)
• [x] clang-analyzer-optin.osx.cocoa.localizability.NonLocalizedStringChecker (Nothing found)
• [x] clang-analyzer-optin.performance.GCDAntipattern (Nothing found)
• [ ] clang-analyzer-optin.performance.Padding
• [x] clang-analyzer-optin.portability.UnixAPI (Nothing found)
• [x] clang-analyzer-osx.API (Nothing found)
• [x] clang-analyzer-osx.NumberObjectConversion (Nothing found)
• [x] clang-analyzer-osx.OSObjectRetainCount (Nothing found)
• [x] clang-analyzer-osx.ObjCProperty (Nothing found)
• [x] clang-analyzer-osx.SecKeychainAPI (Nothing found)
• [x] clang-analyzer-osx.cocoa.AtSync (Nothing found)
• [x] clang-analyzer-osx.cocoa.AutoreleaseWrite (Nothing found)
• [x] clang-analyzer-osx.cocoa.ClassRelease (Nothing found)
• [x] clang-analyzer-osx.cocoa.Dealloc (Nothing found)
• [x] clang-analyzer-osx.cocoa.IncompatibleMethodTypes (Nothing found)
• [x] clang-analyzer-osx.cocoa.Loops (Nothing found)
• [x] clang-analyzer-osx.cocoa.MissingSuperCall (Nothing found)
• [x] clang-analyzer-osx.cocoa.NSAutoreleasePool (Nothing found)
• [x] clang-analyzer-osx.cocoa.NSError (Nothing found)
• [x] clang-analyzer-osx.cocoa.NilArg (Nothing found)
• [x] clang-analyzer-osx.cocoa.NonNilReturnValue (Nothing found)
• [x] clang-analyzer-osx.cocoa.ObjCGenerics (Nothing found)
• [x] clang-analyzer-osx.cocoa.RetainCount (Nothing found)
• [x] clang-analyzer-osx.cocoa.RunLoopAutoreleaseLeak (Nothing found)
• [x] clang-analyzer-osx.cocoa.SelfInit (Nothing found)
• [x] clang-analyzer-osx.cocoa.SuperDealloc (Nothing found)
• [x] clang-analyzer-osx.cocoa.UnusedIvars (Nothing found)
• [x] clang-analyzer-osx.cocoa.VariadicMethodTypes (Nothing found)
• [x] clang-analyzer-osx.coreFoundation.CFError (Nothing found)
• [x] clang-analyzer-osx.coreFoundation.CFNumber (Nothing found)
• [x] clang-analyzer-osx.coreFoundation.CFRetainRelease (Nothing found)
• [x] clang-analyzer-osx.coreFoundation.containers.OutOfBounds (Nothing found)
• [x] clang-analyzer-osx.coreFoundation.containers.PointerSizedValues (Nothing found)
• [ ] clang-analyzer-security.FloatLoopCounter
• [x] clang-analyzer-security.insecureAPI.UncheckedReturn (Nothing found)
• [x] clang-analyzer-security.insecureAPI.bcmp (Nothing found)
• [x] clang-analyzer-security.insecureAPI.bcopy (Nothing found)
• [x] clang-analyzer-security.insecureAPI.bzero (Nothing found)
• [x] clang-analyzer-security.insecureAPI.getpw (Nothing found)
• [x] clang-analyzer-security.insecureAPI.gets (Nothing found)
• [x] clang-analyzer-security.insecureAPI.mkstemp (Nothing found)
• [x] clang-analyzer-security.insecureAPI.mktemp (Nothing found)
• [x] clang-analyzer-security.insecureAPI.rand (Nothing found)
• [ ] clang-analyzer-security.insecureAPI.strcpy
• [x] clang-analyzer-security.insecureAPI.vfork (Nothing found)
• [ ] clang-analyzer-unix.API
• [ ] clang-analyzer-unix.Malloc
• [x] clang-analyzer-unix.MallocSizeof (Nothing found)
• [ ] clang-analyzer-unix.MismatchedDeallocator
• clang-analyzer-unix.StdCLibraryFunctions (check not released in a stable version of clang-tidy)
• [x] clang-analyzer-unix.Vfork (Nothing found)
• [x] clang-analyzer-unix.cstring.BadSizeArg (Nothing found)
• [x] clang-analyzer-unix.cstring.NullArg (Nothing found)
• [x] clang-analyzer-valist.CopyToSelf (Nothing found)
• [x] clang-analyzer-valist.Uninitialized (Nothing found)
• [x] clang-analyzer-valist.Unterminated (Nothing found)

cppcoreguidelines

• [ ] cppcoreguidelines-avoid-goto
• [x] cppcoreguidelines-interfaces-global-init (Nothing found)
• [ ] cppcoreguidelines-macro-usage
• [ ] cppcoreguidelines-narrowing-conversions
• [ ] cppcoreguidelines-no-malloc
• [ ] cppcoreguidelines-owning-memory
• [ ] cppcoreguidelines-pro-bounds-array-to-pointer-decay
• [ ] cppcoreguidelines-pro-bounds-constant-array-index
• [ ] cppcoreguidelines-pro-bounds-pointer-arithmetic
• [ ] cppcoreguidelines-pro-type-const-cast
• [ ] cppcoreguidelines-pro-type-cstyle-cast
• [ ] cppcoreguidelines-pro-type-member-init
• [ ] cppcoreguidelines-pro-type-reinterpret-cast
• [ ] cppcoreguidelines-pro-type-static-cast-downcast
• [ ] cppcoreguidelines-pro-type-union-access
• [ ] cppcoreguidelines-pro-type-vararg
• [ ] cppcoreguidelines-slicing
• [ ] cppcoreguidelines-special-member-functions

hicpp:

• [ ] hicpp-avoid-goto
• [ ] hicpp-exception-baseclass
• [ ] hicpp-multiway-paths-covered
• [x] hicpp-no-assembler (Nothing found)
• [ ] hicpp-signed-bitwise

Misc:

• [ ] misc-definitions-in-headers
• [ ] misc-misplaced-const
• [x] misc-new-delete-overloads (Nothing found)
• [x] misc-non-copyable-objects (Nothing found)
• [ ] misc-non-private-member-variables-in-classes
• [ ] misc-redundant-expression
• [x] misc-static-assert (Nothing found)
• [x] misc-throw-by-value-catch-by-reference (Nothing found)
• [ ] misc-unconventional-assign-operator
• [x] misc-uniqueptr-reset-release (Nothing found)
• [x] misc-unused-alias-decls (Nothing found)
• [ ] misc-unused-parameters
• [ ] misc-unused-using-decls

Modernize:

• [x] modernize-avoid-bind (Nothing found)
• [ ] modernize-avoid-c-arrays (PR: #4489)
• [x] ~~modernize-concat-nested-namespaces~~ (requires C++17)
• [x] modernize-deprecated-headers (PR: #2737)
• [ ] modernize-deprecated-ios-base-aliases
• [x] modernize-loop-convert (PR: #2835, #2836, #2837, #2838, #2839, #2840, #2841, #2842, #2843, #2844, #2844, #2845, #2846, #2847, #2848, #2849, #2850, #2851, #2853, #2854, #2855, #2856, #2857, #2858, #2859, #2860)
• [x] modernize-make-shared (Nothing found)
• [x] modernize-make-unique (Nothing found)
• [ ] modernize-pass-by-value
• [x] modernize-raw-string-literal (PR: #2789)
• [x] modernize-redundant-void-arg (PR: #2780)
• [x] modernize-replace-auto-ptr (Nothing found)
• [x] modernize-replace-random-shuffle (PR: #3069)
• [x] modernize-return-braced-init-list (PR: #3039)
• [x] modernize-shrink-to-fit (Nothing found)
• [x] ~~modernize-unary-static-assert~~ (requires C++17)
• [x] ~~modernize-use-auto~~ (developer decision, so no auto transformation, see discussion in #2838)
• [x] modernize-use-bool-literals (PR: #2793)
• [ ] modernize-use-default-member-init
• [x] modernize-use-emplace (PR: #2784)
• [ ] modernize-use-equals-default (PR: #5386)
• [x] modernize-use-equals-delete (PR: #2779)
• [x] ~~modernize-use-nodiscard~~ (requires C++17)
• [ ] modernize-use-noexcept
• [x] modernize-use-nullptr (PR: #3004, #3005, #3006, #3007, #3008, #3009, #3010, #3011, #3012, #3013, #3014, #3015, #3016, #3017, #3018, #3019, #3020, #3021, #3022, #3023, #3024, #3025, #3026, #3027, #3028, #3029)
• [x] modernize-use-override (PR: #2728)
• modernize-use-trailing-return-type (check not released in a stable version of clang-tidy)
• [x] modernize-use-transparent-functors (PR: #3224)
• [x] ~~modernize-use-uncaught-exceptions~~ (requires C++17)
• [x] modernize-use-using (PR: #3112, #3113, #3115, #3117, #3118, #3121, #3122, #3123, #3124, #3125, #3129, #3130, #3132, #3134, #3137, #3138, #3139)

MPI

• [x] mpi-buffer-deref (Nothing found)
• [x] mpi-type-mismatch (Nothing found)

OpenMP

• [x] openmp-exception-escape (Nothing found)
• [x] openmp-use-default-none (PR: #3524)

Performance

• [x] performance-faster-string-find (PR: #2794)
• [x] performance-for-range-copy (Nothing found)
• [ ] performance-implicit-conversion-in-loop (requires #3234)
• [x] performance-inefficient-algorithm (Nothing found)
• [x] ~~performance-inefficient-string-concatenation~~ (discussion: #3233)
• [x] performance-inefficient-vector-operation (Nothing found)
• [x] performance-move-const-arg (Nothing found)
• [x] performance-move-constructor-init (Nothing found)
• [x] performance-noexcept-move-constructor (Nothing found)
• [x] performance-type-promotion-in-math-fn (PR: #3087)
• [ ] performance-unnecessary-copy-initialization
• [ ] performance-unnecessary-value-param (PR: #3232, #3253)

Readability:

• [ ] readability-avoid-const-params-in-decls
• [ ] readability-braces-around-statements
• [ ] readability-const-return-type
• [x] readability-container-size-empty (PR: #3033)
• [x] readability-delete-null-pointer (PR: #2990)
• [x] readability-deleted-default (Nothing found)
• [x] readability-else-after-return (PR: #3180, #3181, #3182, #3183, #3184, #3185, #3186)
• [ ] readability-function-size
• [x] readability-identifier-naming (Nothing found)
• [ ] readability-implicit-bool-conversion
• [ ] readability-inconsistent-declaration-parameter-name
• [ ] readability-isolate-declaration
• [ ] readability-magic-numbers
• [x] readability-make-member-function-const (PR: #3836)
• [ ] readability-misleading-indentation
• [x] readability-misplaced-array-index (Nothing found)
• [ ] readability-named-parameter
• [ ] readability-non-const-parameter
• [ ] readability-redundant-control-flow
• [x] readability-redundant-declaration (Nothing found)
• [x] readability-redundant-function-ptr-dereference (Nothing found)
• [x] readability-redundant-member-init (PR: #3070)
• [x] readability-redundant-preprocessor (Nothing found)
• [x] readability-redundant-smartptr-get (Nothing found)
• [x] readability-redundant-string-cstr (PR: #3517)
• [x] readability-redundant-string-init (PR: #2758)
• [x] readability-simplify-boolean-expr (PR: #2790)
• [ ] readability-simplify-subscript-expr
• [x] readability-static-accessed-through-instance (PR: #2776)
• [x] readability-static-definition-in-anonymous-namespace (Nothing found)
• [x] readability-string-compare (PR: #2986)
• [x] readability-uniqueptr-delete-release (Nothing found)
• [ ] readability-uppercase-literal-suffix

Not relevant checks:

• abseil-*
• fuchsia-*
• google-*
• llvm-*
• objc-*
• portability-simd-intrinsics (experimental part of std)
• zircon-*

In case you see a modernization we could apply or can't apply, feel free to write it as comment. In case there is a modernization that needs discussion, open a new issue and write reference to it as comment, so I can add it to this list.

[jasjuang]

I think modernize-use-auto makes a lot of sense for iterators, but for new expressions and cast expressions to make sense, MinTypeNameLength has to be at least larger or equal the default 5.

modernize-avoid-c-arrays and modernize-deprecated-headers is definitely needed.

I think modernize-make-shared, modernize-make-unique, modernize-use-emplace, modernize-use-nullptr are also good to add in.

[taketwo]

modernize-use-auto: be very careful with Eigen types. Basically, do not use the auto keywords with Eigen's expressions, unless you are 100% sure about what you are doing (ref).

[SunBlack]

@jasjuang modernize-make-shared & modernize-make-unique will not have any effect, because we didn't had C++11 before ;).

[SunBlack]

I added two points for discussion.

[SunBlack]

I think modernize-use-auto makes a lot of sense for iterators, but for new expressions and cast expressions to make sense, MinTypeNameLength has to be at least larger or equal the default 5.

@jasjuang Discussion is for this is currently here: #2838 - in case you want to be part of it ;)

[denix56]

Maybe we should also replace typedef with using ? Clang also suggests it as modernization. Moreover, it is, imho, much more readable

[taketwo]

It'd certainly be nice to switch to using in the long run. @SunBlack already attempted to apply automatic fix, but there were problems, see #2791.

[SunBlack]

I suggest to skip modernize-use-default-member-init, because I really don't like to have default values stored in header.

[taketwo]

Generally, I agree with you on this. However, due to the fact that PCL is a heavily templated library, we already have default values in ".h" files most of the times. And even if we don't have them in ".h", then they are in ".hpp", which is effectively the same. Meantime, there are benefits in using default member init. For example, when there are multiple constructors (which is often the case in PCL) it helps to maintain consistency between default values.

[SergioRAgostinho]

Just wanted to add here the decision we took for readability-implicit-bool-conversion. tldr activate the option AllowPointerConditions at that time. See this comment for reference

[SunBlack]

Just to mention: I don't believe we can integrate clang-tidy in near feature into build server. I tried it in our project, but there are some issues:

• I have no idea how header-filter really works. It should be a Regex, but I don't know which syntax they allow, because all my regexe works just partially (need to exclude moc and ui files of Qt from output)
• If we switch to AUTOMOC, AUTOUI and AUTORCC, these file will not anymore generated during CMake generation phase. Instead they will be generated at start of a build (there are _autogen targets then). So we need to build all _autogen targets to get run clang-tidy (if not clang-tidy will complain missing moc/ui/rcc files). Because there is no global autogen target, we need to write a script to grep all _autogen targets of makefile, because makefiles doesn't support wildcard, so we can't call make *_autogen. Easiest solution use CMake 3.14 and CMAKE_EXPORT_COMPILE_COMMANDS=ON
• Output of clang-tidy 6.0 / 7 / 8 produces sometimes corrupted output, if you parse console output (starts printing output a hit within another hit). So in case we want to parse output, we need to use a YAML parser (this output seems to be valid)
• Not sure how long a run will need on Azure, could be longer than MSVC build ;).

[taketwo]

Not sure how long a run will need on Azure, could be longer than MSVC build ;).

:scream: In case it turns out to be very expensive to run, we may consider making it a periodic job which runs, say, weekly.

But the rest of the items you posted make this all sound like a very daunting task.

[SunBlack]

General question, due to:

Lack of time to review. Like I said, I'm not putting any priority in reviewing these PRs because they are time consuming and not in the goals for this release. That being said, they are fairly trivial in the changes they make, so feel I'm perfectly ok in merging them with just your review.

Originally posted by @SergioRAgostinho in https://github.com/PointCloudLibrary/pcl/pull/3112#issuecomment-500226328

Should I stop applying modernization until release of 1.10.0 or still continue?

[SergioRAgostinho]

If you have time to spare, I would rather have your help on the transition to std until the release is complete. There are a couple of items now that are simply manual labor, e.g.: the bind -> lambda replacement. But let Sergey express his opinion, since he was still reviewing things.

[taketwo]

I am fine reviewing/merging clang-tidy conversions. But indeed help on the milestone items would be more important.

[SunBlack]

Well ok, don't know if I really have (currently) time to help on boost. There is a small thing I will change, if I have time (PCL is using boost filesystem still via iterator instead of for-ranged loop).

Nevertheless I will continue with some clang-tidy changes, because I can run clang-tidy if I don't need my PC and just need to review it after it .

[SunBlack]

Updated list, so progress is better to see.

[SunBlack]

I did run this night clang-tidy with all checks on our build server, to get a better feeling, which checks will produce smaller PRs and which bigger one. Well, this did run a bit out of control 😆

Job statistic:

• Error Log-File Size of clang-tidy: 287.71MB
• clang-tidy time: 1h 13min
• Java parsing time: Still not finished after > 7h => to much for the Jenkins Plugin xD.

Following stats are manually grepped via regex. So no duplicating detection was done.

General warnings statistic:

• Total: 869 173
• Outside of PCL code: 12 251
• Within the 3rd-Party code of PCL: 56 167
• Within the non-3rd-Party code of PCL: 809 755

Categories:

• abseil: 0
• android: 2
• boost: 0
• bugprone: 6 461
• cert: 2 834
• clang-analyzer: 791
• cppcoreguidelines: 246 098
• fuchsia: 153 825
• google: 151 613
• hicpp: 82 940
• llvm: 46 737
• misc: 37 543
• modernize: 35 527
• mpi: 0
• objc: 0
• performance: 1 367
• portability: 2 690
• readability: 100 747
• zircon: 0

[SergioRAgostinho]

Sheer curiosity: what exactly is your company doing with PCL? I'm curious on the reasons for subjecting all your code base to this extensive battery of checks. 👀

[SunBlack]

Sheer curiosity: what exactly is your company doing with PCL?

We are not using this much modules of PCL. Most reason to use PCL for us: If we want to experiment with data we have here already a lot of algorithms, so we don't need to implement everything on our own.

I'm curious on the reasons for subjecting all your code base to this extensive battery of checks. 👀

We are working with a lot of students, so we have a lot of developer who only spend a few month/years on our project. So automatic checks are perfect to reduce time we maintainer have to spend on MRs to get the code ready to merge. In general: My main task in our project is to refactor code (I'm doing it since some years now), so I have less work later if the code kept clean automatically (Build- server with Google Test, warnings as errors, CppCheck, clang-tidy and in near future clang-format).

And clang-tidy is nice, because e.g. when to use std::move and when const reference is sometimes hard to know, because I don't know of every class of us if it is trivially copyable or not.

[taketwo]

performance: 1 367

Curious to see those :)

[SunBlack]

performance: 1 367

Curious to see those :)

With deduplication these are not this much:

• performance-implicit-conversion-in-loop: 1
• performance-inefficient-string-concatenation: 4
• performance-type-promotion-in-math-fn: 2
• performance-unnecessary-copy-initialization: 8
• performance-unnecessary-value-param: 359

[stale[bot]]

Marking this as stale due to 30 days of inactivity. It will be closed in 7 days if no further activity occurs.

[gnawme]

Easiest way:

• Add set(CMAKE_EXPORT_COMPILE_COMMANDS ON) to CMakeLists.txt
• Run cmake .
• This generates compile_commands.json
• Use run-clang-tidy, which will look for, and iterate through, compile_commands.json and apply the specified checks
• Policy at my employer is to run clang-tidy (with a subset of the modernize-* checks) when your branch is close to being ready to merge (running it in CI for every push is not useful)

[kunaltyagi]

Is there a way to generate compile_commands.json without requiring all dependencies to be installed?

[gnawme]

Is there a way to generate compile_commands.json without requiring all dependencies to be installed?

set(CMAKE_EXPORT_COMPILE_COMMANDS ON) just generates the compile database, I'm not sure that it's analyzing or requiring dependencies as well.

[SunBlack]

Is there a way to generate compile_commands.json without requiring all dependencies to be installed?

Yes, as it requires a successful run of CMake. Furthermore clang-tidy requires also all dependencies, to can check the code (not like CppCheck).

[gnawme]

Is there a way to generate compile_commands.json without requiring all dependencies to be installed?

Yes, as it requires a successful run of CMake. Furthermore clang-tidy requires also all dependencies, to can check the code (not like CppCheck).

Generating compile_commands.json only requires that cmake . complete, is that what you mean? It will often complain that certain dependencies are missing, but still generate the compile database.

[SunBlack]

Generating compile_commands.json only requires that cmake . complete, is that what you mean? It will often complain that certain dependencies are missing, but still generate the compile database.

Yes, but when a dependency is missing, the target which requires these are not part of the database, as we skip some CMake code than.

[gnawme]

Opened PR #4560, working from the base modules of the PCL dependency graph to keep the LOC changed to a manageable quantity. Future PRs will continue up the dependency graph.

Supersedes #4249 which was too large for sensible review.