Skip to content

Commit

Permalink
Eliminate using namespace commands (#130)
Browse files Browse the repository at this point in the history
* 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
  • Loading branch information
kostis authored Feb 7, 2024
1 parent 3c8b6c2 commit fcd7184
Show file tree
Hide file tree
Showing 10 changed files with 66 additions and 79 deletions.
2 changes: 1 addition & 1 deletion .github/workflows/cpplint.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
17 changes: 6 additions & 11 deletions src/allocators/allocators.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@

#include "allocators.hpp"

namespace mem = argo::mempools;
namespace alloc = argo::allocators;

/* default allocators */
Expand All @@ -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<void*>(default_collective_allocator.allocate(size));
return static_cast<void*>(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<atype*>(ptr));
alloc::default_collective_allocator.free(static_cast<atype*>(ptr));
}

extern "C"
void* dynamic_alloc(size_t size) {
using namespace argo::allocators;
return static_cast<void*>(default_dynamic_allocator.allocate(size));
return static_cast<void*>(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<atype*>(ptr));
alloc::default_dynamic_allocator.free(static_cast<atype*>(ptr));
}
8 changes: 4 additions & 4 deletions src/allocators/collective_allocator.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -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<void> gptr(ptr, false, false);
// The home node of ptr handles initialization
if (initialize && argo::backend::node_id() == gptr.node()) {
Expand Down Expand Up @@ -165,7 +165,7 @@ void codelete_(T* ptr) {
synchronize = false;
}

using namespace data_distribution;
using data_distribution::global_ptr;
global_ptr<T> gptr(ptr, false, false);
// The home node of ptr handles deinitialization
if (deinitialize && argo::backend::node_id() == gptr.node()) {
Expand Down Expand Up @@ -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<void> gptr(ptr, false, false);
// The home node of ptr handles initialization
if (initialize && argo::backend::node_id() == gptr.node()) {
Expand Down Expand Up @@ -269,7 +269,7 @@ void codelete_array(T* ptr) {
synchronize = false;
}

using namespace data_distribution;
using data_distribution::global_ptr;
global_ptr<T> gptr(ptr, false, false);
// The home node of ptr handles deinitialization
if (deinitialize && argo::backend::node_id() == gptr.node()) {
Expand Down
15 changes: 8 additions & 7 deletions src/argo.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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<global_allocator, NODE_ZERO_ONLY>(&default_global_allocator);
dynamic_prepool = dynamic_memory_pool<global_allocator, ALWAYS>(&default_global_allocator);
default_global_allocator = global_allocator<char>();
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<alloc::global_allocator, mem::NODE_ZERO_ONLY>(&default_global_allocator);
dynamic_prepool = mem::dynamic_memory_pool<alloc::global_allocator, mem::ALWAYS>(&default_global_allocator);
default_global_allocator = alloc::global_allocator<char>();
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);
Expand Down
3 changes: 0 additions & 3 deletions src/backend/mpi/mpi.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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:
Expand Down
8 changes: 3 additions & 5 deletions src/backend/singlenode/singlenode.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -85,12 +84,12 @@ void init(std::size_t argo_size, std::size_t cache_size) {
(void)(cache_size);
memory = static_cast<char*>(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<SIGSEGV>::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);
Expand Down Expand Up @@ -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);
Expand Down
5 changes: 3 additions & 2 deletions src/mempools/global_mempool.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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<char> gptr(&memory[max_size]);
if(backend::node_id() == gptr.node()) {
*offset = static_cast<std::ptrdiff_t>(0);
Expand All @@ -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<char> gptr(&memory[max_size]);
if(backend::node_id() == gptr.node()) {
*offset = static_cast<std::ptrdiff_t>(0);
Expand Down
26 changes: 13 additions & 13 deletions src/signal/signal.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -40,18 +40,18 @@ 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 */
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
*/
Expand All @@ -69,9 +69,9 @@ template<int SIGNAL>
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:
/**
Expand All @@ -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);
Expand All @@ -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;
}
Expand Down Expand Up @@ -133,8 +133,8 @@ class signal_handler {
}
};

template<int S> handler_ftype signal_handler<S>::argo_handler = nullptr;
template<int S> sig_handler signal_handler<S>::application_handler;
template<int S> sig_internal::handler_ftype signal_handler<S>::argo_handler = nullptr;
template<int S> sig_internal::sig_handler signal_handler<S>::application_handler;

} // namespace signal
} // namespace argo
Expand Down
43 changes: 22 additions & 21 deletions tests/allocators.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -369,41 +369,42 @@ 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<allocation::initialize,
allocation::initialize>::value));
ASSERT_TRUE((alloc_param_in<allocation::no_initialize,
allocation::no_initialize>::value));
ASSERT_TRUE((alloc_param_in<allocation::deinitialize,
allocation::deinitialize>::value));
ASSERT_TRUE((alloc_param_in<allocation::no_deinitialize,
allocation::no_deinitialize>::value));
ASSERT_TRUE((alloc_param_in<allocation::synchronize,
allocation::synchronize>::value));
ASSERT_TRUE((alloc_param_in<allocation::no_synchronize,
allocation::no_synchronize>::value));

using all_yes = alloc_params<allocation::initialize,
allocation::deinitialize, allocation::synchronize>;
using alloc = argo::allocation;

using argo::_internal::alloc_param_in;
ASSERT_TRUE((alloc_param_in<alloc::initialize,
alloc::initialize>::value));
ASSERT_TRUE((alloc_param_in<alloc::no_initialize,
alloc::no_initialize>::value));
ASSERT_TRUE((alloc_param_in<alloc::deinitialize,
alloc::deinitialize>::value));
ASSERT_TRUE((alloc_param_in<alloc::no_deinitialize,
alloc::no_deinitialize>::value));
ASSERT_TRUE((alloc_param_in<alloc::synchronize,
alloc::synchronize>::value));
ASSERT_TRUE((alloc_param_in<alloc::no_synchronize,
alloc::no_synchronize>::value));

using argo::_internal::alloc_params;
using all_yes = alloc_params<alloc::initialize,
alloc::deinitialize, alloc::synchronize>;
ASSERT_TRUE(all_yes::initialize);
ASSERT_TRUE(all_yes::deinitialize);
ASSERT_TRUE(all_yes::synchronize);
ASSERT_FALSE(all_yes::no_initialize);
ASSERT_FALSE(all_yes::no_deinitialize);
ASSERT_FALSE(all_yes::no_synchronize);

using all_no = alloc_params<allocation::no_initialize,
allocation::no_deinitialize, allocation::no_synchronize>;
using all_no = alloc_params<alloc::no_initialize,
alloc::no_deinitialize, alloc::no_synchronize>;
ASSERT_FALSE(all_no::initialize);
ASSERT_FALSE(all_no::deinitialize);
ASSERT_FALSE(all_no::synchronize);
ASSERT_TRUE(all_no::no_initialize);
ASSERT_TRUE(all_no::no_deinitialize);
ASSERT_TRUE(all_no::no_synchronize);

using just_one = alloc_params<allocation::synchronize>;
using just_one = alloc_params<alloc::synchronize>;
ASSERT_FALSE(just_one::initialize);
ASSERT_FALSE(just_one::deinitialize);
ASSERT_TRUE(just_one::synchronize);
Expand Down
18 changes: 6 additions & 12 deletions tests/stlallocation.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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<int, dynamic_allocator<int>>;
using my_list = std::list<int, argo::allocators::dynamic_allocator<int>>;

my_list* l = conew_<my_list>();
my_list* l = argo::conew_<my_list>();

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;
Expand All @@ -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<int, dynamic_allocator<int>>;
using my_vector = std::vector<int, argo::allocators::dynamic_allocator<int>>;

my_vector* v = conew_<my_vector>();
my_vector* v = argo::conew_<my_vector>();

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;
Expand Down

0 comments on commit fcd7184

Please sign in to comment.