Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Resolve MSVC Warnings, main branch (2021.09.13.) #101

Merged
merged 3 commits into from
Sep 14, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 11 additions & 0 deletions cmake/vecmem-compiler-options-cpp.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -30,4 +30,15 @@ if( ( "${CMAKE_CXX_COMPILER_ID}" MATCHES "GNU" ) OR
# More rigorous tests for the Debug builds.
vecmem_add_flag( CMAKE_CXX_FLAGS_DEBUG "-Werror" )
vecmem_add_flag( CMAKE_CXX_FLAGS_DEBUG "-pedantic" )

elseif( "${CMAKE_CXX_COMPILER_ID}" MATCHES "MSVC" )

# Basic flags for all build modes.
foreach( mode RELEASE RELWITHDEBINFO MINSIZEREL DEBUG )
vecmem_add_flag( CMAKE_CXX_FLAGS_${mode} "/W4" )
endforeach()

# More rigorous tests for the Debug builds.
vecmem_add_flag( CMAKE_CXX_FLAGS_DEBUG "/WX" )

endif()
24 changes: 16 additions & 8 deletions core/include/vecmem/containers/impl/jagged_vector.ipp
Original file line number Diff line number Diff line change
Expand Up @@ -24,12 +24,14 @@ data::jagged_vector_data<TYPE> get_data(jagged_vector<TYPE>& vec,
size,
(resource != nullptr ? *resource : *(vec.get_allocator().resource())));

// Helper local type definition.
// Helper local type definition(s).
typedef typename data::jagged_vector_data<TYPE>::value_type value_type;
typedef typename value_type::size_type size_type;

// Fill the result object with information.
for (std::size_t i = 0; i < size; ++i) {
result.m_ptr[i] = value_type(vec[i].size(), vec[i].data());
result.m_ptr[i] =
value_type(static_cast<size_type>(vec[i].size()), vec[i].data());
}

// Return the created object.
Expand All @@ -50,12 +52,14 @@ data::jagged_vector_data<TYPE> get_data(
// Construct the object to be returned.
data::jagged_vector_data<TYPE> result(size, *resource);

// Helper local type definition.
// Helper local type definition(s).
typedef typename data::jagged_vector_data<TYPE>::value_type value_type;
typedef typename value_type::size_type size_type;

// Fill the result object with information.
for (std::size_t i = 0; i < size; ++i) {
result.m_ptr[i] = value_type(vec[i].size(), vec[i].data());
result.m_ptr[i] =
value_type(static_cast<size_type>(vec[i].size()), vec[i].data());
}

// Return the created object.
Expand All @@ -74,13 +78,15 @@ data::jagged_vector_data<const TYPE> get_data(const jagged_vector<TYPE>& vec,
size,
(resource != nullptr ? *resource : *(vec.get_allocator().resource())));

// Helper local type definition.
// Helper local type definition(s).
typedef
typename data::jagged_vector_data<const TYPE>::value_type value_type;
typedef typename value_type::size_type size_type;

// Fill the result object with information.
for (std::size_t i = 0; i < size; ++i) {
result.m_ptr[i] = value_type(vec[i].size(), vec[i].data());
result.m_ptr[i] =
value_type(static_cast<size_type>(vec[i].size()), vec[i].data());
}

// Return the created object.
Expand All @@ -101,13 +107,15 @@ data::jagged_vector_data<const TYPE> get_data(
// Construct the object to be returned.
data::jagged_vector_data<const TYPE> result(size, *resource);

// Helper local type definition.
// Helper local type definition(s).
typedef
typename data::jagged_vector_data<const TYPE>::value_type value_type;
typedef typename value_type::size_type size_type;

// Fill the result object with information.
for (std::size_t i = 0; i < size; ++i) {
result.m_ptr[i] = value_type(vec[i].size(), vec[i].data());
result.m_ptr[i] =
value_type(static_cast<size_type>(vec[i].size()), vec[i].data());
}

// Return the created object.
Expand Down
21 changes: 12 additions & 9 deletions core/include/vecmem/containers/impl/jagged_vector_buffer.ipp
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,9 @@ allocate_jagged_buffer_inner_memory(const std::vector<std::size_t>& sizes,
bool isResizable) {

typename vecmem::data::jagged_vector_buffer<TYPE>::size_type byteSize =
std::accumulate(sizes.begin(), sizes.end(), 0) * sizeof(TYPE);
std::accumulate(sizes.begin(), sizes.end(),
static_cast<std::size_t>(0)) *
sizeof(TYPE);
if (isResizable) {
byteSize +=
sizes.size() * sizeof(typename vecmem::data::jagged_vector_buffer<
Expand Down Expand Up @@ -103,8 +105,9 @@ jagged_vector_buffer<TYPE>::jagged_vector_buffer(
// Set up the host accessible memory array.
std::ptrdiff_t ptrdiff = 0;
for (std::size_t i = 0; i < sizes.size(); ++i) {
new (host_ptr() + i) value_type(
sizes[i], reinterpret_cast<TYPE*>(m_inner_memory.get() + ptrdiff));
new (host_ptr() + i)
value_type(static_cast<typename value_type::size_type>(sizes[i]),
reinterpret_cast<TYPE*>(m_inner_memory.get() + ptrdiff));
ptrdiff += sizes[i] * sizeof(TYPE);
}
}
Expand Down Expand Up @@ -136,12 +139,12 @@ jagged_vector_buffer<TYPE>::jagged_vector_buffer(
std::ptrdiff_t ptrdiff =
(capacities.size() * sizeof(typename value_type::size_type));
for (std::size_t i = 0; i < capacities.size(); ++i) {
new (host_ptr() + i)
value_type(capacities[i],
reinterpret_cast<typename value_type::size_pointer>(
m_inner_memory.get() +
i * sizeof(typename value_type::size_type)),
reinterpret_cast<TYPE*>(m_inner_memory.get() + ptrdiff));
new (host_ptr() + i) value_type(
static_cast<typename value_type::size_type>(capacities[i]),
reinterpret_cast<typename value_type::size_pointer>(
m_inner_memory.get() +
i * sizeof(typename value_type::size_type)),
reinterpret_cast<TYPE*>(m_inner_memory.get() + ptrdiff));
ptrdiff += capacities[i] * sizeof(TYPE);
}
}
Expand Down
11 changes: 6 additions & 5 deletions tests/core/test_core_array.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -47,32 +47,33 @@ class core_array_test
void test_array(vecmem::array<T, N>& a) {

// Fill the array with some simple values.
for (std::size_t i = 0; i < a.size(); ++i) {
for (int i = 0; i < static_cast<int>(a.size()); ++i) {
a.at(i) = T(i);
}

// Check the contents using iterator based loops.
{
auto itr = a.begin();
for (std::size_t i = 0; itr != a.end(); ++itr, ++i) {
for (int i = 0; itr != a.end(); ++itr, ++i) {
EXPECT_EQ(*itr, T(i));
}
auto ritr = a.rbegin();
for (std::size_t i = a.size() - 1; ritr != a.rend(); ++ritr, --i) {
for (int i = static_cast<int>(a.size() - 1); ritr != a.rend();
++ritr, --i) {
EXPECT_EQ(*ritr, T(i));
}
}

// Check its contents using a range based loop.
{
std::size_t i = 0;
int i = 0;
for (T value : a) {
EXPECT_EQ(value, T(i++));
}
}

// Fill the array with a constant value.
static constexpr std::size_t VALUE = 123;
static constexpr int VALUE = 123;
a.fill(T(VALUE));
for (T value : a) {
EXPECT_EQ(value, T(VALUE));
Expand Down
52 changes: 28 additions & 24 deletions tests/core/test_core_containers.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -40,15 +40,17 @@ TEST_F(core_container_test, const_device_vector) {

const vecmem::const_device_vector<int> test_vector(
vecmem::get_data(m_reference_vector));
EXPECT_TRUE(test_vector.size() == m_reference_vector.size());
EXPECT_TRUE(test_vector.empty() == m_reference_vector.empty());
EXPECT_EQ(test_vector.size(), m_reference_vector.size());
EXPECT_EQ(test_vector.empty(), m_reference_vector.empty());
EXPECT_TRUE(std::equal(m_reference_vector.begin(), m_reference_vector.end(),
test_vector.begin()));
EXPECT_TRUE(std::accumulate(test_vector.begin(), test_vector.end(), 0) ==
std::accumulate(test_vector.rbegin(), test_vector.rend(), 0));
EXPECT_EQ(std::accumulate(test_vector.begin(), test_vector.end(), 0),
std::accumulate(test_vector.rbegin(), test_vector.rend(), 0));
for (std::size_t i = 0; i < m_reference_vector.size(); ++i) {
EXPECT_TRUE(test_vector.at(i) == m_reference_vector.at(i));
EXPECT_TRUE(test_vector[i] == m_reference_vector[i]);
const vecmem::const_device_vector<int>::size_type ii =
static_cast<vecmem::const_device_vector<int>::size_type>(i);
EXPECT_EQ(test_vector.at(ii), m_reference_vector.at(i));
EXPECT_EQ(test_vector[ii], m_reference_vector[i]);
}
}

Expand All @@ -57,15 +59,17 @@ TEST_F(core_container_test, device_vector) {

const vecmem::device_vector<int> test_vector(
vecmem::get_data(m_reference_vector));
EXPECT_TRUE(test_vector.size() == m_reference_vector.size());
EXPECT_TRUE(test_vector.empty() == m_reference_vector.empty());
EXPECT_EQ(test_vector.size(), m_reference_vector.size());
EXPECT_EQ(test_vector.empty(), m_reference_vector.empty());
EXPECT_TRUE(std::equal(m_reference_vector.begin(), m_reference_vector.end(),
test_vector.begin()));
EXPECT_TRUE(std::accumulate(test_vector.begin(), test_vector.end(), 0) ==
std::accumulate(test_vector.rbegin(), test_vector.rend(), 0));
EXPECT_EQ(std::accumulate(test_vector.begin(), test_vector.end(), 0),
std::accumulate(test_vector.rbegin(), test_vector.rend(), 0));
for (std::size_t i = 0; i < m_reference_vector.size(); ++i) {
EXPECT_TRUE(test_vector.at(i) == m_reference_vector.at(i));
EXPECT_TRUE(test_vector[i] == m_reference_vector[i]);
const vecmem::const_device_vector<int>::size_type ii =
static_cast<vecmem::const_device_vector<int>::size_type>(i);
EXPECT_EQ(test_vector.at(ii), m_reference_vector.at(i));
EXPECT_EQ(test_vector[ii], m_reference_vector[i]);
}
}

Expand Down Expand Up @@ -95,15 +99,15 @@ TEST_F(core_container_test, const_device_array) {

const vecmem::const_device_array<int, 9> test_array(
vecmem::get_data(m_reference_vector));
EXPECT_TRUE(test_array.size() == m_reference_vector.size());
EXPECT_TRUE(test_array.empty() == m_reference_vector.empty());
EXPECT_EQ(test_array.size(), m_reference_vector.size());
EXPECT_EQ(test_array.empty(), m_reference_vector.empty());
EXPECT_TRUE(std::equal(m_reference_vector.begin(), m_reference_vector.end(),
test_array.begin()));
EXPECT_TRUE(std::accumulate(test_array.begin(), test_array.end(), 0) ==
std::accumulate(test_array.rbegin(), test_array.rend(), 0));
EXPECT_EQ(std::accumulate(test_array.begin(), test_array.end(), 0),
std::accumulate(test_array.rbegin(), test_array.rend(), 0));
for (std::size_t i = 0; i < m_reference_vector.size(); ++i) {
EXPECT_TRUE(test_array.at(i) == m_reference_vector.at(i));
EXPECT_TRUE(test_array[i] == m_reference_vector[i]);
EXPECT_EQ(test_array.at(i), m_reference_vector.at(i));
EXPECT_EQ(test_array[i], m_reference_vector[i]);
}
}

Expand All @@ -112,15 +116,15 @@ TEST_F(core_container_test, device_array) {

const vecmem::device_array<int, 9> test_array(
vecmem::get_data(m_reference_vector));
EXPECT_TRUE(test_array.size() == m_reference_vector.size());
EXPECT_TRUE(test_array.empty() == m_reference_vector.empty());
EXPECT_EQ(test_array.size(), m_reference_vector.size());
EXPECT_EQ(test_array.empty(), m_reference_vector.empty());
EXPECT_TRUE(std::equal(m_reference_vector.begin(), m_reference_vector.end(),
test_array.begin()));
EXPECT_TRUE(std::accumulate(test_array.begin(), test_array.end(), 0) ==
std::accumulate(test_array.rbegin(), test_array.rend(), 0));
EXPECT_EQ(std::accumulate(test_array.begin(), test_array.end(), 0),
std::accumulate(test_array.rbegin(), test_array.rend(), 0));
for (std::size_t i = 0; i < m_reference_vector.size(); ++i) {
EXPECT_TRUE(test_array.at(i) == m_reference_vector.at(i));
EXPECT_TRUE(test_array[i] == m_reference_vector[i]);
EXPECT_EQ(test_array.at(i), m_reference_vector.at(i));
EXPECT_EQ(test_array[i], m_reference_vector[i]);
}
}

Expand Down
24 changes: 16 additions & 8 deletions tests/core/test_core_contiguous_memory_resource.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,12 @@ class core_contiguous_memory_resource_test : public testing::Test {
/// Test the dumb allocation of a bunch of vectors
TEST_F(core_contiguous_memory_resource_test, allocations) {

// Skip this test with MSVC, in debug builds. As MSVC pads vectors in a way
// that makes this test fail.
#if defined(_MSC_VER) && defined(_DEBUG)
GTEST_SKIP();
#else

/// Fixed size for the test vectors
static constexpr std::size_t VECTOR_SIZE = 100;

Expand All @@ -38,18 +44,20 @@ TEST_F(core_contiguous_memory_resource_test, allocations) {
vecmem::vector<int> vec1(VECTOR_SIZE, &m_resource);

vecmem::vector<char> vec2(VECTOR_SIZE, &m_resource);
EXPECT_TRUE(static_cast<void*>(&*(vec2.begin())) ==
static_cast<void*>(&*(vec1.end())));
EXPECT_EQ(static_cast<void*>(&*(vec2.begin())),
static_cast<void*>(&*(vec1.end())));

vecmem::vector<double> vec3(VECTOR_SIZE, &m_resource);
EXPECT_TRUE(static_cast<void*>(&*(vec3.begin())) ==
static_cast<void*>(&*(vec2.end())));
EXPECT_EQ(static_cast<void*>(&*(vec3.begin())),
static_cast<void*>(&*(vec2.end())));

vecmem::vector<float> vec4(VECTOR_SIZE, &m_resource);
EXPECT_TRUE(static_cast<void*>(&*(vec4.begin())) ==
static_cast<void*>(&*(vec3.end())));
EXPECT_EQ(static_cast<void*>(&*(vec4.begin())),
static_cast<void*>(&*(vec3.end())));

vecmem::vector<int> vec5(VECTOR_SIZE, &m_resource);
EXPECT_TRUE(static_cast<void*>(&*(vec5.begin())) ==
static_cast<void*>(&*(vec4.end())));
EXPECT_EQ(static_cast<void*>(&*(vec5.begin())),
static_cast<void*>(&*(vec4.end())));

#endif // MSVC debug build...
}
36 changes: 21 additions & 15 deletions tests/core/test_core_device_containers.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -128,10 +128,16 @@ TEST_F(core_device_container_test, resizable_vector_buffer) {
// Create an input vector in regular host memory.
std::vector<int> host_vector{1, 2, 3, 4, 5, 6, 7, 8, 9, 10};

// Helper local type definitions.
typedef
typename vecmem::data::vector_buffer<int>::size_type buffer_size_type;
typedef typename vecmem::device_vector<int>::size_type vector_size_type;

// Create a resizable buffer from that data.
static constexpr std::size_t BUFFER_SIZE = 100;
static constexpr buffer_size_type BUFFER_SIZE = 100;
vecmem::data::vector_buffer<int> resizable_buffer(
BUFFER_SIZE, host_vector.size(), m_resource);
BUFFER_SIZE, static_cast<buffer_size_type>(host_vector.size()),
m_resource);
m_copy.setup(resizable_buffer);
EXPECT_EQ(resizable_buffer.capacity(), BUFFER_SIZE);
m_copy(vecmem::get_data(host_vector), resizable_buffer);
Expand All @@ -151,45 +157,45 @@ TEST_F(core_device_container_test, resizable_vector_buffer) {
// Modify the device vector in different ways, and check that it would work
// as expected.
device_vector.clear();
EXPECT_EQ(device_vector.size(), 0);
EXPECT_EQ(device_vector.size(), static_cast<vector_size_type>(0));

device_vector.push_back(10);
EXPECT_EQ(device_vector.size(), 1);
EXPECT_EQ(device_vector.size(), static_cast<vector_size_type>(1));
EXPECT_EQ(device_vector.at(0), 10);

device_vector.emplace_back(15);
EXPECT_EQ(device_vector.size(), 2);
EXPECT_EQ(device_vector.size(), static_cast<vector_size_type>(2));
EXPECT_EQ(device_vector.back(), 15);

device_vector.assign(20, 123);
EXPECT_EQ(device_vector.size(), 20);
EXPECT_EQ(device_vector.size(), static_cast<vector_size_type>(20));
for (int value : device_vector) {
EXPECT_EQ(value, 123);
}

device_vector.resize(40, 234);
EXPECT_EQ(device_vector.size(), 40);
for (std::size_t i = 0; i < 20; ++i) {
EXPECT_EQ(device_vector.size(), static_cast<vector_size_type>(40));
for (vector_size_type i = 0; i < 20; ++i) {
EXPECT_EQ(device_vector[i], 123);
}
for (std::size_t i = 20; i < 40; ++i) {
for (vector_size_type i = 20; i < 40; ++i) {
EXPECT_EQ(device_vector[i], 234);
}
device_vector.resize(25);
EXPECT_EQ(device_vector.size(), 25);
for (std::size_t i = 0; i < 20; ++i) {
EXPECT_EQ(device_vector.size(), static_cast<vector_size_type>(25));
for (vector_size_type i = 0; i < 20; ++i) {
EXPECT_EQ(device_vector[i], 123);
}
for (std::size_t i = 20; i < 25; ++i) {
for (vector_size_type i = 20; i < 25; ++i) {
EXPECT_EQ(device_vector[i], 234);
}

device_vector.pop_back();
EXPECT_EQ(device_vector.size(), 24);
for (std::size_t i = 0; i < 20; ++i) {
EXPECT_EQ(device_vector.size(), static_cast<vector_size_type>(24));
for (vector_size_type i = 0; i < 20; ++i) {
EXPECT_EQ(device_vector[i], 123);
}
for (std::size_t i = 20; i < 24; ++i) {
for (vector_size_type i = 20; i < 24; ++i) {
EXPECT_EQ(device_vector[i], 234);
}

Expand Down
Loading