Merge pull request #258 from slaperche-scality/eh/add_compiler_warnings

Add more compilation flags
scality · Nov 1, 2018 · 988f9e8 · 988f9e8
2 parents 556c521 + b9cf4c0
commit 988f9e8
Show file tree

Hide file tree

Showing 45 changed files with 815 additions and 909 deletions.
diff --git a/.circleci/config.yml b/.circleci/config.yml
@@ -6,28 +6,40 @@ jobs:
     steps:
       - checkout
       - run:
-          name: Compile with Clang 6 (Debug mode)
+          name: Compile with Clang 6 (Debug mode/No SIMD)
           command: >
             rm -rf build && mkdir build && cd build &&
-            CXX=/usr/bin/clang++-6.0 cmake -G 'Unix Makefiles' -DCMAKE_BUILD_TYPE=Debug .. &&
+            CXX=/usr/bin/clang++-6.0 cmake -G 'Unix Makefiles' -DCMAKE_BUILD_TYPE=Debug -DUSE_SIMD=OFF .. &&
             make
       - run:
-          name: Compile with Clang 6 (Release+SIMD mode)
+          name: Compile with Clang 6 (Debug mode/SSE)
           command: >
             rm -rf build && mkdir build && cd build &&
-            CXX=/usr/bin/clang++-6.0 cmake -G 'Unix Makefiles' -DCMAKE_BUILD_TYPE=Release -DUSE_SIMD=ON .. &&
+            CXX=/usr/bin/clang++-6.0 cmake -G 'Unix Makefiles' -DCMAKE_BUILD_TYPE=Debug -DUSE_SIMD=SSE .. &&
             make
       - run:
-          name: Compile with GCC 8 (Debug mode)
+          name: Compile with Clang 6 (Debug mode/SSE)
           command: >
             rm -rf build && mkdir build && cd build &&
-            CXX=/usr/bin/g++-8 cmake -G 'Unix Makefiles' -DCMAKE_BUILD_TYPE=Debug .. &&
+            CXX=/usr/bin/clang++-6.0 cmake -G 'Unix Makefiles' -DCMAKE_BUILD_TYPE=Debug -DUSE_SIMD=AVX .. &&
             make
       - run:
-          name: Compile with GCC 8 (Release+SIMD mode)
+          name: Compile with GCC 8 (Debug mode/No SIMD)
           command: >
             rm -rf build && mkdir build && cd build &&
-            CXX=/usr/bin/g++-8 cmake -G 'Unix Makefiles' -DCMAKE_BUILD_TYPE=Release -DUSE_SIMD=ON .. &&
+            CXX=/usr/bin/g++-8 cmake -G 'Unix Makefiles' -DCMAKE_BUILD_TYPE=Debug -DUSE_SIMD=OFF .. &&
+            make
+      - run:
+          name: Compile with GCC 8 (Debug mode/SSE)
+          command: >
+            rm -rf build && mkdir build && cd build &&
+            CXX=/usr/bin/g++-8 cmake -G 'Unix Makefiles' -DCMAKE_BUILD_TYPE=Debug -DUSE_SIMD=SSE .. &&
+            make
+      - run:
+          name: Compile with GCC 8 (Debug mode/AVX)
+          command: >
+            rm -rf build && mkdir build && cd build &&
+            CXX=/usr/bin/g++-8 cmake -G 'Unix Makefiles' -DCMAKE_BUILD_TYPE=Debug -DUSE_SIMD=AVX .. &&
             make
   android:
     docker:

