Add bugprone-macro-parentheses clang-tidy check (#5967)

* Added bugprone-macro-parentheses check to .clang-tidy * Added exceptions to bugprone-macro-parentheses * Addressed CI issues, review comments * Addressed clang-tidy issue for untracked(?) file * Addressed pcl_config.h issue properly * Fixed another clang-tidy complaint * Rebased, reverted mistaken commit * Exempted uncooperative macro from check
PointCloudLibrary · Mar 5, 2024 · d39bfc3 · d39bfc3
1 parent 1bf96dc
commit d39bfc3
Show file tree

Hide file tree

Showing 21 changed files with 40 additions and 29 deletions.
diff --git a/.clang-tidy b/.clang-tidy
@@ -1,6 +1,8 @@
 ---
 Checks: >
     -*,
+    bugprone-copy-constructor-init,
+    bugprone-macro-parentheses,
     cppcoreguidelines-pro-type-cstyle-cast,
     cppcoreguidelines-pro-type-static-cast-downcast,
     google-readability-casting,

diff --git a/common/include/pcl/console/print.h b/common/include/pcl/console/print.h
@@ -45,7 +45,9 @@
 // Use e.g. like this:
 // PCL_INFO_STREAM("Info: this is a point: " << pcl::PointXYZ(1.0, 2.0, 3.0) << std::endl);
 // PCL_ERROR_STREAM("Error: an Eigen vector: " << std::endl << Eigen::Vector3f(1.0, 2.0, 3.0) << std::endl);
+// NOLINTBEGIN(bugprone-macro-parentheses)
 #define PCL_LOG_STREAM(LEVEL, STREAM, CSTR, ATTR, FG, ARGS) if(pcl::console::isVerbosityLevelEnabled(pcl::console::LEVEL)) { fflush(stdout); pcl::console::change_text_color(CSTR, pcl::console::ATTR, pcl::console::FG); STREAM << ARGS; pcl::console::reset_text_color(CSTR); }
+// NOLINTEND(bugprone-macro-parentheses)
 #define PCL_ALWAYS_STREAM(ARGS)  PCL_LOG_STREAM(L_ALWAYS,  std::cout, stdout, TT_RESET,  TT_WHITE,  ARGS)
 #define PCL_ERROR_STREAM(ARGS)   PCL_LOG_STREAM(L_ERROR,   std::cerr, stderr, TT_BRIGHT, TT_RED,    ARGS)
 #define PCL_WARN_STREAM(ARGS)    PCL_LOG_STREAM(L_WARN,    std::cerr, stderr, TT_BRIGHT, TT_YELLOW, ARGS)

diff --git a/common/include/pcl/exceptions.h b/common/include/pcl/exceptions.h
@@ -46,12 +46,14 @@
   * PCL_THROW_EXCEPTION(IOException,
   *                     "encountered an error while opening " << filename << " PCD file");
   */
+// NOLINTBEGIN(bugprone-macro-parentheses)
 #define PCL_THROW_EXCEPTION(ExceptionName, message)                         \
 {                                                                           \
   std::ostringstream s;                                                     \
   s << message;                                                             \
   throw ExceptionName(s.str(), __FILE__, BOOST_CURRENT_FUNCTION, __LINE__); \
 }
+// NOLINTEND(bugprone-macro-parentheses)
 
 namespace pcl
 {

diff --git a/common/include/pcl/pcl_macros.h b/common/include/pcl/pcl_macros.h
@@ -120,7 +120,7 @@
  * \brief A handy way to inform the user of the removal deadline
  */
 #define _PCL_PREPARE_REMOVAL_MESSAGE(Major, Minor, Msg)                                 \
-  Msg " (It will be removed in PCL " BOOST_PP_STRINGIZE(Major.Minor) ")"
+  Msg " (It will be removed in PCL " BOOST_PP_STRINGIZE((Major).Minor) ")"
 
 /**
  * \brief Tests for Minor < PCL_MINOR_VERSION
@@ -294,18 +294,18 @@ pcl_round (float number)
 #endif
 
 #define FIXED(s) \
-  std::fixed << s << std::resetiosflags(std::ios_base::fixed)
+  std::fixed << (s) << std::resetiosflags(std::ios_base::fixed)
 
 #ifndef ERASE_STRUCT
-#define ERASE_STRUCT(var) memset(&var, 0, sizeof(var))
+#define ERASE_STRUCT(var) memset(&(var), 0, sizeof(var))
 #endif
 
 #ifndef ERASE_ARRAY
-#define ERASE_ARRAY(var, size) memset(var, 0, size*sizeof(*var))
+#define ERASE_ARRAY(var, size) memset(var, 0, (size)*sizeof(*(var)))
 #endif
 
 #ifndef SET_ARRAY
-#define SET_ARRAY(var, value, size) { for (decltype(size) i = 0; i < size; ++i) var[i]=value; }
+#define SET_ARRAY(var, value, size) { for (decltype(size) i = 0; i < (size); ++i) (var)[i]=value; }
 #endif
 
 #ifndef PCL_EXTERN_C

diff --git a/features/include/pcl/features/esf.h b/features/include/pcl/features/esf.h
@@ -42,7 +42,7 @@
 
 #include <pcl/features/feature.h>
 #define GRIDSIZE 64
-#define GRIDSIZE_H GRIDSIZE/2
+#define GRIDSIZE_H (GRIDSIZE/2)
 #include <vector>
 
 namespace pcl

diff --git a/filters/include/pcl/filters/impl/conditional_removal.hpp b/filters/include/pcl/filters/impl/conditional_removal.hpp
@@ -529,8 +529,8 @@ pcl::PointDataAtOffset<PointT>::compare (const PointT& p, const double& val)
   case CASE_LABEL: {                                                                   \
     pcl::traits::asType_t<CASE_LABEL> pt_val;                                          \
     memcpy(&pt_val, pt_data + this->offset_, sizeof(pt_val));                          \
-    return (pt_val > static_cast<pcl::traits::asType_t<CASE_LABEL>>(val)) -            \
-           (pt_val < static_cast<pcl::traits::asType_t<CASE_LABEL>>(val));             \
+    return (pt_val > static_cast<pcl::traits::asType_t<(CASE_LABEL)>>(val)) -          \
+           (pt_val < static_cast<pcl::traits::asType_t<(CASE_LABEL)>>(val));           \
   }
 
   switch (datatype_)

diff --git a/io/src/ascii_io.cpp b/io/src/ascii_io.cpp
@@ -198,7 +198,7 @@ pcl::ASCIIReader::parse (
 #define ASSIGN_TOKEN(CASE_LABEL)                                                       \
   case CASE_LABEL: {                                                                   \
     *(reinterpret_cast<pcl::traits::asType_t<CASE_LABEL>*>(data_target)) =             \
-        boost::lexical_cast<pcl::traits::asType_t<CASE_LABEL>>(token);                 \
+        boost::lexical_cast<pcl::traits::asType_t<(CASE_LABEL)>>(token);               \
     return sizeof(pcl::traits::asType_t<CASE_LABEL>);                                  \
   }
   switch (field.datatype)

diff --git a/io/src/lzf.cpp b/io/src/lzf.cpp
@@ -75,7 +75,7 @@ using LZF_STATE = unsigned int[1 << (HLOG)];
 
 // IDX works because it is very similar to a multiplicative hash, e.g.
 // ((h * 57321 >> (3*8 - HLOG)) & ((1 << (HLOG)) - 1))
-#define IDX(h) ((( h >> (3*8 - HLOG)) - h  ) & ((1 << (HLOG)) - 1))
+#define IDX(h) ((( (h) >> (3*8 - HLOG)) - (h)  ) & ((1 << (HLOG)) - 1))
 
 ///////////////////////////////////////////////////////////////////////////////////////////
 //

diff --git a/io/src/pcd_io.cpp b/io/src/pcd_io.cpp
@@ -509,7 +509,7 @@ pcl::PCDReader::readBodyASCII (std::istream &fs, pcl::PCLPointCloud2 &cloud, int
         {
 #define COPY_STRING(CASE_LABEL)                                                        \
   case CASE_LABEL: {                                                                   \
-    copyStringValue<pcl::traits::asType_t<CASE_LABEL>>(                                \
+    copyStringValue<pcl::traits::asType_t<(CASE_LABEL)>>(                              \
         st.at(total + c), cloud, idx, d, c, is);                                       \
     break;                                                                             \
   }
@@ -640,11 +640,11 @@ pcl::PCDReader::readBodyBinary (const unsigned char *map, pcl::PCLPointCloud2 &c
     {
       for (uindex_t c = 0; c < cloud.fields[d].count; ++c)
       {
-#define SET_CLOUD_DENSE(CASE_LABEL)                                                    \
-  case CASE_LABEL: {                                                                   \
-    if (!isValueFinite<pcl::traits::asType_t<CASE_LABEL>>(cloud, i, point_size, d, c)) \
-      cloud.is_dense = false;                                                          \
-    break;                                                                             \
+#define SET_CLOUD_DENSE(CASE_LABEL)                                                      \
+  case CASE_LABEL: {                                                                     \
+    if (!isValueFinite<pcl::traits::asType_t<(CASE_LABEL)>>(cloud, i, point_size, d, c)) \
+      cloud.is_dense = false;                                                            \
+    break;                                                                               \
   }
         switch (cloud.fields[d].datatype)
         {
@@ -1156,7 +1156,7 @@ pcl::PCDWriter::writeASCII (const std::string &file_name, const pcl::PCLPointClo
       {
 #define COPY_VALUE(CASE_LABEL)                                                         \
   case CASE_LABEL: {                                                                   \
-    copyValueString<pcl::traits::asType_t<CASE_LABEL>>(                                \
+    copyValueString<pcl::traits::asType_t<(CASE_LABEL)>>(                              \
         cloud, i, point_size, d, c, stream);                                           \
     break;                                                                             \
   }

diff --git a/ml/include/pcl/ml/svm_wrapper.h b/ml/include/pcl/ml/svm_wrapper.h
@@ -47,6 +47,7 @@
 #include <limits> // for numeric_limits
 #include <string> // for string
 #include <vector>
+// NOLINTNEXTLINE(bugprone-macro-parentheses)
 #define Malloc(type, n) static_cast<type*>(malloc((n) * sizeof(type)))
 
 namespace pcl {

diff --git a/ml/src/svm.cpp b/ml/src/svm.cpp
@@ -104,8 +104,10 @@ powi(double base, int times)
 
 #define INF HUGE_VAL
 #define TAU 1e-12
+// NOLINTBEGIN(bugprone-macro-parentheses)
 #define Malloc(type, n) static_cast<type*>(malloc((n) * sizeof(type)))
 #define Realloc(var, type, n) static_cast<type*>(realloc(var, (n) * sizeof(type)))
+// NOLINTEND(bugprone-macro-parentheses)
 
 static void
 print_string_stdout(const char* s)

diff --git a/pcl_config.h.in b/pcl_config.h.in
@@ -14,7 +14,7 @@
 #define PCL_REVISION_VERSION ${PCL_VERSION_PATCH}
 #define PCL_DEV_VERSION ${PCL_DEV_VERSION}
 #define PCL_VERSION_PRETTY "${PCL_VERSION_PRETTY}"
-#define PCL_VERSION_CALC(MAJ, MIN, PATCH) (MAJ*100000+MIN*100+PATCH)
+#define PCL_VERSION_CALC(MAJ, MIN, PATCH) ((MAJ)*100000+(MIN)*100+(PATCH))
 #define PCL_VERSION \
     PCL_VERSION_CALC(PCL_MAJOR_VERSION, PCL_MINOR_VERSION, PCL_REVISION_VERSION)
 #define PCL_VERSION_COMPARE(OP, MAJ, MIN, PATCH) \

diff --git a/surface/include/pcl/surface/3rdparty/poisson4/poisson_exceptions.h b/surface/include/pcl/surface/3rdparty/poisson4/poisson_exceptions.h
@@ -48,12 +48,14 @@
   * Adapted from PCL_THROW_EXCEPTION. We intentionally do not reuse PCL_THROW_EXCEPTION here
   * to avoid introducing any dependencies on PCL in this 3rd party module.       
   */
+// NOLINTBEGIN(bugprone-macro-parentheses)
 #define POISSON_THROW_EXCEPTION(ExceptionName, message)                     \
 {                                                                           \
   std::ostringstream s;                                                     \
   s << message;                                                             \
   throw ExceptionName(s.str(), __FILE__, BOOST_CURRENT_FUNCTION, __LINE__); \
 }
+// NOLINTEND(bugprone-macro-parentheses)
 
 namespace pcl
 {

diff --git a/tools/oni_viewer_simple.cpp b/tools/oni_viewer_simple.cpp
@@ -58,7 +58,7 @@ do \
     if (pcl::getTime() - last >= 1.0) \
     { \
       double now = pcl::getTime (); \
-      std::cout << "Average framerate("<< _WHAT_ << "): " << double(count)/double(now - last) << " Hz" <<  std::endl; \
+      std::cout << "Average framerate("<< (_WHAT_) << "): " << double(count)/double(now - last) << " Hz" <<  std::endl; \
       count = 0; \
       last = now; \
     } \

diff --git a/tools/openni2_viewer.cpp b/tools/openni2_viewer.cpp
@@ -58,7 +58,7 @@
   ++count; \
   if (now - last >= 1.0) \
 { \
-  std::cout << "Average framerate ("<< _WHAT_ << "): " << double (count)/double (now - last) << " Hz" <<  std::endl; \
+  std::cout << "Average framerate ("<< (_WHAT_) << "): " << double (count)/double (now - last) << " Hz" <<  std::endl; \
   count = 0; \
   last = now; \
 } \

diff --git a/tools/openni_image.cpp b/tools/openni_image.cpp
@@ -97,7 +97,7 @@ do \
     ++count; \
     if (now - last >= 1.0) \
     { \
-      std::cerr << "Average framerate("<< _WHAT_ << "): " << double(count)/double(now - last) << " Hz. Queue size: " << buff.getSize () << ", number of frames written so far: " << nr_frames_total << "\n"; \
+      std::cerr << "Average framerate("<< (_WHAT_) << "): " << double(count)/double(now - last) << " Hz. Queue size: " << (buff).getSize () << ", number of frames written so far: " << nr_frames_total << "\n"; \
       count = 0; \
       last = now; \
     } \
@@ -112,7 +112,7 @@ do \
     ++count; \
     if (now - last >= 1.0) \
     { \
-      std::cerr << "Average framerate("<< _WHAT_ << "): " << double(count)/double(now - last) << " Hz. Queue size: " << buff.getSize () << "\n"; \
+      std::cerr << "Average framerate("<< (_WHAT_) << "): " << double(count)/double(now - last) << " Hz. Queue size: " << (buff).getSize () << "\n"; \
       count = 0; \
       last = now; \
     } \
@@ -128,9 +128,9 @@ do \
     if (now - last >= 1.0) \
     { \
       if (visualize && global_visualize) \
-        std::cerr << "Average framerate("<< _WHAT_ << "): " << double(count)/double(now - last) << " Hz. Queue size: " << buff1.getSize () << " (w) / " << buff2.getSize () << " (v)\n"; \
+        std::cerr << "Average framerate("<< (_WHAT_) << "): " << double(count)/double(now - last) << " Hz. Queue size: " << (buff1).getSize () << " (w) / " << (buff2).getSize () << " (v)\n"; \
       else \
-        std::cerr << "Average framerate("<< _WHAT_ << "): " << double(count)/double(now - last) << " Hz. Queue size: " << buff1.getSize () << " (w)\n"; \
+        std::cerr << "Average framerate("<< (_WHAT_) << "): " << double(count)/double(now - last) << " Hz. Queue size: " << (buff1).getSize () << " (w)\n"; \
       count = 0; \
       last = now; \
     } \

diff --git a/tools/openni_pcd_recorder.cpp b/tools/openni_pcd_recorder.cpp
@@ -200,7 +200,7 @@ do \
     ++count; \
     if (now - last >= 1.0) \
     { \
-      std::cerr << "Average framerate("<< _WHAT_ << "): " << double(count)/double(now - last) << " Hz. Queue size: " << buff.getSize () << "\n"; \
+      std::cerr << "Average framerate("<< (_WHAT_) << "): " << double(count)/double(now - last) << " Hz. Queue size: " << (buff).getSize () << "\n"; \
       count = 0; \
       last = now; \
     } \

diff --git a/tools/openni_save_image.cpp b/tools/openni_save_image.cpp
@@ -64,7 +64,7 @@ do \
     ++count; \
     if (now - last >= 1.0) \
     { \
-      std::cout << "Average framerate("<< _WHAT_ << "): " << double(count)/double(now - last) << " Hz" <<  std::endl; \
+      std::cout << "Average framerate("<< (_WHAT_) << "): " << double(count)/double(now - last) << " Hz" <<  std::endl; \
       count = 0; \
       last = now; \
     } \

diff --git a/tools/openni_viewer.cpp b/tools/openni_viewer.cpp
@@ -56,7 +56,7 @@ do \
     ++count; \
     if (now - last >= 1.0) \
     { \
-      std::cout << "Average framerate("<< _WHAT_ << "): " << double(count)/double(now - last) << " Hz" <<  std::endl; \
+      std::cout << "Average framerate("<< (_WHAT_) << "): " << double(count)/double(now - last) << " Hz" <<  std::endl; \
       count = 0; \
       last = now; \
     } \

diff --git a/tools/openni_viewer_simple.cpp b/tools/openni_viewer_simple.cpp
@@ -66,7 +66,7 @@ do \
     ++count; \
     if (now - last >= 1.0) \
     { \
-      std::cout << "Average framerate("<< _WHAT_ << "): " << double(count)/double(now - last) << " Hz" <<  std::endl; \
+      std::cout << "Average framerate("<< (_WHAT_) << "): " << double(count)/double(now - last) << " Hz" <<  std::endl; \
       count = 0; \
       last = now; \
     } \

diff --git a/tools/transform_point_cloud.cpp b/tools/transform_point_cloud.cpp
@@ -225,7 +225,7 @@ scaleInPlace (pcl::PCLPointCloud2 &cloud, double* multiplier)
 #define MULTIPLY(CASE_LABEL)                                                           \
   case CASE_LABEL: {                                                                   \
     for (int i = 0; i < 3; ++i)                                                        \
-      multiply<pcl::traits::asType_t<CASE_LABEL>>(                                     \
+      multiply<pcl::traits::asType_t<(CASE_LABEL)>>(                                   \
           cloud, xyz_offset[i], multiplier[i]);                                        \
     break;                                                                             \
   }