Added modernize-use-emplace, modernize-loop-convert clang-tidy check (#5610)

Fixed some odd misspellings


Fixed formatting escapes


Ran 'build format' to resolve clang-format issues


Debugging odd compile error


Fixed more issues from CI


Addressed CI issues and review feedback


Added missing const

Added yet more missing const


Reverted another omp parallel for


Reverted for third-party code


Disabled clang-tidy for third-party code

Author: gnawme

.clang-tidy

@@ -5,10 +5,13 @@ Checks: >
     cppcoreguidelines-pro-type-static-cast-downcast,
     google-readability-casting,
     modernize-deprecated-headers,
+    modernize-loop-convert,
     modernize-redundant-void-arg,
     modernize-replace-random-shuffle,
     modernize-return-braced-init-list,
+    modernize-shrink-to-fit,
     modernize-use-auto,
+    modernize-use-emplace,
     modernize-use-equals-default,
     modernize-use-equals-delete,
     modernize-use-nullptr,

common/include/pcl/point_types_conversion.h

@@ -173,12 +173,12 @@ namespace pcl
     f[2] /= 1.08883;
 
     // CIEXYZ -> CIELAB
-    for (int i = 0; i < 3; ++i) {
-      if (f[i] > 0.008856) {
-        f[i] = std::pow(f[i], 1.0 / 3.0);
+    for (float & xyz : f) {
+      if (xyz > 0.008856) {
+        xyz = std::pow(xyz, 1.0 / 3.0);
       }
       else {
-        f[i] = 7.787 * f[i] + 16.0 / 116.0;
+        xyz = 7.787 * xyz + 16.0 / 116.0;
       }
     }
 

features/include/pcl/features/impl/multiscale_feature_persistence.hpp

@@ -91,10 +91,10 @@ pcl::MultiscaleFeaturePersistence<PointSource, PointFeature>::computeFeaturesAtA
   features_at_scale_.reserve (scale_values_.size ());
   features_at_scale_vectorized_.clear ();
   features_at_scale_vectorized_.reserve (scale_values_.size ());
-  for (std::size_t scale_i = 0; scale_i < scale_values_.size (); ++scale_i)
+  for (float & scale_value : scale_values_)
   {
     FeatureCloudPtr feature_cloud (new FeatureCloud ());
-    computeFeatureAtScale (scale_values_[scale_i], feature_cloud);
+    computeFeatureAtScale (scale_value, feature_cloud);
     features_at_scale_.push_back(feature_cloud);
 
     // Vectorize each feature and insert it into the vectorized feature storage

features/include/pcl/features/our_cvfh.h

@@ -190,8 +190,8 @@ namespace pcl
       inline void
       getCentroidClusters (std::vector<Eigen::Vector3f, Eigen::aligned_allocator<Eigen::Vector3f> > & centroids)
       {
-        for (std::size_t i = 0; i < centroids_dominant_orientations_.size (); ++i)
-          centroids.push_back (centroids_dominant_orientations_[i]);
+        for (const auto & centroids_dominant_orientation : centroids_dominant_orientations_)
+          centroids.push_back (centroids_dominant_orientation);
       }
 
       /** \brief Get the normal centroids used to compute different CVFH descriptors
@@ -200,8 +200,8 @@ namespace pcl
       inline void
       getCentroidNormalClusters (std::vector<Eigen::Vector3f, Eigen::aligned_allocator<Eigen::Vector3f> > & centroids)
       {
-        for (std::size_t i = 0; i < dominant_normals_.size (); ++i)
-          centroids.push_back (dominant_normals_[i]);
+        for (const auto & dominant_normal : dominant_normals_)
+          centroids.push_back (dominant_normal);
       }
 
       /** \brief Sets max. Euclidean distance between points to be added to the cluster 

features/src/narf.cpp

@@ -376,6 +376,8 @@ Narf::extractForInterestPoints (const RangeImage& range_image, const PointCloud<
   schedule(dynamic, 10) \
   num_threads(max_no_of_threads)
   //!!! nizar 20110408 : for OpenMP sake on MSVC this must be kept signed
+  // Disable lint since this 'for' is part of the pragma
+  // NOLINTNEXTLINE(modernize-loop-convert)
   for (std::ptrdiff_t idx = 0; idx < static_cast<std::ptrdiff_t>(interest_points.size ()); ++idx)
   {
     const auto& interest_point = interest_points[idx];
@@ -389,7 +391,7 @@ Narf::extractForInterestPoints (const RangeImage& range_image, const PointCloud<
     else {
       if (!rotation_invariant)
       {
-#       pragma omp critical
+#pragma omp critical
         {
           feature_list.push_back(feature);
         }
@@ -409,7 +411,7 @@ Narf::extractForInterestPoints (const RangeImage& range_image, const PointCloud<
               delete feature2;
               continue;
             }
-#           pragma omp critical
+#pragma omp critical
             {
               feature_list.push_back(feature2);
             }

filters/include/pcl/filters/impl/covariance_sampling.hpp

@@ -75,8 +75,8 @@ pcl::CovarianceSampling<PointT, PointNT>::initCompute ()
   }
 
   average_norm /= static_cast<double>(scaled_points_.size ());
-  for (std::size_t p_i = 0; p_i < scaled_points_.size (); ++p_i)
-    scaled_points_[p_i] /= static_cast<float>(average_norm);
+  for (auto & scaled_point : scaled_points_)
+    scaled_point /= static_cast<float>(average_norm);
 
   return (true);
 }

filters/include/pcl/filters/impl/crop_hull.hpp

@@ -153,10 +153,10 @@ pcl::CropHull<PointT>::applyFilter3D (Indices &indices)
       Eigen::Vector3f(0.856514f,  0.508771f, 0.0868081f)
     };
 
-    for (std::size_t poly = 0; poly < hull_polygons_.size (); poly++)
+    for (const auto & hull_polygon : hull_polygons_)
       for (std::size_t ray = 0; ray < 3; ray++)
         crossings[ray] += rayTriangleIntersect
-          ((*input_)[(*indices_)[index]], rays[ray], hull_polygons_[poly], *hull_cloud_);
+          ((*input_)[(*indices_)[index]], rays[ray], hull_polygon, *hull_cloud_);
 
     bool crosses = (crossings[0]&1) + (crossings[1]&1) + (crossings[2]&1) > 1;
     if ((crop_outside_ && crosses) || (!crop_outside_ && !crosses))

filters/include/pcl/filters/impl/local_maximum.hpp

@@ -154,9 +154,9 @@ pcl::LocalMaximum<PointT>::applyFilterIndices (Indices &indices)
 
     // Check to see if a neighbor is higher than the query point
     float query_z = (*input_)[iii].z;
-    for (std::size_t k = 0; k < radius_indices.size (); ++k) // the query point itself is in the (unsorted) radius_indices, but that is okay since we compare with ">"
+    for (const auto& radius_index : radius_indices) // the query point itself is in the (unsorted) radius_indices, but that is okay since we compare with ">"
     {
-      if ((*input_)[radius_indices[k]].z > query_z)
+      if ((*input_)[radius_index].z > query_z)
       {
         // Query point is not the local max, no need to check others
         point_is_max[iii] = false;
@@ -168,9 +168,9 @@ pcl::LocalMaximum<PointT>::applyFilterIndices (Indices &indices)
     // visited, excluding them from future consideration as local maxima
     if (point_is_max[iii])
     {
-      for (std::size_t k = 0; k < radius_indices.size (); ++k) // the query point itself is in the (unsorted) radius_indices, but it must also be marked as visited
+      for (const auto& radius_index : radius_indices) // the query point itself is in the (unsorted) radius_indices, but it must also be marked as visited
       {
-        point_is_visited[radius_indices[k]] = true;
+        point_is_visited[radius_index] = true;
       }
     }
 

geometry/include/pcl/geometry/mesh_base.h

@@ -323,25 +323,28 @@ class MeshBase {
     }
 
     // Adjust the indices
-    for (auto it = vertices_.begin(); it != vertices_.end(); ++it) {
-      if (it->idx_outgoing_half_edge_.isValid()) {
-        it->idx_outgoing_half_edge_ =
-            new_half_edge_indices[it->idx_outgoing_half_edge_.get()];
+    for (auto& vertex : vertices_) {
+      if (vertex.idx_outgoing_half_edge_.isValid()) {
+        vertex.idx_outgoing_half_edge_ =
+            new_half_edge_indices[vertex.idx_outgoing_half_edge_.get()];
       }
     }
 
-    for (auto it = half_edges_.begin(); it != half_edges_.end(); ++it) {
-      it->idx_terminating_vertex_ =
-          new_vertex_indices[it->idx_terminating_vertex_.get()];
-      it->idx_next_half_edge_ = new_half_edge_indices[it->idx_next_half_edge_.get()];
-      it->idx_prev_half_edge_ = new_half_edge_indices[it->idx_prev_half_edge_.get()];
-      if (it->idx_face_.isValid()) {
-        it->idx_face_ = new_face_indices[it->idx_face_.get()];
+    for (auto& half_edge : half_edges_) {
+      half_edge.idx_terminating_vertex_ =
+          new_vertex_indices[half_edge.idx_terminating_vertex_.get()];
+      half_edge.idx_next_half_edge_ =
+          new_half_edge_indices[half_edge.idx_next_half_edge_.get()];
+      half_edge.idx_prev_half_edge_ =
+          new_half_edge_indices[half_edge.idx_prev_half_edge_.get()];
+      if (half_edge.idx_face_.isValid()) {
+        half_edge.idx_face_ = new_face_indices[half_edge.idx_face_.get()];
       }
     }
 
-    for (auto it = faces_.begin(); it != faces_.end(); ++it) {
-      it->idx_inner_half_edge_ = new_half_edge_indices[it->idx_inner_half_edge_.get()];
+    for (auto& face : faces_) {
+      face.idx_inner_half_edge_ =
+          new_half_edge_indices[face.idx_inner_half_edge_.get()];
     }
   }
 

keypoints/include/pcl/keypoints/impl/smoothed_surfaces_keypoint.hpp

@@ -52,7 +52,7 @@ pcl::SmoothedSurfacesKeypoint<PointT, PointNT>::addSmoothedPointCloud (const Poi
   clouds_.push_back (cloud);
   cloud_normals_.push_back (normals);
   cloud_trees_.push_back (kdtree);
-  scales_.push_back (std::pair<float, std::size_t> (scale, scales_.size ()));
+  scales_.emplace_back(scale, scales_.size ());
 }
 
 
@@ -115,15 +115,15 @@ pcl::SmoothedSurfacesKeypoint<PointT, PointNT>::detectKeypoints (PointCloudT &ou
     if (is_min || is_max)
     {
       bool passed_min = true, passed_max = true;
-      for (std::size_t scale_i = 0; scale_i < scales_.size (); ++scale_i)
+      for (const auto & scale : scales_)
       {
-        std::size_t cloud_i = scales_[scale_i].second;
+        std::size_t cloud_i = scale.second;
         // skip input cloud
         if (cloud_i == clouds_.size () - 1)
           continue;
 
         nn_indices.clear (); nn_distances.clear ();
-        cloud_trees_[cloud_i]->radiusSearch (point_i, scales_[scale_i].first * neighborhood_constant_, nn_indices, nn_distances);
+        cloud_trees_[cloud_i]->radiusSearch (point_i, scale.first * neighborhood_constant_, nn_indices, nn_distances);
 
         bool is_min_other_scale = true, is_max_other_scale = true;
         for (const auto &nn_index : nn_indices)
@@ -225,7 +225,7 @@ pcl::SmoothedSurfacesKeypoint<PointT, PointNT>::initCompute ()
   }
 
   // Add the input cloud as the last index
-  scales_.push_back (std::pair<float, std::size_t> (input_scale_, scales_.size ()));
+  scales_.emplace_back(input_scale_, scales_.size ());
   clouds_.push_back (input_);
   cloud_normals_.push_back (normals_);
   cloud_trees_.push_back (tree_);
@@ -241,7 +241,7 @@ pcl::SmoothedSurfacesKeypoint<PointT, PointNT>::initCompute ()
     }
 
   PCL_INFO ("Scales: ");
-  for (std::size_t i = 0; i < scales_.size (); ++i) PCL_INFO ("(%d %f), ", scales_[i].second, scales_[i].first);
+  for (const auto & scale : scales_) PCL_INFO ("(%d %f), ", scale.second, scale.first);
   PCL_INFO ("\n");
 
   return (true);

keypoints/include/pcl/keypoints/impl/trajkovic_2d.hpp

@@ -246,6 +246,8 @@ TrajkovicKeypoint2D<PointInT, PointOutT, IntensityT>::detectKeypoints (PointClou
   shared(height, indices, occupency_map, output, width) \
   num_threads(threads_)
 #endif
+  // Disable lint since this 'for' is part of the pragma
+  // NOLINTNEXTLINE(modernize-loop-convert)
   for (std::ptrdiff_t i = 0; i < static_cast<std::ptrdiff_t> (indices.size ()); ++i)
   {
     int idx = indices[i];

ml/include/pcl/ml/impl/dt/decision_tree_trainer.hpp

@@ -159,21 +159,15 @@ DecisionTreeTrainer<FeatureType, DataSet, LabelType, ExampleIndex, NodeType>::
     if (!thresholds_.empty()) {
       // compute information gain for each threshold and store threshold with highest
       // information gain
-      for (std::size_t threshold_index = 0; threshold_index < thresholds_.size();
-           ++threshold_index) {
+      for (const float& threshold : thresholds_) {
 
-        const float information_gain =
-            stats_estimator_->computeInformationGain(data_set_,
-                                                     examples,
-                                                     label_data,
-                                                     feature_results,
-                                                     flags,
-                                                     thresholds_[threshold_index]);
+        const float information_gain = stats_estimator_->computeInformationGain(
+            data_set_, examples, label_data, feature_results, flags, threshold);
 
         if (information_gain > best_feature_information_gain) {
           best_feature_information_gain = information_gain;
           best_feature_index = static_cast<int>(feature_index);
-          best_feature_threshold = thresholds_[threshold_index];
+          best_feature_threshold = threshold;
         }
       }
     }

people/include/pcl/people/impl/head_based_subcluster.hpp

@@ -270,9 +270,9 @@ pcl::people::HeadBasedSubclustering<PointT>::subcluster (std::vector<pcl::people
   }
 
   // Person clusters creation from clusters indices:
-  for(std::vector<pcl::PointIndices>::const_iterator it = cluster_indices_.begin(); it != cluster_indices_.end(); ++it)
+  for(const auto & cluster_index : cluster_indices_)
   {
-    pcl::people::PersonCluster<PointT> cluster(cloud_, *it, ground_coeffs_, sqrt_ground_coeffs_, head_centroid_, vertical_);  // PersonCluster creation
+    pcl::people::PersonCluster<PointT> cluster(cloud_, cluster_index, ground_coeffs_, sqrt_ground_coeffs_, head_centroid_, vertical_);  // PersonCluster creation
     clusters.push_back(cluster);
   }
 

recognition/include/pcl/recognition/3rdparty/metslib/model.hh

@@ -36,6 +36,8 @@
 #define METS_MODEL_HH_
 
 namespace mets {
+// Exempt third-party code from clang-tidy
+// NOLINTBEGIN
 
   /// @brief Type of the objective/cost function.
   ///
@@ -656,7 +658,7 @@ namespace mets {
 
     /// @brief Dtor.
     ~swap_full_neighborhood() override { 
-      for(auto it = moves_m.begin(); 
+      for(auto it = moves_m.begin();
 	  it != moves_m.end(); ++it)
 	delete *it;
     }
@@ -681,7 +683,7 @@ namespace mets {
 
     /// @brief Dtor.
     ~invert_full_neighborhood() override { 
-      for(auto it = moves_m.begin(); 
+      for(auto it = moves_m.begin();
 	  it != moves_m.end(); ++it)
 	delete *it;
     }
@@ -711,7 +713,7 @@ namespace mets {
 		    const Tp r) const 
     { return l->operator==(*r); }
   };
-
+// NOLINTEND
 }
 
 //________________________________________________________________________

recognition/include/pcl/recognition/3rdparty/metslib/tabu-search.hh

@@ -497,7 +497,9 @@ mets::tabu_list_chain::is_tabu(feasible_solution& sol, /* const */ move& mov) co
 
 inline mets::simple_tabu_list::~simple_tabu_list()
 { 
-  for(move_map_type::iterator m = tabu_hash_m.begin(); 
+  // Disable lint for third-party code
+  // NOLINTNEXTLINE(modernize-loop-convert)
+  for(move_map_type::iterator m = tabu_hash_m.begin();
       m!=tabu_hash_m.end(); ++m)
     delete m->first;
 }

recognition/include/pcl/recognition/impl/hv/hv_go.hpp

@@ -410,9 +410,9 @@ void pcl::GlobalHypothesesVerification<ModelT, SceneT>::SAOptimize(std::vector<i
   }
 
   int occupied_multiple = 0;
-  for(std::size_t i=0; i < complete_cloud_occupancy_by_RM_.size(); i++) {
-    if(complete_cloud_occupancy_by_RM_[i] > 1) {
-      occupied_multiple+=complete_cloud_occupancy_by_RM_[i];
+  for(const auto& i : complete_cloud_occupancy_by_RM_) {
+    if(i > 1) {
+      occupied_multiple+=i;
     }
   }