Fix pedantic

CMakeLists.txt

@@ -53,11 +53,23 @@ endif()
 # For now, we only support Pedantic on the main library build.
 # If/when we fine-tune the examples and test cases, move this block
 # to smartredis_defs.cmake
+# Note: -Wextra can be added after unused parameters are addressed
 if (SR_PEDANTIC)
-    if((CMAKE_CXX_COMPILER_ID STREQUAL "GNU") AND (CMAKE_C_COMPILER_ID STREQUAL "GNU"))
-        add_compile_options(-Wall -Werror)
+    if(
+        (CMAKE_CXX_COMPILER_ID STREQUAL "GNU") OR
+        (CMAKE_CXX_COMPILER_ID STREQUAL "NVHPC")
+    )
+	set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Wall -Werror -pedantic -Wextra")
+	set(CMAKE_Fortran_FLAGS "${CMAKE_Fortran_FLAGS} -Wall -Werror -Wextra")
+    elseif(
+        (CMAKE_CXX_COMPILER_ID STREQUAL "Intel") OR
+        (CMAKE_CXX_COMPILER_ID STREQUAL "IntelLLVM")
+    )
+        set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Wall -Werror -pedantic")
+        set(CMAKE_Fortran_FLAGS "${CMAKE_Fortran_FLAGS} -warn all -warn error")
     else()
-        message(WARNING "SR_PEDANTIC was specified, but the CMAKE compiler is not GCC")
+        message(WARNING "SR_PEDANTIC not supported for ${CMAKE_CXX_COMPILER_ID}")
+        message(WARNING ${CMAKE_CXX_COMPILER_ID})
     endif()
     if(CMAKE_Fortran_COMPILER_ID STREQUAL "GNU")
         set(CMAKE_Fortran_FLAGS "${CMAKE_Fortran_FLAGS} -Wno-maybe-uninitialized")

Makefile

@@ -137,12 +137,12 @@ lib-with-fortran: lib
 
 # help: test-lib                       - Build SmartRedis clients into a dynamic library with least permissive compiler settings
 .PHONY: test-lib
-test-lib: SR_PEDANTIC=OFF #TODO: fix warnings in C++
+test-lib: SR_PEDANTIC=ON
 test-lib: lib
 
 # help: test-lib-with-fortran          - Build SmartRedis clients into a dynamic library with least permissive compiler settings
 .PHONY: test-lib-with-fortran
-test-lib-with-fortran: SR_PEDANTIC=OFF #TODO: fix warnings in C++
+test-lib-with-fortran: SR_PEDANTIC=ON
 test-lib-with-fortran: lib-with-fortran
 
 # help: test-deps                      - Make SmartRedis testing dependencies

doc/changelog.rst

@@ -8,6 +8,7 @@ To be released at some future point in time
 
 Description
 
+- Fix C++ cosmetic defects leading to compiler warnings
 - Enforce changelog updates
 - Removed unused TensorBase constructor parameter
 - Remove unused parameter in internal redis cluster method
@@ -17,6 +18,9 @@ Description
 
 Detailed Notes
 
+- Fixes some mainly cosmetic defects in the C++ client that were leading to warnings
+  when pedantic compiler flags were enabled (PR476_)
+- Re-enable SR_PEDANTIC for the `test-lib` and `test-lib-with-fortran` Makefile targets (PR476_)
 - Add Github Actions workflow that checks if changelog is edited
   on pull requests into develop. (PR480_)
 - The TensorBase constructor SRMemoryLayout parameter was removed because it was
@@ -31,6 +35,7 @@ Detailed Notes
   installed to ensure consistent versions. (PR475_)
 - Fix an inconsistency in the C-API ConfigOptions is_configured() parameter names. (PR471_)
 
+.. _PR476: https://github.com/CrayLabs/SmartRedis/pull/476
 .. _PR480: https://github.com/CrayLabs/SmartRedis/pull/480
 .. _PR479: https://github.com/CrayLabs/SmartRedis/pull/479
 .. _PR478: https://github.com/CrayLabs/SmartRedis/pull/478
@@ -51,7 +56,7 @@ Description
 Detailed Notes
 
 - A previous bug fix for the Python client which addressed a problem when sending
-  numpy views inadvertently kept the original put_tensor call in place. This 
+  numpy views inadvertently kept the original put_tensor call in place. This
   essentially doubles the cost of the operation. (PR464_)
 
 .. _PR464: https://github.com/CrayLabs/SmartRedis/pull/464

include/addressallcommand.h

@@ -77,4 +77,4 @@ class AddressAllCommand : public NonKeyedCommand
 
 } // namespace SmartRedis
 
-#endif // SMARTREDIS_ADDRRESSALLCOMMAND_H
+#endif // SMARTREDIS_ADDRRESSALLCOMMAND_H

include/addressanycommand.h

@@ -70,4 +70,4 @@ class AddressAnyCommand : public NonKeyedCommand
 
 } // namespace SmartRedis
 
-#endif // SMARTREDIS_ADDRESSANYCOMMAND_H
+#endif // SMARTREDIS_ADDRESSANYCOMMAND_H

include/addressatcommand.h

@@ -136,4 +136,4 @@ class AddressAtCommand : public NonKeyedCommand
 
 } // namespace SmartRedis
 
-#endif // SMARTREDIS_ADDRESSATCOMMAND_H
+#endif // SMARTREDIS_ADDRESSATCOMMAND_H

include/command.tcc

