Merge topic 'ci-clang-tidy'

04eea4eb gitlab-ci: add a clang-tidy job
931bd313 clang-tidy: add configuration files
0cdc7300 gitlab-ci: use the new CI image
6f436e6f ci: install dev tools and matplotlib into the CI image
adec0a6c clang-tidy: fix `readability-misleading-indentation` lints
12fb7aa9 clang-tidy: fix `readability-avoid-const-params-in-decls` lints
2c16114b clang-tidy: fix `readability-const-return-type` lints
0fed5987 clang-tidy: fix `performance-faster-string-find` lints
...
Acked-by: Kitware Robot <kwrobot@kitware.com>
Merge-request: !4738

.clang-tidy

+---
+Checks: "-*,\
+boost-*,\
+bugprone-*,\
+-bugprone-argument-comment,\
+-bugprone-branch-clone,\
+-bugprone-fold-init-type,\
+-bugprone-incorrect-roundings,\
+-bugprone-infinite-loop,\
+-bugprone-integer-division,\
+-bugprone-macro-parentheses,\
+-bugprone-misplaced-widening-cast,\
+-bugprone-narrowing-conversions,\
+-bugprone-not-null-terminated-result,\
+-bugprone-parent-virtual-call,\
+-bugprone-reserved-identifier,\
+-bugprone-signed-char-misuse,\
+-bugprone-sizeof-expression,\
+-bugprone-string-constructor,\
+-bugprone-suspicious-enum-usage,\
+-bugprone-suspicious-include,\
+-bugprone-suspicious-missing-comma,\
+-bugprone-suspicious-semicolon,\
+-bugprone-suspicious-string-compare,\
+-bugprone-throw-keyword-missing,\
+-bugprone-unhandled-self-assignment,\
+-bugprone-unused-raii,\
+-bugprone-unused-return-value,\
+-bugprone-use-after-move,\
+clang-analyzer-*,\
+-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,\
+misc-*,\
+-misc-no-recursion,\
+-misc-non-private-member-variables-in-classes,\
+-misc-redundant-expression,\
+-misc-throw-by-value-catch-by-reference,\
+-misc-unconventional-assign-operator,\
+-misc-unused-parameters,\
+-misc-unused-using-decls,\
+modernize-*,\
+-modernize-avoid-c-arrays,\
+-modernize-deprecated-headers,\
+-modernize-loop-convert,\
+-modernize-pass-by-value,\
+-modernize-raw-string-literal,\
+-modernize-redundant-void-arg,\
+-modernize-replace-random-shuffle,\
+-modernize-return-braced-init-list,\
+-modernize-shrink-to-fit,\
+-modernize-use-auto,\
+-modernize-use-bool-literals,\
+-modernize-use-default-member-init,\
+-modernize-use-emplace,\
+-modernize-use-equals-delete,\
+-modernize-use-trailing-return-type,\
+-modernize-use-using,\
+mpi-*,\
+openmp-*,\
+performance-*,\
+-performance-for-range-copy,\
+-performance-inefficient-string-concatenation,\
+-performance-inefficient-vector-operation,\
+-performance-move-const-arg,\
+-performance-no-automatic-move,\
+-performance-noexcept-move-constructor,\
+-performance-trivially-destructible,\
+-performance-type-promotion-in-math-fn,\
+-performance-unnecessary-copy-initialization,\
+-performance-unnecessary-value-param,\
+portability-*,\
+readability-*,\
+-readability-braces-around-statements,\
+-readability-container-size-empty,\
+-readability-convert-member-functions-to-static,\
+-readability-delete-null-pointer,\
+-readability-else-after-return,\
+-readability-function-size,\
+-readability-implicit-bool-conversion,\
+-readability-inconsistent-declaration-parameter-name,\
+-readability-isolate-declaration,\
+-readability-magic-numbers,\
+-readability-make-member-function-const,\
+-readability-named-parameter,\
+-readability-non-const-parameter,\
+-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-simplify-boolean-expr,\
+-readability-static-accessed-through-instance,\
+-readability-static-definition-in-anonymous-namespace,\
+-readability-string-compare,\
+-readability-uppercase-literal-suffix,\
+-readability-use-anyofallof,\
+"
+#WarningsAsErrors: "*"
+...

