From fcd71843f42425041d11d9402002eedb02e195f4 Mon Sep 17 00:00:00 2001 From: Kostis Sagonas Date: Wed, 7 Feb 2024 15:37:56 +0100 Subject: [PATCH] Eliminate `using namespace` commands (#130) * Remove unused `using namespace` * Substitute `using namespace` commands with more specific `using` ones * Avoid `using namespace` by namespace-qualified calls * Make anonymous namespace an internal one inside the signal namespace * Enable the build/namespaces cpplint check in actions --- .github/workflows/cpplint.yml | 2 +- src/allocators/allocators.cpp | 17 ++++------ src/allocators/collective_allocator.hpp | 8 ++--- src/argo.cpp | 15 +++++---- src/backend/mpi/mpi.cpp | 3 -- src/backend/singlenode/singlenode.cpp | 8 ++--- src/mempools/global_mempool.hpp | 5 +-- src/signal/signal.hpp | 26 +++++++-------- tests/allocators.cpp | 43 +++++++++++++------------ tests/stlallocation.cpp | 18 ++++------- 10 files changed, 66 insertions(+), 79 deletions(-) diff --git a/.github/workflows/cpplint.yml b/.github/workflows/cpplint.yml index 1adda990..afa6dd8e 100644 --- a/.github/workflows/cpplint.yml +++ b/.github/workflows/cpplint.yml @@ -7,4 +7,4 @@ jobs: - uses: actions/checkout@v3 - uses: actions/setup-python@v4 - run: pip install cpplint - - run: cpplint --quiet --root=../ --filter=-build/c++11,-build/include,-build/namespaces,-runtime/array,-runtime/string,-whitespace/braces,-whitespace/indent,-whitespace/line_length,-whitespace/tab --recursive src tests + - run: cpplint --quiet --root=../ --filter=-build/c++11,-build/include,-runtime/array,-runtime/string,-whitespace/braces,-whitespace/indent,-whitespace/line_length,-whitespace/tab --recursive src tests diff --git a/src/allocators/allocators.cpp b/src/allocators/allocators.cpp index 40b715a9..f2017265 100644 --- a/src/allocators/allocators.cpp +++ b/src/allocators/allocators.cpp @@ -6,7 +6,6 @@ #include "allocators.hpp" -namespace mem = argo::mempools; namespace alloc = argo::allocators; /* default allocators */ @@ -16,32 +15,28 @@ alloc::collective_allocator alloc::default_collective_allocator; extern "C" void* collective_alloc(size_t size) { - using namespace argo::allocators; /** @bug this is wrong: either it should not be done at all, or also when using the C++ interface */ argo::backend::barrier(); - return static_cast(default_collective_allocator.allocate(size)); + return static_cast(alloc::default_collective_allocator.allocate(size)); } extern "C" void collective_free(void* ptr) { - using namespace argo::allocators; - using atype = decltype(default_collective_allocator)::value_type; + using atype = decltype(alloc::default_collective_allocator)::value_type; if (ptr == NULL) return; - default_collective_allocator.free(static_cast(ptr)); + alloc::default_collective_allocator.free(static_cast(ptr)); } extern "C" void* dynamic_alloc(size_t size) { - using namespace argo::allocators; - return static_cast(default_dynamic_allocator.allocate(size)); + return static_cast(alloc::default_dynamic_allocator.allocate(size)); } extern "C" void dynamic_free(void* ptr) { - using namespace argo::allocators; - using atype = decltype(default_dynamic_allocator)::value_type; + using atype = decltype(alloc::default_dynamic_allocator)::value_type; if (ptr == NULL) return; - default_dynamic_allocator.free(static_cast(ptr)); + alloc::default_dynamic_allocator.free(static_cast(ptr)); } diff --git a/src/allocators/collective_allocator.hpp b/src/allocators/collective_allocator.hpp index fc9f5db9..7668d0fc 100644 --- a/src/allocators/collective_allocator.hpp +++ b/src/allocators/collective_allocator.hpp @@ -107,7 +107,7 @@ T* conew_(Ps&&... ps) { } void* ptr = collective_alloc(sizeof(T)); - using namespace data_distribution; + using data_distribution::global_ptr; global_ptr gptr(ptr, false, false); // The home node of ptr handles initialization if (initialize && argo::backend::node_id() == gptr.node()) { @@ -165,7 +165,7 @@ void codelete_(T* ptr) { synchronize = false; } - using namespace data_distribution; + using data_distribution::global_ptr; global_ptr gptr(ptr, false, false); // The home node of ptr handles deinitialization if (deinitialize && argo::backend::node_id() == gptr.node()) { @@ -218,7 +218,7 @@ T* conew_array(size_t size) { } void* ptr = collective_alloc(sizeof(T) * size); - using namespace data_distribution; + using data_distribution::global_ptr; global_ptr gptr(ptr, false, false); // The home node of ptr handles initialization if (initialize && argo::backend::node_id() == gptr.node()) { @@ -269,7 +269,7 @@ void codelete_array(T* ptr) { synchronize = false; } - using namespace data_distribution; + using data_distribution::global_ptr; global_ptr gptr(ptr, false, false); // The home node of ptr handles deinitialization if (deinitialize && argo::backend::node_id() == gptr.node()) { diff --git a/src/argo.cpp b/src/argo.cpp index 6c2ee383..2f62adca 100644 --- a/src/argo.cpp +++ b/src/argo.cpp @@ -88,13 +88,14 @@ namespace argo { */ static void reset_allocators() { default_global_mempool->reset(); - using namespace alloc; - using namespace mem; - collective_prepool = dynamic_memory_pool(&default_global_allocator); - dynamic_prepool = dynamic_memory_pool(&default_global_allocator); - default_global_allocator = global_allocator(); - default_dynamic_allocator = default_dynamic_allocator_t(); - default_collective_allocator = collective_allocator(); + using alloc::default_dynamic_allocator; + using alloc::default_collective_allocator; + using alloc::default_global_allocator; + collective_prepool = mem::dynamic_memory_pool(&default_global_allocator); + dynamic_prepool = mem::dynamic_memory_pool(&default_global_allocator); + default_global_allocator = alloc::global_allocator(); + default_dynamic_allocator = alloc::default_dynamic_allocator_t(); + default_collective_allocator = alloc::collective_allocator(); default_global_allocator.set_mempool(default_global_mempool); default_dynamic_allocator.set_mempool(&dynamic_prepool); default_collective_allocator.set_mempool(&collective_prepool); diff --git a/src/backend/mpi/mpi.cpp b/src/backend/mpi/mpi.cpp index 690a9fb3..3053e12d 100644 --- a/src/backend/mpi/mpi.cpp +++ b/src/backend/mpi/mpi.cpp @@ -25,7 +25,6 @@ */ static MPI_Datatype fitting_mpi_int(std::size_t size) { MPI_Datatype t_type; - using namespace argo; switch (size) { // Until UCX supports 1/2 byte atomics, they must be disabled @@ -60,7 +59,6 @@ static MPI_Datatype fitting_mpi_int(std::size_t size) { */ static MPI_Datatype fitting_mpi_uint(std::size_t size) { MPI_Datatype t_type; - using namespace argo; switch (size) { // Until UCX supports 1/2 byte atomics, they must be disabled @@ -93,7 +91,6 @@ static MPI_Datatype fitting_mpi_uint(std::size_t size) { */ static MPI_Datatype fitting_mpi_float(std::size_t size) { MPI_Datatype t_type; - using namespace argo; switch (size) { case 4: diff --git a/src/backend/singlenode/singlenode.cpp b/src/backend/singlenode/singlenode.cpp index 8ed91045..32cf405e 100644 --- a/src/backend/singlenode/singlenode.cpp +++ b/src/backend/singlenode/singlenode.cpp @@ -21,7 +21,6 @@ namespace vm = argo::virtual_memory; namespace sig = argo::signal; -using namespace argo::backend; /** @brief a lock for atomically executed operations */ std::mutex atomic_op_mutex; @@ -85,12 +84,12 @@ void init(std::size_t argo_size, std::size_t cache_size) { (void)(cache_size); memory = static_cast(vm::allocate_mappable(PAGE_SIZE, argo_size)); memory_size = argo_size; - using namespace data_distribution; + using argo::data_distribution::base_distribution; base_distribution<0>::set_memory_space(nodes, memory, argo_size); sig::signal_handler::install_argo_handler(&singlenode_handler); /** @note first-touch needs a directory for fetching * the homenode and offset for an address */ - if (is_first_touch_policy()) { + if (argo::data_distribution::is_first_touch_policy()) { /* calculate the directory size and allocate memory */ std::size_t owners_dir_size = 3*(argo_size/PAGE_SIZE); std::size_t owners_dir_size_bytes = owners_dir_size*sizeof(std::size_t); @@ -132,10 +131,9 @@ void reset_stats() {} void finalize() {} void reset_coherence() { - using namespace data_distribution; /** @note first-touch needs a directory for fetching * the homenode and offset for an address */ - if (is_first_touch_policy()) { + if (argo::data_distribution::is_first_touch_policy()) { /* calculate the directory size and allocate memory */ std::size_t owners_dir_size = 3*(memory_size/PAGE_SIZE); std::size_t owners_dir_size_bytes = owners_dir_size*sizeof(std::size_t); diff --git a/src/mempools/global_mempool.hpp b/src/mempools/global_mempool.hpp index f759dc70..9bbcc365 100644 --- a/src/mempools/global_mempool.hpp +++ b/src/mempools/global_mempool.hpp @@ -54,7 +54,7 @@ class global_memory_pool { memory = backend::global_base(); max_size = backend::global_size(); /**@todo this initialization should move to tools::init() land */ - using namespace data_distribution; + using argo::data_distribution::base_distribution; base_distribution<0>::set_memory_space(nodes, memory, max_size); // Reset maximum size to the full memory size minus the space reserved for internal use @@ -68,6 +68,7 @@ class global_memory_pool { global_tas_lock = new tas_lock(field); // Home node makes sure that offset points to Argo's starting address + using argo::data_distribution::global_ptr; global_ptr gptr(&memory[max_size]); if(backend::node_id() == gptr.node()) { *offset = static_cast(0); @@ -93,7 +94,7 @@ class global_memory_pool { max_size = backend::global_size() - reserved; // Home node makes sure that offset points to Argo's starting address - using namespace data_distribution; + using argo::data_distribution::global_ptr; global_ptr gptr(&memory[max_size]); if(backend::node_id() == gptr.node()) { *offset = static_cast(0); diff --git a/src/signal/signal.hpp b/src/signal/signal.hpp index fd43a573..0c51e93f 100644 --- a/src/signal/signal.hpp +++ b/src/signal/signal.hpp @@ -40,7 +40,9 @@ enum x86_pf_error_code { }; #endif /* REG_ERR */ -namespace { +namespace argo { +namespace signal { +namespace sig_internal { /** @brief typedef for signal handlers */ using sig_handler = struct sigaction; /** @brief typedef for function type used by ArgoDSM */ @@ -48,10 +50,8 @@ using handler_ftype = void(*)(int, siginfo_t*, void*); /** @brief error message string */ const std::string msg_argo_unitialized = "ArgoDSM must be configured to capture a signal before application handlers can be installed"; -} // namespace +} // namespace sig_internal -namespace argo { -namespace signal { /** * @brief Originating access type of segfault */ @@ -69,9 +69,9 @@ template class signal_handler { private: /** @brief signal handling function for ArgoDSM */ - static handler_ftype argo_handler; + static sig_internal::handler_ftype argo_handler; /** @brief signal handler for application use */ - static sig_handler application_handler; + static sig_internal::sig_handler application_handler; public: /** @@ -80,9 +80,9 @@ class signal_handler { * @details The function will only be called for signals * that relate to ArgoDSM's internal workings */ - static void install_argo_handler(const handler_ftype h) { + static void install_argo_handler(const sig_internal::handler_ftype h) { argo_handler = h; - sig_handler s; + sig_internal::sig_handler s; s.sa_flags = SA_SIGINFO; s.sa_sigaction = argo_signal_handler; sigaction(SIGNAL, &s, &application_handler); @@ -95,11 +95,11 @@ class signal_handler { * @details The signal handler will only be called for signals * that are not consumed by ArgoDSM internally */ - static sig_handler install_application_handler(sig_handler* h) { + static sig_internal::sig_handler install_application_handler(sig_internal::sig_handler* h) { if(argo_handler == nullptr) { - throw std::runtime_error(msg_argo_unitialized); + throw std::runtime_error(sig_internal::msg_argo_unitialized); } - sig_handler r = *h; + sig_internal::sig_handler r = *h; std::swap(r, application_handler); return r; } @@ -133,8 +133,8 @@ class signal_handler { } }; -template handler_ftype signal_handler::argo_handler = nullptr; -template sig_handler signal_handler::application_handler; +template sig_internal::handler_ftype signal_handler::argo_handler = nullptr; +template sig_internal::sig_handler signal_handler::application_handler; } // namespace signal } // namespace argo diff --git a/tests/allocators.cpp b/tests/allocators.cpp index 8b985a52..6249eed4 100644 --- a/tests/allocators.cpp +++ b/tests/allocators.cpp @@ -369,24 +369,25 @@ TEST_F(AllocatorTest, PerfectForwardingCollectiveNew) { * @brief Test the "parser" for the allocation parameters */ TEST_F(AllocatorTest, AllocationParametersParsing) { - using namespace argo; - using namespace argo::_internal; - - ASSERT_TRUE((alloc_param_in::value)); - ASSERT_TRUE((alloc_param_in::value)); - ASSERT_TRUE((alloc_param_in::value)); - ASSERT_TRUE((alloc_param_in::value)); - ASSERT_TRUE((alloc_param_in::value)); - ASSERT_TRUE((alloc_param_in::value)); - - using all_yes = alloc_params; + using alloc = argo::allocation; + + using argo::_internal::alloc_param_in; + ASSERT_TRUE((alloc_param_in::value)); + ASSERT_TRUE((alloc_param_in::value)); + ASSERT_TRUE((alloc_param_in::value)); + ASSERT_TRUE((alloc_param_in::value)); + ASSERT_TRUE((alloc_param_in::value)); + ASSERT_TRUE((alloc_param_in::value)); + + using argo::_internal::alloc_params; + using all_yes = alloc_params; ASSERT_TRUE(all_yes::initialize); ASSERT_TRUE(all_yes::deinitialize); ASSERT_TRUE(all_yes::synchronize); @@ -394,8 +395,8 @@ TEST_F(AllocatorTest, AllocationParametersParsing) { ASSERT_FALSE(all_yes::no_deinitialize); ASSERT_FALSE(all_yes::no_synchronize); - using all_no = alloc_params; + using all_no = alloc_params; ASSERT_FALSE(all_no::initialize); ASSERT_FALSE(all_no::deinitialize); ASSERT_FALSE(all_no::synchronize); @@ -403,7 +404,7 @@ TEST_F(AllocatorTest, AllocationParametersParsing) { ASSERT_TRUE(all_no::no_deinitialize); ASSERT_TRUE(all_no::no_synchronize); - using just_one = alloc_params; + using just_one = alloc_params; ASSERT_FALSE(just_one::initialize); ASSERT_FALSE(just_one::deinitialize); ASSERT_TRUE(just_one::synchronize); diff --git a/tests/stlallocation.cpp b/tests/stlallocation.cpp index a170f1b6..c4da5822 100644 --- a/tests/stlallocation.cpp +++ b/tests/stlallocation.cpp @@ -36,18 +36,15 @@ class cppTest : public testing::Test { * @brief Unittest that checks that an STL list can be allocated globally and populated */ TEST_F(cppTest, simpleList) { - using namespace std; - using namespace argo; - using namespace argo::allocators; - using my_list = std::list>; + using my_list = std::list>; - my_list* l = conew_(); + my_list* l = argo::conew_(); for(unsigned int i = 0; i < argo_number_of_nodes(); i++) { if(argo_node_id() == i) { ASSERT_NO_THROW(l->push_back(i)); } - barrier(); + argo::barrier(); } int id = 0; @@ -61,18 +58,15 @@ TEST_F(cppTest, simpleList) { * @brief Unittest that checks that an STL vector can be allocated globally and populated */ TEST_F(cppTest, simpleVector) { - using namespace std; - using namespace argo; - using namespace argo::allocators; - using my_vector = std::vector>; + using my_vector = std::vector>; - my_vector* v = conew_(); + my_vector* v = argo::conew_(); for(unsigned int i = 0; i < argo_number_of_nodes(); i++) { if(argo_node_id() == i) { ASSERT_NO_THROW(v->push_back(i)); } - barrier(); + argo::barrier(); } int id = 0;