[impala] branch master updated: IMPALA-11695: Reduce clang tidy warning output size

[jo...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

joemcdonnell pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/impala.git


The following commit(s) were added to refs/heads/master by this push:
     new 8271bdd58 IMPALA-11695: Reduce clang tidy warning output size
8271bdd58 is described below

commit 8271bdd587d241cd5a61ccae7422bbb5fafcfaf7
Author: Joe McDonnell <jo...@cloudera.com>
AuthorDate: Mon Oct 31 11:53:07 2022 -0700

    IMPALA-11695: Reduce clang tidy warning output size
    
    The Clang Tidy build enables all warnings via -Wall
    and -Weverything. This produces enormous output.
    Looking at a recent failed Clang Tidy build, there
    are ~4.5 million warnings generated. Of these,
    about 4 million are from C++98 compatibility warnings.
    A further 250 thousand are from padding warnings.
    Since these are not particularly interesting, this
    disables both of those to reduce the output size.
    
    Disabling these warnings allowed Clang Tidy to find
    some issues in DataSketches that it was previously
    missing. Perhaps there is some limit on the number
    or size of warnings that it was processing. This
    modifies the DataSketches code to fix those (which
    are all minor issues with const correctness).
    
    Testing:
     - Built with clang tidy locally
    
    Change-Id: I28c6ed1e7a4f525d81a9c48e90d051b374d44941
    Reviewed-on: http://gerrit.cloudera.org:8080/19182
    Reviewed-by: Wenzhe Zhou <wz...@cloudera.com>
    Reviewed-by: Michael Smith <mi...@cloudera.com>
    Tested-by: Impala Public Jenkins <im...@cloudera.com>
---
 be/CMakeLists.txt                                  |  5 +++--
 be/src/thirdparty/datasketches/cpc_sketch_impl.hpp | 14 +++++++-------
 be/src/thirdparty/datasketches/kll_sketch_impl.hpp | 14 +++++++-------
 3 files changed, 17 insertions(+), 16 deletions(-)

diff --git a/be/CMakeLists.txt b/be/CMakeLists.txt
index 22ac08fc3..72022fd1f 100644
--- a/be/CMakeLists.txt
+++ b/be/CMakeLists.txt
@@ -183,8 +183,9 @@ SET(CXX_FLAGS_TIDY "${CXX_FLAGS_TIDY} -O1")
 SET(CXX_FLAGS_TIDY "${CXX_FLAGS_TIDY} -DNDEBUG")
 # Turn all warnings back on. Some will be ignored via .clang-tidy's "Checks" value, but
 # this allows different "Checks" settings to be used in different clang-tidy runs without
-# recompiling.
-SET(CXX_FLAGS_TIDY "${CXX_FLAGS_TIDY} -Wall -W -Weverything")
+# recompiling. There are two exceptions: C++98 compatibility and padding are not interesting
+# and excluding them dramatically reduces the output.
+SET(CXX_FLAGS_TIDY "${CXX_FLAGS_TIDY} -Wall -W -Weverything -Wno-c++98-compat -Wno-padded")
 # The Tidy build is so verbose (becasue of -Weverything) that it is unlikely to be viewed
 # in a terminal and most likely will be redirecto to less, a log file, or /dev/null. In
 # those places color codes just make the output harder to read.
diff --git a/be/src/thirdparty/datasketches/cpc_sketch_impl.hpp b/be/src/thirdparty/datasketches/cpc_sketch_impl.hpp
index a314de838..bf18a1e2f 100644
--- a/be/src/thirdparty/datasketches/cpc_sketch_impl.hpp
+++ b/be/src/thirdparty/datasketches/cpc_sketch_impl.hpp
@@ -430,29 +430,29 @@ void cpc_sketch_alloc<A>::serialize(std::ostream& os) const {
   );
   os.write(reinterpret_cast<const char*>(&flags_byte), sizeof(flags_byte));
   const uint16_t seed_hash(compute_seed_hash(seed));
-  os.write((char*)&seed_hash, sizeof(seed_hash));
+  os.write((const char*)&seed_hash, sizeof(seed_hash));
   if (!is_empty()) {
-    os.write((char*)&num_coupons, sizeof(num_coupons));
+    os.write((const char*)&num_coupons, sizeof(num_coupons));
     if (has_table && has_window) {
       // if there is no window it is the same as number of coupons
-      os.write((char*)&compressed.table_num_entries, sizeof(compressed.table_num_entries));
+      os.write((const char*)&compressed.table_num_entries, sizeof(compressed.table_num_entries));
       // HIP values can be in two different places in the sequence of fields
       // this is the first HIP decision point
       if (has_hip) write_hip(os);
     }
     if (has_table) {
-      os.write((char*)&compressed.table_data_words, sizeof(compressed.table_data_words));
+      os.write((const char*)&compressed.table_data_words, sizeof(compressed.table_data_words));
     }
     if (has_window) {
-      os.write((char*)&compressed.window_data_words, sizeof(compressed.window_data_words));
+      os.write((const char*)&compressed.window_data_words, sizeof(compressed.window_data_words));
     }
     // this is the second HIP decision point
     if (has_hip && !(has_table && has_window)) write_hip(os);
     if (has_window) {
-      os.write((char*)compressed.window_data.data(), compressed.window_data_words * sizeof(uint32_t));
+      os.write((const char*)compressed.window_data.data(), compressed.window_data_words * sizeof(uint32_t));
     }
     if (has_table) {
-      os.write((char*)compressed.table_data.data(), compressed.table_data_words * sizeof(uint32_t));
+      os.write((const char*)compressed.table_data.data(), compressed.table_data_words * sizeof(uint32_t));
     }
   }
 }
diff --git a/be/src/thirdparty/datasketches/kll_sketch_impl.hpp b/be/src/thirdparty/datasketches/kll_sketch_impl.hpp
index 0e0ef8742..ec5903c0a 100644
--- a/be/src/thirdparty/datasketches/kll_sketch_impl.hpp
+++ b/be/src/thirdparty/datasketches/kll_sketch_impl.hpp
@@ -399,17 +399,17 @@ void kll_sketch<T, C, S, A>::serialize(std::ostream& os) const {
     | (is_single_item ? 1 << flags::IS_SINGLE_ITEM : 0)
   );
   os.write(reinterpret_cast<const char*>(&flags_byte), sizeof(flags_byte));
-  os.write((char*)&k_, sizeof(k_));
-  os.write((char*)&m_, sizeof(m_));
+  os.write((const char*)&k_, sizeof(k_));
+  os.write((const char*)&m_, sizeof(m_));
   const uint8_t unused = 0;
   os.write(reinterpret_cast<const char*>(&unused), sizeof(unused));
   if (is_empty()) return;
   if (!is_single_item) {
-    os.write((char*)&n_, sizeof(n_));
-    os.write((char*)&min_k_, sizeof(min_k_));
-    os.write((char*)&num_levels_, sizeof(num_levels_));
-    os.write((char*)&unused, sizeof(unused));
-    os.write((char*)levels_.data(), sizeof(levels_[0]) * num_levels_);
+    os.write((const char*)&n_, sizeof(n_));
+    os.write((const char*)&min_k_, sizeof(min_k_));
+    os.write((const char*)&num_levels_, sizeof(num_levels_));
+    os.write((const char*)&unused, sizeof(unused));
+    os.write((const char*)levels_.data(), sizeof(levels_[0]) * num_levels_);
     S().serialize(os, min_value_, 1);
     S().serialize(os, max_value_, 1);
   }