diff --git a/CMakeLists.txt b/CMakeLists.txt index e55e733e..9dad87ff 100644 --- a/CMakeLists.txt +++ b/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") diff --git a/Makefile b/Makefile index 92802016..6d28bda4 100644 --- a/Makefile +++ b/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 diff --git a/doc/changelog.rst b/doc/changelog.rst index 9e171a3b..6bbf3f06 100644 --- a/doc/changelog.rst +++ b/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 diff --git a/include/addressallcommand.h b/include/addressallcommand.h index 734682d9..1d3cf498 100644 --- a/include/addressallcommand.h +++ b/include/addressallcommand.h @@ -77,4 +77,4 @@ class AddressAllCommand : public NonKeyedCommand } // namespace SmartRedis -#endif // SMARTREDIS_ADDRRESSALLCOMMAND_H \ No newline at end of file +#endif // SMARTREDIS_ADDRRESSALLCOMMAND_H diff --git a/include/addressanycommand.h b/include/addressanycommand.h index 30ce9ece..e9d1f27a 100644 --- a/include/addressanycommand.h +++ b/include/addressanycommand.h @@ -70,4 +70,4 @@ class AddressAnyCommand : public NonKeyedCommand } // namespace SmartRedis -#endif // SMARTREDIS_ADDRESSANYCOMMAND_H \ No newline at end of file +#endif // SMARTREDIS_ADDRESSANYCOMMAND_H diff --git a/include/addressatcommand.h b/include/addressatcommand.h index 4c8a763e..a097e01b 100644 --- a/include/addressatcommand.h +++ b/include/addressatcommand.h @@ -136,4 +136,4 @@ class AddressAtCommand : public NonKeyedCommand } // namespace SmartRedis -#endif // SMARTREDIS_ADDRESSATCOMMAND_H \ No newline at end of file +#endif // SMARTREDIS_ADDRESSATCOMMAND_H diff --git a/include/command.tcc b/include/command.tcc index c06f016a..52f8544d 100644 --- a/include/command.tcc +++ b/include/command.tcc @@ -45,4 +45,4 @@ void Command::add_keys(const std::vector& keyfields) } } -#endif // SMARTREDIS_COMMAND_TCC \ No newline at end of file +#endif // SMARTREDIS_COMMAND_TCC diff --git a/include/commandreply.h b/include/commandreply.h index 8fd34ca9..96e78963 100644 --- a/include/commandreply.h +++ b/include/commandreply.h @@ -29,7 +29,7 @@ #ifndef SMARTREDIS_COMMANDREPLY_H #define SMARTREDIS_COMMANDREPLY_H -#include "stdlib.h" +#include #include #include #include diff --git a/include/scalarfield.h b/include/scalarfield.h index 18bbeef1..bc816a60 100644 --- a/include/scalarfield.h +++ b/include/scalarfield.h @@ -159,4 +159,4 @@ class ScalarField : public MetadataField { } // namespace SmartRedis -#endif // SMARTREDIS_SCALARFIELD_H \ No newline at end of file +#endif // SMARTREDIS_SCALARFIELD_H diff --git a/include/sharedmemorylist.tcc b/include/sharedmemorylist.tcc index ebbb7d2b..17fc0bdc 100644 --- a/include/sharedmemorylist.tcc +++ b/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 + void operator()(T* ptr) const { + delete[] ptr; + } +}; + // Record a memory allocation template void SharedMemoryList::add_allocation(size_t bytes, T* ptr) { - std::shared_ptr s_ptr(ptr); + std::shared_ptr s_ptr(ptr, PointerDeletion()); _inventory.push_front(s_ptr); } diff --git a/include/stringfield.h b/include/stringfield.h index eeba14d8..051b9cd0 100644 --- a/include/stringfield.h +++ b/include/stringfield.h @@ -150,4 +150,4 @@ class StringField : public MetadataField { } // namespace SmartRedis -#endif // SMARTREDIS_STRINGFIELD_H \ No newline at end of file +#endif // SMARTREDIS_STRINGFIELD_H diff --git a/src/cpp/commandlist.cpp b/src/cpp/commandlist.cpp index d33ee0f1..add5474b 100644 --- a/src/cpp/commandlist.cpp +++ b/src/cpp/commandlist.cpp @@ -117,4 +117,4 @@ CommandList::const_iterator CommandList::cend() size_t CommandList::size() { return _commands.size(); -} \ No newline at end of file +} diff --git a/src/cpp/pipelinereply.cpp b/src/cpp/pipelinereply.cpp index ab29ea1a..7038631a 100644 --- a/src/cpp/pipelinereply.cpp +++ b/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))); } -} \ No newline at end of file +} diff --git a/src/cpp/rediscluster.cpp b/src/cpp/rediscluster.cpp index 2813a67b..47879184 100644 --- a/src/cpp/rediscluster.cpp +++ b/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 !!!!!! "< #include #include -#include "stdlib.h" +#include void load_mnist_image_to_array(float**** img) {