Skip to content

GitLab

  • Menu
Projects Groups Snippets
  • Help
    • Help
    • Support
    • Community forum
    • Submit feedback
    • Contribute to GitLab
  • Sign in / Register
  • ParaView ParaView
  • Project information
    • Project information
    • Activity
    • Labels
    • Members
  • Repository
    • Repository
    • Files
    • Commits
    • Branches
    • Tags
    • Contributors
    • Graph
    • Compare
  • Issues 1,846
    • Issues 1,846
    • List
    • Boards
    • Service Desk
    • Milestones
  • Merge requests 87
    • Merge requests 87
  • CI/CD
    • CI/CD
    • Pipelines
    • Jobs
    • Schedules
  • Deployments
    • Deployments
    • Environments
    • Releases
  • Monitor
    • Monitor
    • Incidents
  • Analytics
    • Analytics
    • Value stream
    • CI/CD
    • Repository
  • Wiki
    • Wiki
  • Snippets
    • Snippets
  • Activity
  • Graph
  • Create a new issue
  • Jobs
  • Commits
  • Issue Boards
Collapse sidebar
  • ParaView
  • ParaViewParaView
  • Issues
  • #20596

Closed
Open
Created Mar 23, 2021 by Ben Boeckel@ben.boeckelOwner36 of 88 tasks completed36/88 tasks

[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
Edited Mar 24, 2021 by Cory Quammen
To upload designs, you'll need to enable LFS and have an admin enable hashed storage. More information
Assignee
Assign to
Time tracking