Merge pull request #386 from jpmorganchase/timkpaine-cpp_warnings

Re-enable cpp warnings

CMakeLists.txt

@@ -82,6 +82,8 @@ endif()
 
 if (PSP_CPP_BUILD AND NOT PSP_CPP_BUILD_STRICT)
 	message(WARNING "${Cyan}Building C++ without strict warnings${ColorReset}")
+else()
+	message(WARNING "${Cyan}Building C++ with strict warnings${ColorReset}")
 endif()
 
 string(TOLOWER "${CMAKE_BUILD_TYPE}" CMAKE_BUILD_TYPE_LOWER )

scripts/build_cpp.js

@@ -24,6 +24,8 @@ function docker(image = "emsdk") {
     return cmd;
 }
 
+let flags = " -DPSP_WASM_BUILD=0 -DPSP_CPP_BUILD=1 -DPSP_CPP_BUILD_TESTS=1 -DPSP_CPP_BUILD_STRICT=1";
+
 try {
     execute("mkdir -p cppbuild");
 
@@ -35,10 +37,10 @@ try {
         cmd = "cd cppbuild && ";
     }
 
-    cmd += " cmake ../ -DPSP_WASM_BUILD=0 -DPSP_CPP_BUILD=1 -DPSP_CPP_BUILD_TESTS=1";
+    cmd += ` cmake ../ ${flags}`;
 
     if (process.env.PSP_DEBUG) {
-        cmd += `-DCMAKE_BUILD_TYPE=debug`;
+        cmd += ` -DCMAKE_BUILD_TYPE=debug`;
     }
 
     if (process.env.PSP_DOCKER) {
@@ -49,6 +51,6 @@ try {
         execute("cd cppbuild && make -j${PSP_CPU_COUNT-8}");
     }
 } catch (e) {
-    console.log(e);
+    console.log(e.message);
     process.exit(1);
 }

scripts/build_python.js

@@ -29,6 +29,6 @@ try {
         execute(cmd);
     }
 } catch (e) {
-    console.log(e);
+    console.log(e.message);
     process.exit(1);
 }

scripts/test.js

@@ -108,6 +108,6 @@ try {
         }
     }
 } catch (e) {
-    console.log(e);
+    console.log(e.message);
     process.exit(1);
 }

scripts/test_cpp.js

@@ -22,13 +22,12 @@ function docker(image = "emsdk") {
 }
 
 try {
-    execute("mkdir -p cppbuild");
     if (process.env.PSP_DOCKER) {
         execute(docker("cpp") + " ./test/psp_test");
     } else {
         execute("./cppbuild/test/psp_test");
     }
 } catch (e) {
-    console.log(e);
+    console.log(e.message);
     process.exit(1);
 }

scripts/test_python.js

@@ -21,14 +21,13 @@ function docker(image = "emsdk") {
     return cmd;
 }
 
