[clang-tidy] Hackathon coordination issue
clang-tidy
has a lot of warnings that we're excluding right now. Attached is a build log containing all clang-tidy
warnings for the current source tree. To coordinate, there are some tools I recommend using:
I use Vim, so I'll describe my setup. Users of other IDEs/editors can chime in how to set this up in other environments.
Step 1
Download the compile_output.log
file containing all warnings.
Step 2
Mock out the compiler to support filtering warnings. I do this via:
set makeprg=make
with this file in my source tree (and compile_output.log
beside it).
all:
grep warning: compile_output.log | sed -e 's,/builds/gitlab-kitware-sciviz-ci/build/../,,'
lints:
grep warning: compile_output.log | grep -v -e -W | sed -e 's,/builds/gitlab-kitware-sciviz-ci/build/../,,'
%:
grep warning: compile_output.log | grep -v -e -W | \
sed -e 's,/builds/gitlab-kitware-sciviz-ci/build/../,,' | \
sed -e 's,^\.\./,,' | \
sed -e 's,^\.\./,,' | \
grep -e "$@"
Step 3
Run :make name-of-lint
to act as if the build made the warnings seen in CI. Special targets are :make
which outputs all warnings and :make lints
to output all clang-tidy
lints. Note that any absolute paths that show up are generated files and a code generator may need patched to fix problems. In addition, additional context (such as fixits) may be available in the full log. Note that line numbers do not update, so lint fixes that change line counts are going to need some extra care when fixing multiple things within a single file.
For the main body of ParaView source code, please grab a single lint for each commit and remove it from the .clang-tidy
file. In addition to the main ParaView source code, there are a few third-party bits that should be tackled individually:
-
Plugins/AnalyzeNIfTIReaderWriter/NIfTIIO/ThirdParty
-
Plugins/CDIReader/Reader/ThirdParty
-
Plugins/Datamine/Readers/ThirdParty
-
Plugins/GenericIOReader/Readers/LANL
-
Plugins/GeodesicMeasurement/Filters/FmmMesh
-
Utilities/VisItBridge/Library
(VisIt code) -
Utilities/VisItBridge/databases
(VisIt code) -
Utilities/VisItBridge/databases/AvtAlgorithms
(ParaView code for adapting VisIt readers) -
ThirdParty
(these use the import mechanism; probably best left for later)
Here is a list of lints that are unmasked. Some are far larger than others, so grab what seems reasonable for a single MR at a time (please check the box to claim the lint to avoid duplicating work). Fixing one lint may make another appear in the changed code; we can fix up such logical conflicts in a followup. Local testing and building is highly recommended because the dashboards aren't going to get through all of these fixes in a day.
-
bugprone-branch-clone
(typically indicates bugs) -
bugprone-fold-init-type
(MSVC also warns about this) -
bugprone-infinite-loop
-
bugprone-integer-division
-
bugprone-macro-parentheses
-
bugprone-misplaced-widening-cast
-
bugprone-narrowing-conversions
-
bugprone-not-null-terminated-result
(usually requires more investigation) -
bugprone-parent-virtual-call
(might be intentional; comments usually guide this) -
bugprone-string-constructor
-
bugprone-suspicious-enum-usage
-
bugprone-suspicious-missing-comma
-
bugprone-suspicious-semicolon
-
bugprone-suspicious-string-compare
-
bugprone-throw-keyword-missing
-
bugprone-unhandled-self-assignment
(we should have tests for this case) -
bugprone-unused-raii
(almost always a bug) -
bugprone-unused-return-value
-
bugprone-use-after-move
(almost always a bug) -
misc-redundant-expression
-
misc-throw-by-value-catch-by-reference
(low-hanging fruit) -
misc-unconventional-assign-operator
(VTK makes a bunch of these; see vtk/vtk#18119) -
misc-unused-parameters
(usually a comment, but could be conditional code) -
misc-unused-using-decls
(could be conditional code) -
modernize-deprecated-headers
("foo.h"
→<cfoo>
) -
modernize-raw-string-literal
-
modernize-redundant-void-arg
(trivial fix) -
modernize-use-bool-literals
(noisy, but usually worthwhile) -
modernize-use-default-member-init
-
performance-for-range-copy
-
performance-inefficient-string-concatenation
-
performance-inefficient-vector-operation
-
performance-no-automatic-move
-
performance-noexcept-move-constructor
-
performance-unnecessary-copy-initialization
-
readability-container-size-empty
-
readability-delete-null-pointer
-
readability-else-after-return
-
readability-isolate-declaration
-
readability-qualified-auto
-
readability-redundant-access-specifiers
-
readability-redundant-control-flow
-
readability-redundant-declaration
-
readability-redundant-member-init
-
readability-redundant-preprocessor
-
readability-redundant-smartptr-get
-
readability-redundant-string-cstr
-
readability-redundant-string-init
-
readability-static-accessed-through-instance
-
readability-static-definition-in-anonymous-namespace
-
readability-string-compare
Analyzer-based warnings usually require more in-depth investigation, but can point out holes in logic. Fixes for these likely deserve a commit per fix to explain how things are better now.
-
clang-analyzer-core.CallAndMessage
-
clang-analyzer-core.DivideZero
-
clang-analyzer-core.NonNullParamChecker
-
clang-analyzer-core.NullDereference
-
clang-analyzer-core.StackAddressEscape
-
clang-analyzer-core.UndefinedBinaryOperatorResult
-
clang-analyzer-core.VLASize
-
clang-analyzer-core.uninitialized.ArraySubscript
-
clang-analyzer-core.uninitialized.Assign
-
clang-analyzer-core.uninitialized.Branch
-
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-deadcode.DeadStores
-
clang-analyzer-optin.cplusplus.UninitializedObject
-
clang-analyzer-optin.cplusplus.VirtualCall
-
clang-analyzer-optin.mpi.MPI-Checker
-
clang-analyzer-optin.portability.UnixAPI
-
clang-analyzer-security.FloatLoopCounter
-
clang-analyzer-security.insecureAPI.DeprecatedOrUnsafeBufferHandling
-
clang-analyzer-security.insecureAPI.strcpy
-
clang-analyzer-unix.Malloc
-
clang-analyzer-unix.MallocSizeof
-
clang-analyzer-unix.MismatchedDeallocator
-
clang-analyzer-valist.Unterminated