diff --git a/CMakeLists.txt b/CMakeLists.txt
@@ -120,18 +120,44 @@ include(CheckCXXCompilerFlag)
 
 set(COMMON_CXX_FLAGS
   -pipe
-  -Wall
+  -Wall -Wextra -Wdeprecated -Wpedantic
+  -Wdocumentation -Wdocumentation-pedantic
+  -Wno-documentation-unknown-command  # Doesn't recognize @cite :(
+  -Wunused-exception-parameter -Wunused-macros -Wunused-member-function -Wunused-template
+  -Wgcc-compat -Wgnu
+  -Warray-bounds-pointer-arithmetic
+  -Wassign-enum
+  -Wbad-function-cast -Wcast-qual -Wcast-align -Wold-style-cast -Wuseless-cast
+  -Wconditional-uninitialized
+  -Wduplicate-method-arg -Wduplicate-method-match
+  -Wfloat-equal -Wdouble-promotion
+  -Wformat-pedantic
+  -Winit-self
+  -Wimplicit-fallthrough=4
+  -Wlogical-op
+  -Wmissing-noreturn -Wmissing-prototypes -Wmissing-include-dirs -Wmissing-declarations
+  -Wnested-anon-types
+  -Wnon-virtual-dtor
+  -Woverlength-strings -Wwrite-strings
+  -Woverloaded-virtual
+  -Wredundant-decls
+  -Wshift-sign-overflow
+  -Wstring-compare -Wstring-conversion -Wstring-plus-char
+  -Wstrict-null-sentinel
+  -Wsometimes-uninitialized
+  -Wswitch-enum
+  -Wunreachable-code-aggressive
 )
 
 # Option for enabling/disabling SIMD flags is for both of debug and release
 if (USE_SIMD STREQUAL "ON")
-  SET(COMMON_CXX_FLAGS "${COMMON_CXX_FLAGS} -march=native")
+  list(APPEND COMMON_CXX_FLAGS "-march=native")
   add_definitions(-DQUADIRON_USE_SIMD)
 elseif (USE_SIMD STREQUAL "SSE")
-  SET(COMMON_CXX_FLAGS "${COMMON_CXX_FLAGS} -msse4.1")
+  list(APPEND COMMON_CXX_FLAGS "-msse4.1")
   add_definitions(-DQUADIRON_USE_SIMD)
 elseif (USE_SIMD STREQUAL "AVX")
-  SET(COMMON_CXX_FLAGS "${COMMON_CXX_FLAGS} -mavx2")
+  list(APPEND COMMON_CXX_FLAGS "-mavx2")
   add_definitions(-DQUADIRON_USE_SIMD)
 endif()
 

diff --git a/benchmark/benchmark.cpp b/benchmark/benchmark.cpp
@@ -181,6 +181,8 @@ int Benchmark<T>::init()
         fec = new quadiron::fec::RsFnt<T>(
             quadiron::fec::FecType::SYSTEMATIC, word_size, k, m, pkt_size);
         break;
+    case EC_TYPE_ALL: /* FALLTHROUGH */
+    case EC_TYPE_END: /* FALLTHROUGH */
     default:
         return ERR_FEC_TYPE_NOT_SUPPORTED;
     }
@@ -214,18 +216,18 @@ int Benchmark<T>::init()
     c_ostreambufs = new std::vector<ostreambuf<char>*>(n_c);
     r_ostreambufs = new std::vector<ostreambuf<char>*>(k);
     for (i = 0; i < k; i++) {
-        d_istreambufs->at(i) =
-            new istreambuf<char>((char*)d_chunks->at(i), chunk_size);
+        d_istreambufs->at(i) = new istreambuf<char>(
+            reinterpret_cast<char*>(d_chunks->at(i)), chunk_size);
     }
     for (i = 0; i < n_c; i++) {
-        c_istreambufs->at(i) =
-            new istreambuf<char>((char*)c_chunks->at(i), chunk_size);
-        c_ostreambufs->at(i) =
-            new ostreambuf<char>((char*)c_chunks->at(i), chunk_size);
+        c_istreambufs->at(i) = new istreambuf<char>(
+            reinterpret_cast<char*>(c_chunks->at(i)), chunk_size);
+        c_ostreambufs->at(i) = new ostreambuf<char>(
+            reinterpret_cast<char*>(c_chunks->at(i)), chunk_size);
     }
     for (i = 0; i < k; i++) {
-        r_ostreambufs->at(i) =
-            new ostreambuf<char>((char*)r_chunks->at(i), chunk_size);
+        r_ostreambufs->at(i) = new ostreambuf<char>(
+            reinterpret_cast<char*>(r_chunks->at(i)), chunk_size);
     }
 
     // Allocate memory for streams
@@ -632,7 +634,7 @@ bool Benchmark<T>::enc_dec()
     return true;
 }
 
-void xusage()
+[[noreturn]] static void xusage()
 {
     std::cerr << "Usage: benchmark [options]\n"
               << "Options:\n"
@@ -661,7 +663,7 @@ void xusage()
               << "2, 4, 8, 16 for uint16_t, uint32_t, uint64_t, __uint128_t\n"
               << "\t-g \tNumber of threads\n"
               << "\t-x \tExtra parameter\n\n";
-    exit(1);
+    std::exit(EXIT_FAILURE);
 }
 
 template <typename T>