-
 // Copy .so in place to perspective package
 try {
     let cmd1 = "cp `find build -name 'libbinding.so'` python/perspective/table/";
     let cmd2 = "cp `find build -name 'libpsp.so'` python/perspective/table/";
     if (process.env.PSP_DOCKER) {
-        execute(docker("python") + " bash -c \"" + cmd1 + "\"");
-        execute(docker("python") + " bash -c \"" + cmd2 + "\"");
+        execute(docker("python") + ' bash -c "' + cmd1 + '"');
+        execute(docker("python") + ' bash -c "' + cmd2 + '"');
     } else {
         execute(cmd1);
         execute(cmd2);
@@ -40,11 +39,11 @@ try {
 try {
     let cmd = "cd python && python3 -m nose2 -v perspective --with-coverage --coverage=perspective";
     if (process.env.PSP_DOCKER) {
-        execute(docker("python") + " bash -c \"" + cmd + "\"");
+        execute(docker("python") + ' bash -c "' + cmd + '"');
     } else {
         execute(cmd);
     }
 } catch (e) {
-    console.log(e);
+    console.log(e.message);
     process.exit(1);
 }

src/cpp/aggspec.cpp

@@ -21,11 +21,9 @@ t_col_name_type::t_col_name_type(const std::string& name, t_dtype type)
     : m_name(name)
     , m_type(type) {}
 
-t_aggspec::t_aggspec()
-    : m_kernel(0) {}
+t_aggspec::t_aggspec(){}
 
-t_aggspec::t_aggspec(const t_aggspec_recipe& v)
-    : m_kernel(0) {
+t_aggspec::t_aggspec(const t_aggspec_recipe& v) {
     m_name = v.m_name;
     m_disp_name = v.m_name;
     m_agg = v.m_agg;
@@ -51,37 +49,32 @@ t_aggspec::t_aggspec(
     : m_name(name)
     , m_disp_name(name)
     , m_agg(agg)
-    , m_dependencies(dependencies)
-    , m_kernel(0) {}
+    , m_dependencies(dependencies) {}
 
 t_aggspec::t_aggspec(const std::string& aggname, t_aggtype agg, const std::string& dep)
     : m_name(aggname)
     , m_disp_name(aggname)
     , m_agg(agg)
-    , m_dependencies(std::vector<t_dep>{t_dep(dep, DEPTYPE_COLUMN)})
-    , m_kernel(0) {}
+    , m_dependencies(std::vector<t_dep>{t_dep(dep, DEPTYPE_COLUMN)}) {}
 
 t_aggspec::t_aggspec(t_aggtype agg, const std::string& dep)
     : m_agg(agg)
-    , m_dependencies(std::vector<t_dep>{t_dep(dep, DEPTYPE_COLUMN)})
-    , m_kernel(0) {}
+    , m_dependencies(std::vector<t_dep>{t_dep(dep, DEPTYPE_COLUMN)}) {}
 
 t_aggspec::t_aggspec(const std::string& name, const std::string& disp_name, t_aggtype agg,
     const std::vector<t_dep>& dependencies)
     : m_name(name)
     , m_disp_name(disp_name)
     , m_agg(agg)
-    , m_dependencies(dependencies)
-    , m_kernel(0) {}
+    , m_dependencies(dependencies) {}
 
 t_aggspec::t_aggspec(const std::string& name, const std::string& disp_name, t_aggtype agg,
     const std::vector<t_dep>& dependencies, t_sorttype sort_type)
     : m_name(name)
     , m_disp_name(disp_name)
     , m_agg(agg)
     , m_dependencies(dependencies)
-    , m_sort_type(sort_type)
-    , m_kernel(0) {}
+    , m_sort_type(sort_type) {}
 
 t_aggspec::t_aggspec(const std::string& aggname, const std::string& disp_aggname, t_aggtype agg,
     t_uindex agg_one_idx, t_uindex agg_two_idx, double agg_one_weight, double agg_two_weight)
@@ -91,10 +84,7 @@ t_aggspec::t_aggspec(const std::string& aggname, const std::string& disp_aggname
     , m_agg_one_idx(agg_one_idx)
     , m_agg_two_idx(agg_two_idx)
     , m_agg_one_weight(agg_one_weight)
-    , m_agg_two_weight(agg_two_weight)
-    , m_kernel(0)
-
-{}
+    , m_agg_two_weight(agg_two_weight) {}
 
 t_aggspec::~t_aggspec() {}
 

src/cpp/compat_impl_osx.cpp

@@ -33,28 +33,28 @@ t_uindex
 file_size(t_handle h) {
     struct stat st;
     t_index rcode = fstat(h, &st);
-    PSP_VERBOSE_ASSERT(rcode == 0, "Error in stat");
+    PSP_VERBOSE_ASSERT(rcode, == 0, "Error in stat");
     return st.st_size;
 }
 
 void
 close_file(t_handle h) {
     t_index rcode = close(h);
-    PSP_VERBOSE_ASSERT(rcode == 0, "Error closing file.");
+    PSP_VERBOSE_ASSERT(rcode, == 0, "Error closing file.");
 }
 
 void
 flush_mapping(void* base, t_uindex len) {
     t_index rcode = msync(base, len, MS_SYNC);
-    PSP_VERBOSE_ASSERT(rcode != -1, "Error in msync");
+    PSP_VERBOSE_ASSERT(rcode, != -1, "Error in msync");
 }
 
 t_rfmapping::~t_rfmapping() {
     t_index rcode = munmap(m_base, m_size);
-    PSP_VERBOSE_ASSERT(rcode == 0, "munmap failed.");
+    PSP_VERBOSE_ASSERT(rcode, == 0, "munmap failed.");
 
     rcode = close(m_fd);
-    PSP_VERBOSE_ASSERT(rcode == 0, "Error closing file.");
+    PSP_VERBOSE_ASSERT(rcode, == 0, "Error closing file.");
 }
 
 static void
@@ -69,12 +69,12 @@ map_file_internal_(const std::string& fname, t_fflag fflag, t_fflag fmode,
         size = file_size(fh.value());
     } else {
         t_index rcode = ftruncate(fh.value(), size);
-        PSP_VERBOSE_ASSERT(rcode >= 0, "ftruncate failed.");
+        PSP_VERBOSE_ASSERT(rcode, >= 0, "ftruncate failed.");
     }
 
     void* ptr = mmap(0, size, mprot, mflag, fh.value(), 0);
 
-    PSP_VERBOSE_ASSERT(ptr != MAP_FAILED, "error in mmap");
+    PSP_VERBOSE_ASSERT(ptr, != MAP_FAILED, "error in mmap");
 
     t_handle fd = fh.value();
     fh.release();
@@ -102,7 +102,7 @@ std::int64_t
 psp_curtime() {
     struct timespec t;
     std::int32_t rcode = clock_gettime(CLOCK_MONOTONIC, &t);
-    PSP_VERBOSE_ASSERT(rcode == 0, "Failure in clock_gettime");
+    PSP_VERBOSE_ASSERT(rcode, == 0, "Failure in clock_gettime");
     std::int64_t ns = t.tv_nsec + t.tv_sec * 1000000000;
     return ns;
 }
@@ -143,7 +143,6 @@ psp_curmem() {
 void
 set_thread_name(std::thread& thr, const std::string& name) {
 #ifdef PSP_PARALLEL_FOR
-    auto handle = thr.native_handle();
     pthread_setname_np(name.c_str());
 #endif
 }

src/cpp/context_grouped_pkey.cpp

@@ -26,8 +26,8 @@
 namespace perspective {
 
 t_ctx_grouped_pkey::t_ctx_grouped_pkey()
-    : m_depth_set(false)
-    , m_depth(0) {}
+    : m_depth(0),
+    m_depth_set(false) {}
 
 t_ctx_grouped_pkey::~t_ctx_grouped_pkey() {}
 
@@ -636,7 +636,7 @@ t_ctx_grouped_pkey::rebuild() {
 
 #ifdef PSP_PARALLEL_FOR
     PSP_PFOR(0, int(naggs), 1,
-        [&aggtable, &aggindices, &aggspecs, &tbl, naggs, this](int aggnum)
+        [&aggtable, &aggindices, &aggspecs, &tbl](int aggnum)
 #else
     for (t_uindex aggnum = 0; aggnum < naggs; ++aggnum)
 #endif

src/cpp/context_one.cpp

@@ -23,8 +23,8 @@ namespace perspective {
 
 t_ctx1::t_ctx1(const t_schema& schema, const t_config& pivot_config)
     : t_ctxbase<t_ctx1>(schema, pivot_config)
-    , m_depth_set(false)
-    , m_depth(0) {}
+    , m_depth(0)
+    , m_depth_set(false) {}
 
 t_ctx1::~t_ctx1() {}
 

src/cpp/context_two.cpp

@@ -17,18 +17,19 @@
 #include <perspective/traversal.h>
 
 namespace perspective {
+
 t_ctx2::t_ctx2()
-    : m_row_depth_set(false)
-    , m_column_depth_set(false)
-    , m_row_depth(0)
-    , m_column_depth(0) {}
+    : m_row_depth(0)
+    , m_row_depth_set(false)
+    , m_column_depth(0)
+    , m_column_depth_set(false) {}
 
 t_ctx2::t_ctx2(const t_schema& schema, const t_config& pivot_config)
     : t_ctxbase<t_ctx2>(schema, pivot_config)
-    , m_row_depth_set(false)
-    , m_column_depth_set(false)
     , m_row_depth(0)
-    , m_column_depth(0) {}
+    , m_row_depth_set(false)
+    , m_column_depth(0)
+    , m_column_depth_set(false) {}
 
 t_ctx2::~t_ctx2() {}
 

src/cpp/gnode.cpp

@@ -362,7 +362,7 @@ t_gnode::_process() {
 
         auto key_col = tbl->add_column("psp_pkey", DTYPE_INT64, true);
         std::int64_t start = get_table()->size();
-        for (auto ridx = 0; ridx < tbl->size(); ++ridx) {
+        for (t_uindex ridx = 0; ridx < tbl->size(); ++ridx) {
             key_col->set_nth<std::int64_t>(ridx, start + ridx);
         }
     }
@@ -947,7 +947,7 @@ t_gnode::notify_contexts(const t_table& flattened) {
     } else {
 #ifdef PSP_PARALLEL_FOR
         PSP_PFOR(0, int(num_ctx), 1,
-            [this, &notify_context_helper](int ctxidx)
+            [&notify_context_helper](int ctxidx)
 #else
         for (t_index ctxidx = 0; ctxidx < num_ctx; ++ctxidx)
 #endif

src/cpp/gnode_state.cpp

@@ -209,9 +209,8 @@ t_gstate::update_history(const t_table* tbl) {
     }
 
 #ifdef PSP_PARALLEL_FOR
-    const t_mapping* m = &m_mapping;
     PSP_PFOR(0, int(ncols), 1,
-        [m, tbl, pkey_col, op_col, &col_translation, &fcolumns, &scolumns, &stableidx_vec](
+        [tbl, op_col, &col_translation, &fcolumns, &scolumns, &stableidx_vec](
             int colidx)
 #else
     for (t_uindex colidx = 0; colidx < ncols; ++colidx)

src/cpp/port.cpp

@@ -13,8 +13,7 @@
 namespace perspective {
 
 t_port::t_port(t_port_mode mode, const t_schema& schema)
-    : m_mode(mode)
-    , m_schema(schema)
+    : m_schema(schema)
     , m_init(false)
     , m_table(nullptr)
     , m_prevsize(0) {

src/cpp/raii_impl_osx.cpp

@@ -18,7 +18,7 @@ namespace perspective {
 t_file_handle::~t_file_handle() {
     if (valid()) {
         t_index rcode = close(m_value);
-        PSP_VERBOSE_ASSERT(rcode == 0, "Error closing file.");
+        PSP_VERBOSE_ASSERT(rcode, == 0, "Error closing file.");
     }
 }
 
@@ -35,7 +35,7 @@ t_file_handle::release() {
 t_mmap_handle::~t_mmap_handle() {
     if (valid()) {
         t_index rcode = munmap(m_value, m_len);
-        PSP_VERBOSE_ASSERT(rcode == 0, "munmap failed.");
+        PSP_VERBOSE_ASSERT(rcode, == 0, "munmap failed.");
     }
 }
 

src/cpp/schema_column.cpp

@@ -16,6 +16,5 @@ t_schema_column::t_schema_column(const std::string& tblname, const std::string&
     const std::string& altname, t_dtype dtype)
     : m_tblname(tblname)
     , m_name(name)
-    , m_altname(altname)
-    , m_dtype(dtype) {}
+    , m_altname(altname) {}
 } // namespace perspective

src/cpp/sparse_tree.cpp

@@ -78,7 +78,6 @@ t_stree::t_stree(const std::vector<t_pivot>& pivots, const std::vector<t_aggspec
     , m_aggspecs(aggspecs)
     , m_schema(schema)
     , m_cur_aggidx(1)
-    , m_dotcount(0)
     , m_minmax(aggspecs.size())
     , m_has_delta(false) {
     auto g_agg_str = cfg.get_grand_agg_str();
@@ -682,9 +681,7 @@ t_stree::update_shape_from_static(const t_dtree_ctx& ctx) {
 
             sptidx = iter->m_idx;
             node.set_nstrands(nstrands);
-
-            bool replaced = m_nodes->get<by_pidx_hash>().replace(iter, node);
-            PSP_VERBOSE_ASSERT(replaced, "Failed to replace");
+            PSP_VERBOSE_ASSERT(m_nodes->get<by_pidx_hash>().replace(iter, node), , "Failed to replace"); // middle argument ignored
         }
 
         populate_pkey_idx(ctx, dtree, dptidx, sptidx, ndepth, new_idx_pkey);
@@ -1221,7 +1218,7 @@ t_stree::update_agg_table(t_uindex nidx, t_agg_update_info& info, t_uindex src_r
                 new_value.set(
                     gstate.reduce<std::function<std::uint32_t(std::vector<t_tscalar>&)>>(pkeys,
                         spec.get_dependencies()[0].name(),
-                        [this](std::vector<t_tscalar>& values) {
+                        [](std::vector<t_tscalar>& values) {
                             std::unordered_set<t_tscalar> vset;
                             for (const auto& v : values) {
                                 vset.insert(v);

src/cpp/storage.cpp

@@ -368,7 +368,7 @@ t_lstore::reserve_impl(t_uindex capacity, bool allow_shrink) {
                     void* aligned_base = nullptr;
                     int result = posix_memalign(&aligned_base,
                         std::max(sizeof(void*), size_t(m_alignment)), size_t(capacity));
-                    PSP_VERBOSE_ASSERT(result == 0, "posix_memalign failed");
+                    PSP_VERBOSE_ASSERT(result, == 0, "posix_memalign failed");
 
                     memcpy(aligned_base, base, ocapacity);
                     free(base);