.gitlab-ci.yml

@@ -151,6 +151,18 @@ test:fedora33-shared-mpi-python-core:
     # editions testing needs some work.
     allow_failure: true
 
+## Static analysis
+
+build:fedora33-tidy:
+    extends:
+        - .fedora33_tidy
+        - .cmake_build_linux
+        - .linux_qt_builder_tags
+        - .cmake_tidy_artifacts
+        - .run_manually
+    # clang-tidy is *really* slow because it isn't cached.
+    timeout: 3 hours
+
 # macOS
 
 build:macos-python-qt:

.gitlab/artifacts.yml

@@ -82,3 +82,12 @@
         paths:
             # Debugging logs
             - build/Testing/Temporary/Last*.log
+
+.cmake_tidy_artifacts:
+    artifacts:
+        expire_in: 1d
+        # Because build logs are in here, we should always upload artifacts.
+        when: always
+        paths:
+            # Logs of the build
+            - compile_output.log

.gitlab/ci/cdash-groups.json

@@ -15,6 +15,11 @@
             "site": "gitlab-ci",
             "buildname": "[fedora33_shared_mpi_python_qt]"
         },
+        {
+            "group": "master",
+            "site": "gitlab-ci",
+            "buildname": "[fedora33_shared_mpi_python_qt_tidy]"
+        },
         {
             "group": "master",
             "site": "gitlab-ci",

.gitlab/ci/configure_fedora33_shared_mpi_python_qt_tidy.cmake

+set(CMAKE_C_CLANG_TIDY "/usr/bin/clang-tidy" CACHE FILEPATH "")
+set(CMAKE_CXX_CLANG_TIDY "/usr/bin/clang-tidy" CACHE FILEPATH "")
+
+include("${CMAKE_CURRENT_LIST_DIR}/configure_fedora33_shared_mpi_python_qt.cmake")

.gitlab/ci/docker/fedora33/install_deps.sh

@@ -16,6 +16,10 @@ dnf install -y --setopt=install_weak_deps=False \
 dnf install -y --setopt=install_weak_deps=False \
    mesa-libOSMesa-devel mesa-libOSMesa
 
+# Development tools
+dnf install -y --setopt=install_weak_deps=False \
+    libasan libtsan libubsan clang-tools-extra
+
 # External dependencies
 dnf install -y --setopt=install_weak_deps=False \
     libXcursor-devel libharu-devel utf8cpp-devel pugixml-devel libtiff-devel \
@@ -28,7 +32,7 @@ dnf install -y --setopt=install_weak_deps=False \
 dnf install -y --setopt=install_weak_deps=False \
    python3-twisted python3-autobahn python3 python3-devel python3-numpy \
    python3-pandas python3-pandas-datareader python3-sphinx python3-pip \
-   python3-mpi4py-mpich python3-mpi4py-openmpi
+   python3-mpi4py-mpich python3-mpi4py-openmpi python3-matplotlib
 
 python3 -m pip install wslink
 

.gitlab/os-linux.yml

@@ -22,7 +22,7 @@
 
 .fedora33:
     extends: .linux
-    image: "kitware/paraview:ci-fedora33-20210211"
+    image: "kitware/paraview:ci-fedora33-20210221"
 
 .fedora31_icc:
     extends: .linux
@@ -90,6 +90,15 @@
         CMAKE_CONFIGURATION: fedora33_shared_mpi_python_qt
         CMAKE_BUILD_TYPE: Debug
 
+.fedora33_tidy:
+    extends:
+        - .fedora33
+        - .fedora_mpich_addon
+
+    variables:
+        CMAKE_CONFIGURATION: fedora33_shared_mpi_python_qt_tidy
+        CTEST_NO_WARNINGS_ALLOWED: 1
+
 .linux_builder_tags:
     tags:
         - build

Adaptors/CTH/CTHAdaptor.cxx

@@ -31,8 +31,8 @@
 namespace
 {
 vtkCTHSource gSource;
-vtkCPProcessor* coProcessor = 0;
-vtkCPDataDescription* coProcessorData = 0;
+vtkCPProcessor* coProcessor = nullptr;
+vtkCPDataDescription* coProcessorData = nullptr;
 }
 
 #if !defined(_WIN32)
@@ -90,7 +90,7 @@ void pvspy_fil(char* filename, int len, char* /*runid*/, int* /*error*/)
       char* tok = strtok(line, " ");
       if (tok && strstr(tok, "*paraview"))
       {
-        script = strtok(0, " ");
+        script = strtok(nullptr, " ");
         len = static_cast<int>(strlen(script) + 1);
         break;
       }
@@ -131,10 +131,10 @@ void pvspy_viz(int cycle, double ptime, double /*pdt*/, int, int)
 void pvspy_fin()
 {
   coProcessor->Delete();
-  coProcessor = 0;
+  coProcessor = nullptr;
 
   coProcessorData->Delete();
-  coProcessorData = 0;
+  coProcessorData = nullptr;
 }
 
 //------------------------------------------------------------------------------

Adaptors/CTH/vtkCTHDataArray.cxx

@@ -31,9 +31,9 @@ vtkCTHDataArray::vtkCTHDataArray()
   this->ExtentsSet = false;
   this->PointerTime = 0;
 
-  this->Data = 0;
-  this->CopiedData = 0;
-  this->Tuple = 0;
+  this->Data = nullptr;
+  this->CopiedData = nullptr;
+  this->Tuple = nullptr;
   this->TupleSize = 0;
 
   this->Fallback = vtkDoubleArray::New();
@@ -56,7 +56,7 @@ void vtkCTHDataArray::Initialize()
   if (this->Fallback)
   {
     this->Fallback->Delete();
-    this->Fallback = 0;
+    this->Fallback = nullptr;
   }
 
   if (this->Data)
@@ -70,13 +70,13 @@ void vtkCTHDataArray::Initialize()
     }
     delete[] this->Data;
   }
