Review casts in PCL, Part B (PointCloudLibrary#5508)

* Initial application of google-readability-casting

* Addressed google-readability-casting escapes

* Addressed clang-format violations flagged by CI

* More clang-format escapes

* Addressed clang-tidy escapes

* Fixed compile issue not seen on desktop

* Addressed review comments

* Fixed clang-tidy complaint

* Clarified PCLPointCloud2::concatenate logic

* Addressed latest review comments

* Addressed latest review comments

* Eliminated double casts per review

Author: gnawme

.clang-tidy

@@ -1,5 +1,5 @@
 ---
-Checks: '-*,modernize-use-auto,modernize-deprecated-headers,modernize-redundant-void-arg,modernize-replace-random-shuffle,modernize-use-equals-default,modernize-use-equals-delete,modernize-use-nullptr,modernize-use-override,modernize-use-using,performance-faster-string-find,performance-for-range-copy,performance-implicit-conversion-in-loop,performance-inefficient-algorithm,performance-inefficient-vector-operation,performance-move-const-arg,performance-move-constructor-init,performance-no-automatic-move,performance-noexcept-move-constructor,performance-type-promotion-in-math-fn,cppcoreguidelines-pro-type-cstyle-cast,cppcoreguidelines-pro-type-static-cast-downcast'
-WarningsAsErrors: '-*,modernize-use-auto,modernize-deprecated-headers,modernize-redundant-void-arg,modernize-replace-random-shuffle,modernize-use-equals-default,modernize-use-equals-delete,modernize-use-nullptr,modernize-use-override,modernize-use-using,performance-faster-string-find,performance-for-range-copy,performance-implicit-conversion-in-loop,performance-inefficient-algorithm,performance-inefficient-vector-operation,performance-move-const-arg,performance-move-constructor-init,performance-no-automatic-move,performance-noexcept-move-constructor,performance-type-promotion-in-math-fn,cppcoreguidelines-pro-type-cstyle-cast,cppcoreguidelines-pro-type-static-cast-downcast'
+Checks: '-*,modernize-use-auto,modernize-deprecated-headers,modernize-redundant-void-arg,modernize-replace-random-shuffle,modernize-use-equals-default,modernize-use-equals-delete,modernize-use-nullptr,modernize-use-override,modernize-use-using,performance-faster-string-find,performance-for-range-copy,performance-implicit-conversion-in-loop,performance-inefficient-algorithm,performance-inefficient-vector-operation,performance-move-const-arg,performance-move-constructor-init,performance-no-automatic-move,performance-noexcept-move-constructor,performance-type-promotion-in-math-fn,cppcoreguidelines-pro-type-cstyle-cast,cppcoreguidelines-pro-type-static-cast-downcast,google-readability-casting'
+WarningsAsErrors: '-*,modernize-use-auto,modernize-deprecated-headers,modernize-redundant-void-arg,modernize-replace-random-shuffle,modernize-use-equals-default,modernize-use-equals-delete,modernize-use-nullptr,modernize-use-override,modernize-use-using,performance-faster-string-find,performance-for-range-copy,performance-implicit-conversion-in-loop,performance-inefficient-algorithm,performance-inefficient-vector-operation,performance-move-const-arg,performance-move-constructor-init,performance-no-automatic-move,performance-noexcept-move-constructor,performance-type-promotion-in-math-fn,cppcoreguidelines-pro-type-cstyle-cast,cppcoreguidelines-pro-type-static-cast-downcast,google-readability-casting'
 CheckOptions:
 - {key: modernize-use-auto.MinTypeNameLength, value: 7}

2d/include/pcl/2d/impl/edge.hpp

@@ -277,7 +277,7 @@ Edge<PointInT, PointOutT>::suppressNonMaxima(
 
       // maxima (j, i).intensity = 0;
 
-      switch (int(ptedge.direction)) {
+      switch (static_cast<int>(ptedge.direction)) {
       case 0: {
         if (ptedge.magnitude >= edges(j - 1, i).magnitude &&
             ptedge.magnitude >= edges(j + 1, i).magnitude)

2d/include/pcl/2d/impl/kernel.hpp

@@ -106,9 +106,9 @@ kernel<PointT>::gaussianKernel(pcl::PointCloud<PointT>& kernel)
     for (int j = 0; j < kernel_size_; j++) {
       int iks = (i - kernel_size_ / 2);
       int jks = (j - kernel_size_ / 2);
-      kernel(j, i).intensity =
-          std::exp(float(-double(iks * iks + jks * jks) / sigma_sqr));
-      sum += float(kernel(j, i).intensity);
+      kernel(j, i).intensity = std::exp(
+          static_cast<float>(-static_cast<double>(iks * iks + jks * jks) / sigma_sqr));
+      sum += (kernel(j, i).intensity);
     }
   }
 
@@ -132,7 +132,8 @@ kernel<PointT>::loGKernel(pcl::PointCloud<PointT>& kernel)
     for (int j = 0; j < kernel_size_; j++) {
       int iks = (i - kernel_size_ / 2);
       int jks = (j - kernel_size_ / 2);
-      float temp = float(double(iks * iks + jks * jks) / sigma_sqr);
+      float temp =
+          static_cast<float>(static_cast<double>(iks * iks + jks * jks) / sigma_sqr);
       kernel(j, i).intensity = (1.0f - temp) * std::exp(-temp);
       sum += kernel(j, i).intensity;
     }

common/include/pcl/common/impl/bivariate_polynomial.hpp

@@ -208,19 +208,19 @@ BivariatePolynomialT<real>::findCriticalPoints (std::vector<real>& x_values, std
 
   if (degree == 2)
   {
-    real x = (real(2)*parameters[2]*parameters[3] - parameters[1]*parameters[4]) /
-             (parameters[1]*parameters[1] - real(4)*parameters[0]*parameters[3]),
-         y = (real(-2)*parameters[0]*x - parameters[2]) / parameters[1];
+    real x = (static_cast<real>(2)*parameters[2]*parameters[3] - parameters[1]*parameters[4]) /
+             (parameters[1]*parameters[1] - static_cast<real>(4)*parameters[0]*parameters[3]),
+         y = (static_cast<real>(-2)*parameters[0]*x - parameters[2]) / parameters[1];
 
     if (!std::isfinite(x) || !std::isfinite(y))
       return;
 
     int type = 2;
-    real det_H = real(4)*parameters[0]*parameters[3] - parameters[1]*parameters[1];
+    real det_H = static_cast<real>(4)*parameters[0]*parameters[3] - parameters[1]*parameters[1];
     //std::cout << "det(H) = "<<det_H<<"\n";
-    if (det_H > real(0))  // Check Hessian determinant
+    if (det_H > static_cast<real>(0))  // Check Hessian determinant
     {
-      if (parameters[0]+parameters[3] < real(0))  // Check Hessian trace
+      if (parameters[0]+parameters[3] < static_cast<real>(0))  // Check Hessian trace
         type = 0;
       else
         type = 1;

common/include/pcl/common/impl/common.hpp

@@ -168,7 +168,7 @@ pcl::getPointsInBox (const pcl::PointCloud<PointT> &cloud,
         continue;
       if (cloud[i].x > max_pt[0] || cloud[i].y > max_pt[1] || cloud[i].z > max_pt[2])
         continue;
-      indices[l++] = int (i);
+      indices[l++] = static_cast<int>(i);
     }
   }
   // NaN or Inf values could exist => check for them
@@ -186,7 +186,7 @@ pcl::getPointsInBox (const pcl::PointCloud<PointT> &cloud,
         continue;
       if (cloud[i].x > max_pt[0] || cloud[i].y > max_pt[1] || cloud[i].z > max_pt[2])
         continue;
-      indices[l++] = int (i);
+      indices[l++] = static_cast<int>(i);
     }
   }
   indices.resize (l);
@@ -210,7 +210,7 @@ pcl::getMaxDistance (const pcl::PointCloud<PointT> &cloud, const Eigen::Vector4f
       dist = (pivot_pt3 - pt).norm ();
       if (dist > max_dist)
       {
-        max_idx = int (i);
+        max_idx = static_cast<int>(i);
         max_dist = dist;
       }
     }
@@ -227,7 +227,7 @@ pcl::getMaxDistance (const pcl::PointCloud<PointT> &cloud, const Eigen::Vector4f
       dist = (pivot_pt3 - pt).norm ();
       if (dist > max_dist)
       {
-        max_idx = int (i);
+        max_idx = static_cast<int>(i);
         max_dist = dist;
       }
     }

common/include/pcl/common/impl/pca.hpp

@@ -71,7 +71,7 @@ PCA<PointT>::initCompute ()
   demeanPointCloud (*input_, *indices_, mean_, cloud_demean);
   assert (cloud_demean.cols () == int (indices_->size ()));
   // Compute the product cloud_demean * cloud_demean^T
-  const Eigen::Matrix3f alpha = (1.f / (float (indices_->size ()) - 1.f))
+  const Eigen::Matrix3f alpha = (1.f / (static_cast<float>(indices_->size ()) - 1.f))
                                   * cloud_demean.topRows<3> () * cloud_demean.topRows<3> ().transpose ();
 
   // Compute eigen vectors and values
@@ -102,7 +102,7 @@ PCA<PointT>::update (const PointT& input_point, FLAG flag)
 
   Eigen::Vector3f input (input_point.x, input_point.y, input_point.z);
   const std::size_t n = eigenvectors_.cols ();// number of eigen vectors
-  Eigen::VectorXf meanp = (float(n) * (mean_.head<3>() + input)) / float(n + 1);
+  Eigen::VectorXf meanp = (static_cast<float>(n) * (mean_.head<3>() + input)) / static_cast<float>(n + 1);
   Eigen::VectorXf a = eigenvectors_.transpose() * (input - mean_.head<3>());
   Eigen::VectorXf y = (eigenvectors_ * a) + mean_.head<3>();
   Eigen::VectorXf h = y - input;
@@ -113,12 +113,12 @@ PCA<PointT>::update (const PointT& input_point, FLAG flag)
   float gamma = h.dot(input - mean_.head<3>());
   Eigen::MatrixXf D = Eigen::MatrixXf::Zero (a.size() + 1, a.size() + 1);
   D.block(0,0,n,n) = a * a.transpose();
-  D /=  float(n)/float((n+1) * (n+1));
+  D /=  static_cast<float>(n)/static_cast<float>((n+1) * (n+1));
   for(std::size_t i=0; i < a.size(); i++) {
-    D(i,i)+= float(n)/float(n+1)*eigenvalues_(i);
-    D(D.rows()-1,i) = float(n) / float((n+1) * (n+1)) * gamma * a(i);
+    D(i,i)+= static_cast<float>(n)/static_cast<float>(n+1)*eigenvalues_(i);
+    D(D.rows()-1,i) = static_cast<float>(n) / static_cast<float>((n+1) * (n+1)) * gamma * a(i);
     D(i,D.cols()-1) = D(D.rows()-1,i);
-    D(D.rows()-1,D.cols()-1) = float(n)/float((n+1) * (n+1)) * gamma * gamma;
+    D(D.rows()-1,D.cols()-1) = static_cast<float>(n)/static_cast<float>((n+1) * (n+1)) * gamma * gamma;
   }
 
   Eigen::MatrixXf R(D.rows(), D.cols());

common/include/pcl/impl/cloud_iterator.hpp

@@ -218,12 +218,12 @@ namespace pcl
 
       unsigned getCurrentPointIndex () const override
       {
-        return (unsigned (iterator_ - cloud_.begin ()));
+        return (static_cast<unsigned>(iterator_ - cloud_.begin ()));
       }
 
       unsigned getCurrentIndex () const override
       {
-        return (unsigned (iterator_ - cloud_.begin ()));
+        return (static_cast<unsigned>(iterator_ - cloud_.begin ()));
       }
 
       std::size_t size () const override
@@ -292,12 +292,12 @@ namespace pcl
 
       unsigned getCurrentPointIndex () const override
       {
-        return (unsigned (*iterator_));
+        return (static_cast<unsigned>(*iterator_));
       }
 
       unsigned getCurrentIndex () const override
       {
-        return (unsigned (iterator_ - indices_.begin ()));
+        return (static_cast<unsigned>(iterator_ - indices_.begin ()));
       }
 
       std::size_t size () const override

common/include/pcl/range_image/impl/range_image.hpp

@@ -661,7 +661,7 @@ RangeImage::getAcutenessValue (const PointWithRange& point1, const PointWithRang
   float impact_angle = getImpactAngle (point1, point2);
   if (std::isinf (impact_angle))
     return -std::numeric_limits<float>::infinity ();
-  float ret = 1.0f - float (std::fabs (impact_angle)/ (0.5f*M_PI));
+  float ret = 1.0f - static_cast<float>(std::fabs (impact_angle)/ (0.5f*M_PI));
   if (impact_angle < 0.0f)
     ret = -ret;
   //if (std::abs (ret)>1)

common/src/PCLPointCloud2.cpp

@@ -58,17 +58,13 @@ pcl::PCLPointCloud2::concatenate (pcl::PCLPointCloud2 &cloud1, const pcl::PCLPoi
   const auto size1 = cloud1.width * cloud1.height;
   const auto size2 = cloud2.width * cloud2.height;
   //if one input cloud has no points, but the other input does, just select the cloud with points
-  switch ((bool (size1) << 1) + bool (size2))
-  {
-    case 1:
-      cloud1 = cloud2;
-      PCL_FALLTHROUGH
-    case 0:
-    case 2:
-      cloud1.header.stamp = std::max (cloud1.header.stamp, cloud2.header.stamp);
-      return (true);
-    default:
-      break;
+  if ((size1 == 0) && (size2 != 0)) {
+    cloud1 = cloud2;
+  }
+
+  if ((size1 == 0) || (size2 == 0)) {
+    cloud1.header.stamp = std::max (cloud1.header.stamp, cloud2.header.stamp);
+    return true;
   }
 
   // Ideally this should be in PCLPointField class since this is global behavior

common/src/bearing_angle_image.cpp

@@ -114,7 +114,7 @@ BearingAngleImage::generateBAImage (PointCloud<PointXYZ>& point_cloud)
       points[(i + 1) * width + j].y = point_cloud.at (j, i + 1).y;
       points[(i + 1) * width + j].z = point_cloud.at (j, i + 1).z;
       // set the gray value for every pixel point
-      points[(i + 1) * width + j].rgba = ((int)r) << 24 | ((int)g) << 16 | ((int)b) << 8 | 0xff;
+      points[(i + 1) * width + j].rgba = (static_cast<int>(r)) << 24 | (static_cast<int>(g)) << 16 | (static_cast<int>(b)) << 8 | 0xff;
     }
   }
 }