@@ -686,12 +688,12 @@ void init_run_bench(Params_t* params)
     try {
         Benchmark<T> bench(params);
         run<T>(&bench, params);
-    } catch (const std::exception& e) {
+    } catch (const std::exception&) {
         return;
     }
 }
 
-void run_scenario(Params_t* params)
+static void run_scenario(Params_t* params)
 {
     // get sizeof_T if necessary
     params->get_sizeof_T();
@@ -716,19 +718,19 @@ void run_scenario(Params_t* params)
     }
 }
 
-void run_benchmark(Params_t* params)
+static void run_benchmark(Params_t* params)
 {
     if (params->fec_type == EC_TYPE_ALL) {
         for (int type = EC_TYPE_ALL + 1; type < EC_TYPE_END; type++) {
-            params->fec_type = (ec_type)type;
+            params->fec_type = static_cast<ec_type>(type);
             run_scenario(params);
         }
     } else {
         run_scenario(params);
     }
 }
 
-void do_join(std::thread& t)
+static void do_join(std::thread& t)
 {
     t.join();
 }

diff --git a/benchmark/prng.h b/benchmark/prng.h
@@ -75,36 +75,36 @@ class PRNG {
 
     void gen_chunk(void* chunk, size_t size)
     {
-        uint8_t* buffer = (uint8_t*)chunk;
+        uint8_t* buffer = static_cast<uint8_t*>(chunk);
         if (size < 4) {
-            buffer[0] = (uint8_t)_rand();
+            buffer[0] = static_cast<uint8_t>(_rand()); // narrow_cast
             for (size_t i = 1; i < size; i++) {
                 buffer[i] = buffer[0];
             }
         } else {
             uint32_t crc = 0;
             crc = ~crc;
             for (size_t i = 4; i < size; i++) {
-                uint8_t val = (uint8_t)_rand();
+                uint8_t val = static_cast<uint8_t>(_rand()); // narrow_cast
                 buffer[i] = val;
                 crc ^= val;
                 for (int k = 0; k < 8; k++)
                     crc = crc & 1 ? (crc >> 1) ^ POLY : crc >> 1;
             }
-            *(uint32_t*)buffer = ~crc;
+            *reinterpret_cast<uint32_t*>(buffer) = ~crc;
         }
     }
 
     uint32_t get_crc(void* chunk, size_t size)
     {
         if (size < 4)
             return 0;
-        return *(uint32_t*)chunk;
+        return *static_cast<uint32_t*>(chunk);
     }
 
     bool check_chunk(void* chunk, size_t size)
     {
-        uint8_t* buffer = (uint8_t*)chunk;
+        uint8_t* buffer = static_cast<uint8_t*>(chunk);
         if (size < 4) {
             uint8_t val = buffer[0];
             for (size_t i = 1; i < size; i++) {
@@ -119,7 +119,7 @@ class PRNG {
                 for (int k = 0; k < 8; k++)
                     crc = crc & 1 ? (crc >> 1) ^ POLY : crc >> 1;
             }
-            uint32_t _crc = *(uint32_t*)buffer;
+            uint32_t _crc = *reinterpret_cast<uint32_t*>(buffer);
             if (_crc != ~crc)
                 return false;
         }

diff --git a/benchmark/stats.h b/benchmark/stats.h
@@ -63,8 +63,9 @@ class Stats_t {
 
     void end()
     {
-        avg = (double)sum / (double)nb;
-        std_dev = sqrt((double)sum_2 / (double)nb - avg * avg);
+        avg = static_cast<double>(sum) / static_cast<double>(nb);
+        std_dev = sqrt(
+            static_cast<double>(sum_2) / static_cast<double>(nb) - avg * avg);
     }
 
     void show()
@@ -86,7 +87,7 @@ class Stats_t {
 
     double get_thrpt()
     {
-        return (double)work_load / avg;
+        return work_load / avg;
     }
 
   private:

diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt
@@ -29,7 +29,6 @@ include(GNUInstallDirs)
 
 # Source files.
 set(LIB_SRC
-  ${SOURCE_DIR}/core.cpp
   ${SOURCE_DIR}/fec_vectorisation.cpp
   ${SOURCE_DIR}/misc.cpp
   ${SOURCE_DIR}/gf_nf4.cpp