-  this->Data = 0;
+  this->Data = nullptr;
 
   if (this->CopiedData)
   {
     delete[] this->CopiedData;
   }
-  this->CopiedData = 0;
+  this->CopiedData = nullptr;
 
   if (this->Tuple)
   {
@@ -90,7 +90,7 @@ void vtkCTHDataArray::SetDimensions(int x, int y, int z)
   if (this->Fallback)
   {
     this->Fallback->Delete();
-    this->Fallback = 0;
+    this->Fallback = nullptr;
   }
 
   this->Dimensions[0] = x;
@@ -185,7 +185,7 @@ double* vtkCTHDataArray::GetTuple(vtkIdType i)
     if (this->Tuple)
     {
       delete[] this->Tuple;
-      this->Tuple = 0;
+      this->Tuple = nullptr;
     }
     // fall through to the next if(), i.e. the initial case
   }

Adaptors/CTH/vtkCTHSource.cxx

@@ -32,16 +32,14 @@
 #include "vtkUnsignedCharArray.h"
 
 //---------------------------------------------------------------------------
-vtkCTHSource::vtkCTHSource()
-{
-}
+vtkCTHSource::vtkCTHSource() = default;
 
 vtkCTHSource::~vtkCTHSource()
 {
   if (NeighborArray)
   {
     NeighborArray->Delete();
-    NeighborArray = 0;
+    NeighborArray = nullptr;
   }
   for (size_t i = 0; i < this->Blocks.size(); i++)
   {
@@ -132,7 +130,7 @@ void vtkCTHSource::SetMaterialFieldName(int field_id, char* field_name, char* co
 void vtkCTHSource::SetCellFieldPointer(int block_id, int field_id, int k, int j, double* istrip)
 {
   Block& b = this->Blocks[block_id];
-  if (b.CFieldData[field_id] != 0)
+  if (b.CFieldData[field_id] != nullptr)
   {
     /// TODO fix for multiple components
     b.CFieldData[field_id]->SetDataPointer(0, k, j, istrip);
@@ -144,7 +142,7 @@ void vtkCTHSource::SetMaterialFieldPointer(
   int block_id, int field_id, int mat, int k, int j, double* istrip)
 {
   Block& b = this->Blocks[block_id];
-  if (b.MFieldData[mat][field_id] != 0)
+  if (b.MFieldData[mat][field_id] != nullptr)
   {
     /// TODO fix for multiple components
     b.MFieldData[mat][field_id]->SetDataPointer(0, k, j, istrip);
@@ -164,7 +162,7 @@ void vtkCTHSource::InitializeBlock(int block_id, int Nx, int Ny, int Nz, double*
   b.allocated = allocated;
   b.active = active;
   b.level = level;
-  b.ug = 0;
+  b.ug = nullptr;
   b.Nx = Nx;
   b.Ny = Ny;
   b.Nz = Nz;
@@ -184,7 +182,7 @@ void vtkCTHSource::InitializeBlock(int block_id, int Nx, int Ny, int Nz, double*
     }
     else
     {
-      b.CFieldData[c] = 0;
+      b.CFieldData[c] = nullptr;
     }
   }
   b.actualMaterials = 0; // so far we've filled none
@@ -204,7 +202,7 @@ void vtkCTHSource::InitializeBlock(int block_id, int Nx, int Ny, int Nz, double*
       }
       else
       {
-        b.MFieldData[m][f] = 0;
+        b.MFieldData[m][f] = nullptr;
       }
     }
   }
@@ -282,7 +280,7 @@ void vtkCTHSource::UpdateRepresentation()
     }
     for (size_t c = 0; c < b.CFieldData.size(); c++)
     {
-      if (b.CFieldData[c] == 0)
+      if (b.CFieldData[c] == nullptr)
         continue;
       b.CFieldData[c]->Modified();
       /*
@@ -302,7 +300,7 @@ void vtkCTHSource::UpdateRepresentation()
     {
       for (size_t f = 0; f < b.MFieldData[m].size(); f++)
       {
-        if (b.MFieldData[m][f] == 0)
+        if (b.MFieldData[m][f] == nullptr)
           continue;
         if (strncmp(b.MFieldData[m][f]->GetName(), "Volume Fraction", 15) != 0)
         {
@@ -364,7 +362,7 @@ int vtkCTHSource::FillInputData(vtkCPInputDataDescription* input)
       if (b.ug)
       {
         b.ug->Delete();
-        b.ug = 0;
+        b.ug = nullptr;
         delete b.box;
       }
       if (b.allocated)
@@ -590,7 +588,7 @@ void vtkCTHSource::AddFieldArrays(Block& b)
 
   for (size_t c = 0; c < b.CFieldData.size(); c++)
   {
-    if (b.CFieldData[c] == 0)
+    if (b.CFieldData[c] == nullptr)
       continue;
     b.CFieldData[c]->UnsetExtents();
     cd->AddArray(b.CFieldData[c]);
@@ -608,7 +606,7 @@ void vtkCTHSource::AddFieldArrays(Block& b)
     // TODO In general for in situ, we should only pull the data we'd actually want.
     for (size_t f = 0; f < b.MFieldData[m].size(); f++)
     {
-      if (b.MFieldData[m][f] == 0)
+      if (b.MFieldData[m][f] == nullptr)
         continue;
       b.MFieldData[m][f]->UnsetExtents();
       if (strncmp(b.MFieldData[m][f]->GetName(), "Volume Fraction", 15) != 0)
@@ -643,7 +641,7 @@ void vtkCTHSource::AddFieldArrays(Block& b, int loCorner[3], int hiCorner[3])
 
   for (size_t c = 0; c < b.CFieldData.size(); c++)
   {
-    if (b.CFieldData[c] == 0)
+    if (b.CFieldData[c] == nullptr)
       continue;
     b.CFieldData[c]->SetExtents(loCorner, hiCorner);
     cd->AddArray(b.CFieldData[c]);
@@ -661,7 +659,7 @@ void vtkCTHSource::AddFieldArrays(Block& b, int loCorner[3], int hiCorner[3])
     // TODO In general for in situ, we should only pull the data we'd actually want.
     for (size_t f = 0; f < b.MFieldData[m].size(); f++)
     {
-      if (b.MFieldData[m][f] == 0)
+      if (b.MFieldData[m][f] == nullptr)
         continue;
       b.MFieldData[m][f]->SetExtents(loCorner, hiCorner);
       if (strncmp(b.MFieldData[m][f]->GetName(), "Volume Fraction", 15) != 0)

Adaptors/Cam/Grid.txx

@@ -233,7 +233,7 @@ Grid<gridType>::Grid()
   , NLon(0)
   , NLat(0)
   , NLev(0)
-  , Lev(NULL)
+  , Lev(nullptr)
   , Radius(0)
   , Ne(0)
   , Np(0)

Adaptors/Cam/fv_create_data.cxx

@@ -56,12 +56,12 @@ void fv_coprocessorfinalize()
   if (g_coprocessor)
   {
     g_coprocessor->Delete();
-    g_coprocessor = NULL;
+    g_coprocessor = nullptr;
   }
   if (g_coprocessorData)
   {
     g_coprocessorData->Delete();
-    g_coprocessorData = NULL;
+    g_coprocessorData = nullptr;
   }
 }
 } // anonymous namespace
@@ -116,7 +116,7 @@ void fv_create_grid_(int* dim, double* lonCoord, double* latCoord, double* levCo
   {
     vtkGenericWarningMacro(<< "No input data description");
     delete g_grid;
-    g_grid = NULL;
+    g_grid = nullptr;
   }
 
   g_sgrid = new CamAdaptor::Grid<CamAdaptor::SPHERE>();
@@ -136,7 +136,7 @@ void fv_create_grid_(int* dim, double* lonCoord, double* latCoord, double* levCo
   {
     vtkGenericWarningMacro(<< "No input data description");
     delete g_sgrid;
-    g_sgrid = NULL;
+    g_sgrid = nullptr;
   }
 }
 
@@ -211,7 +211,7 @@ int fv_needtocreategrid_()
   // assume that the grid is not changing so that we only build it
   // the first time, otherwise we clear out the field data
   vtkCPInputDataDescription* idd = g_coprocessorData->GetInputDescriptionByName("input");
-  return (idd == NULL || idd->GetGrid() == NULL);
+  return (idd == nullptr || idd->GetGrid() == nullptr);
 }
 
 //------------------------------------------------------------------------------

Adaptors/Cam/se_create_data.cxx

@@ -55,12 +55,12 @@ void se_coprocessorfinalize()
   if (g_coprocessor)
   {
     g_coprocessor->Delete();
-    g_coprocessor = NULL;
+    g_coprocessor = nullptr;
   }
   if (g_coprocessorData)
   {
     g_coprocessorData->Delete();
-    g_coprocessorData = NULL;
+    g_coprocessorData = nullptr;
   }
 }
 } // anonymous namespace
@@ -112,7 +112,7 @@ void se_create_grid_(int* ne, int* np, int* nlon, double* lonRad, int* nlat, dou
   {
     vtkGenericWarningMacro(<< "No input data description");
     delete g_grid;
-    g_grid = NULL;
+    g_grid = nullptr;
   }
 }
 
@@ -173,7 +173,7 @@ int se_needtocreategrid_()
   // assume that the grid is not changing so that we only build it
   // the first time, otherwise we clear out the field data
   vtkCPInputDataDescription* idd = g_coprocessorData->GetInputDescriptionByName("input");
-  return (idd == NULL || idd->GetGrid() == NULL);
+  return (idd == nullptr || idd->GetGrid() == nullptr);
 }
 
 //------------------------------------------------------------------------------

Adaptors/CatalystTestDriver/vtkCPBaseFieldBuilder.cxx

@@ -15,14 +15,10 @@
 #include "vtkCPBaseFieldBuilder.h"
 
 //----------------------------------------------------------------------------
-vtkCPBaseFieldBuilder::vtkCPBaseFieldBuilder()
-{
-}
+vtkCPBaseFieldBuilder::vtkCPBaseFieldBuilder() = default;
 
 //----------------------------------------------------------------------------
-vtkCPBaseFieldBuilder::~vtkCPBaseFieldBuilder()
-{
-}
+vtkCPBaseFieldBuilder::~vtkCPBaseFieldBuilder() = default;
 
 //----------------------------------------------------------------------------
 void vtkCPBaseFieldBuilder::PrintSelf(ostream& os, vtkIndent indent)

Adaptors/CatalystTestDriver/vtkCPBaseGridBuilder.cxx

@@ -15,14 +15,10 @@
 #include "vtkCPBaseGridBuilder.h"
 
 //----------------------------------------------------------------------------
-vtkCPBaseGridBuilder::vtkCPBaseGridBuilder()
-{
-}
+vtkCPBaseGridBuilder::vtkCPBaseGridBuilder() = default;
 
 //----------------------------------------------------------------------------
-vtkCPBaseGridBuilder::~vtkCPBaseGridBuilder()
-{
-}
+vtkCPBaseGridBuilder::~vtkCPBaseGridBuilder() = default;
 
 //----------------------------------------------------------------------------
 void vtkCPBaseGridBuilder::PrintSelf(ostream& os, vtkIndent indent)

Adaptors/CatalystTestDriver/vtkCPCellFieldBuilder.cxx

@@ -26,20 +26,18 @@
 vtkStandardNewMacro(vtkCPCellFieldBuilder);
 
 //----------------------------------------------------------------------------
-vtkCPCellFieldBuilder::vtkCPCellFieldBuilder()
-{
-}
+vtkCPCellFieldBuilder::vtkCPCellFieldBuilder() = default;
 
 //----------------------------------------------------------------------------
 void vtkCPCellFieldBuilder::BuildField(unsigned long timeStep, double time, vtkDataSet* grid)
 {
   vtkCPTensorFieldFunction* tensorFieldFunction = this->GetTensorFieldFunction();
-  if (tensorFieldFunction == 0)
+  if (tensorFieldFunction == nullptr)
   {
     vtkErrorMacro("Must set TensorFieldFunction.");
     return;
   }
-  if (this->GetArrayName() == 0)
+  if (this->GetArrayName() == nullptr)
   {
     vtkErrorMacro("Must set ArrayName.");
     return;
@@ -68,9 +66,7 @@ void vtkCPCellFieldBuilder::BuildField(unsigned long timeStep, double time, vtkD
 }
 
 //----------------------------------------------------------------------------
-vtkCPCellFieldBuilder::~vtkCPCellFieldBuilder()
-{
-}
+vtkCPCellFieldBuilder::~vtkCPCellFieldBuilder() = default;
 
 //----------------------------------------------------------------------------
 void vtkCPCellFieldBuilder::PrintSelf(ostream& os, vtkIndent indent)

Adaptors/CatalystTestDriver/vtkCPConstantScalarFieldFunction.cxx

@@ -25,9 +25,7 @@ vtkCPConstantScalarFieldFunction::vtkCPConstantScalarFieldFunction()
 }
 
 //----------------------------------------------------------------------------
-vtkCPConstantScalarFieldFunction::~vtkCPConstantScalarFieldFunction()