@@ -45,4 +45,4 @@ void Command::add_keys(const std::vector<T>& keyfields)
     }
 }
 
-#endif // SMARTREDIS_COMMAND_TCC
+#endif // SMARTREDIS_COMMAND_TCC

include/commandreply.h

@@ -29,7 +29,7 @@
 #ifndef SMARTREDIS_COMMANDREPLY_H
 #define SMARTREDIS_COMMANDREPLY_H
 
-#include "stdlib.h"
+#include <stdlib.h>
 #include <sw/redis++/redis++.h>
 #include <iostream>
 #include <vector>

include/scalarfield.h

@@ -159,4 +159,4 @@ class ScalarField : public MetadataField {
 
 } // namespace SmartRedis
 
-#endif // SMARTREDIS_SCALARFIELD_H
+#endif // SMARTREDIS_SCALARFIELD_H

include/sharedmemorylist.tcc

@@ -29,11 +29,21 @@
 #ifndef SMARTREDIS_SHAREDMEMORYLIST_TCC
 #define SMARTREDIS_SHAREDMEMORYLIST_TCC
 
+// Define memory deletion operator (delete[]) to
+// match the memory allocation operator (new[])
+class PointerDeletion {
+    public:
+        template<class T>
+        void operator()(T* ptr) const {
+            delete[] ptr;
+        }
+};
+
 // Record a memory allocation
 template <class T>
 void SharedMemoryList<T>::add_allocation(size_t bytes, T* ptr)
 {
-    std::shared_ptr<T> s_ptr(ptr);
+    std::shared_ptr<T> s_ptr(ptr, PointerDeletion());
     _inventory.push_front(s_ptr);
 }
 

include/stringfield.h

@@ -150,4 +150,4 @@ class StringField : public MetadataField {
 
 } // namespace SmartRedis
 
-#endif // SMARTREDIS_STRINGFIELD_H
+#endif // SMARTREDIS_STRINGFIELD_H

src/cpp/commandlist.cpp

@@ -117,4 +117,4 @@ CommandList::const_iterator CommandList::cend()
 size_t CommandList::size()
 {
     return _commands.size();
-}
+}

src/cpp/pipelinereply.cpp

@@ -110,4 +110,4 @@ void PipelineReply::_add_queuedreplies(QueuedReplies&& reply)
     for (size_t i = 0; i < n_replies; i++) {
         _all_replies.push_back(&(_queued_replies.back().get(i)));
     }
-}
+}

src/cpp/rediscluster.cpp

@@ -238,9 +238,13 @@ PipelineReply RedisCluster::run_via_unordered_pipelines(CommandList& cmd_list)
     volatile size_t pipeline_completion_count = 0;
     size_t num_shards = shard_cmd_index_list.size();
     Exception error_response = Exception("no error");
-    bool success_status[num_shards];
+    bool* success_status = new bool[num_shards];
     std::mutex results_mutex;
 
+    for (size_t s = 0; s < num_shards; s++) {
+        success_status[s] = false;
+    }
+
     // Loop over all shards and execute pipelines
     for (size_t s = 0; s < num_shards; s++) {
         // Get shard prefix
@@ -271,6 +275,7 @@ PipelineReply RedisCluster::run_via_unordered_pipelines(CommandList& cmd_list)
             }
             catch (Exception& e) {
                 error_response = e;
+                std::cout<<"AN ERROR WAS FOUND !!!!!! "<<e.what()<<std::endl;
                 success_status[s] = false;
             }
 
@@ -301,7 +306,13 @@ PipelineReply RedisCluster::run_via_unordered_pipelines(CommandList& cmd_list)
 
     // Throw an exception if one was generated in processing the threads
     for (size_t i = 0; i < num_shards; i++) {
+        std::cout<<"Checking shard "<<i<<std::endl;
         if (!success_status[i]) {
+            for(size_t j = 0; j < num_shards; j++) {
+                std::cout<<"The size of the shard command list " << j << " is "<<shard_cmd_index_list[j].size()<<std::endl;
+                std::cout<<"Shard "<<j<<" is "<<success_status[j]<<std::endl;
+            }
+            std::cout<<"The error response is "<<error_response.what()<<std::endl;
             throw error_response;
         }
     }
@@ -310,6 +321,8 @@ PipelineReply RedisCluster::run_via_unordered_pipelines(CommandList& cmd_list)
     // with order of execution
     all_replies.reorder(cmd_list_index_ooe);
 
+    delete[] success_status;
+
     return all_replies;
 }
 
@@ -1551,3 +1564,5 @@ std::string RedisCluster::to_string() const
     result += RedisServer::to_string();
     return result;
 }
+
+

src/cpp/threadpool.cpp

@@ -22,8 +22,7 @@ ThreadPool::ThreadPool(const SRObject* context, unsigned int num_threads)
     shutting_down = false;
 
     // By default, we'll make one thread for each hardware context
-    if (num_threads == 0)
-        num_threads = std::thread::hardware_concurrency();
+    if (num_threads == 0) num_threads = std::thread::hardware_concurrency();
 
     // Create worker threads
 	if (num_threads < 1) num_threads = 1; // Force a minimum of 1 thread

tests/cpp/client_test_ensemble.cpp

@@ -31,7 +31,7 @@
 #include <vector>
 #include <string>
 #include <cstdlib>
-#include "stdlib.h"
+#include <stdlib.h>
 
 void load_mnist_image_to_array(float**** img)
 {