Skip to content

Commit

Permalink
Fixed VS2019 /W4 warnings with and without C++17 language suport (issue
Browse files Browse the repository at this point in the history 
#211)
  • Loading branch information
@cameron314
cameron314 committed Jul 11, 2020
1 parent 7bb34c9 commit 9cfda6c
Show file tree
Hide file tree
Showing 6 changed files with 89 additions and 56 deletions.
4 changes: 2 additions & 2 deletions build/msvc16/fuzztests.vcxproj
View file
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,7 @@
<ClCompile>
<PrecompiledHeader>
</PrecompiledHeader>
<WarningLevel>Level3</WarningLevel>
<WarningLevel>Level4</WarningLevel>
<Optimization>Disabled</Optimization>
<PreprocessorDefinitions>_CRT_SECURE_NO_WARNINGS;WIN32;_DEBUG;_CONSOLE;_LIB;%(PreprocessorDefinitions)</PreprocessorDefinitions>
<DisableSpecificWarnings>4800</DisableSpecificWarnings>
Expand All @@ -128,7 +128,7 @@
</ItemDefinitionGroup>
<ItemDefinitionGroup Condition="'$(Configuration)|$(Platform)'=='Release|Win32'">
<ClCompile>
<WarningLevel>Level3</WarningLevel>
<WarningLevel>Level4</WarningLevel>
<PrecompiledHeader>
</PrecompiledHeader>
<Optimization>MaxSpeed</Optimization>
Expand Down
4 changes: 2 additions & 2 deletions build/msvc16/unittests.vcxproj
View file
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@
<ClCompile>
<PrecompiledHeader>
</PrecompiledHeader>
<WarningLevel>Level3</WarningLevel>
<WarningLevel>Level4</WarningLevel>
<Optimization>Disabled</Optimization>
<PreprocessorDefinitions>WIN32;_DEBUG;_CONSOLE;_LIB;%(PreprocessorDefinitions)</PreprocessorDefinitions>
<DisableSpecificWarnings>4146</DisableSpecificWarnings>
Expand All @@ -117,7 +117,7 @@
</ItemDefinitionGroup>
<ItemDefinitionGroup Condition="'$(Configuration)|$(Platform)'=='Release|Win32'">
<ClCompile>
<WarningLevel>Level3</WarningLevel>
<WarningLevel>Level4</WarningLevel>
<PrecompiledHeader>
</PrecompiledHeader>
<Optimization>MaxSpeed</Optimization>
Expand Down
79 changes: 51 additions & 28 deletions concurrentqueue.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,13 @@
#endif
#endif

#if defined(_MSC_VER) && (!defined(_HAS_CXX17) || !_HAS_CXX17)
// VS2019 with /W4 warns about constant conditional expressions but unless /std=c++17 or higher
// does not support `if constexpr`, so we have no choice but to simply disable the warning
#pragma warning(push)
#pragma warning(disable: 4127) // conditional expression is constant
#endif

#if defined(__APPLE__)
#include "TargetConditionals.h"
#endif
Expand Down Expand Up @@ -1890,7 +1897,7 @@ class ConcurrentQueue
++pr_blockIndexSlotsUsed;
}

if (!MOODYCAMEL_NOEXCEPT_CTOR(T, U, new ((T*)nullptr) T(std::forward<U>(element)))) {
MOODYCAMEL_CONSTEXPR_IF (!MOODYCAMEL_NOEXCEPT_CTOR(T, U, new ((T*)nullptr) T(std::forward<U>(element)))) {
// The constructor may throw. We want the element not to appear in the queue in
// that case (without corrupting the queue):
MOODYCAMEL_TRY {
Expand All @@ -1916,7 +1923,7 @@ class ConcurrentQueue
blockIndex.load(std::memory_order_relaxed)->front.store(pr_blockIndexFront, std::memory_order_release);
pr_blockIndexFront = (pr_blockIndexFront + 1) & (pr_blockIndexSize - 1);

if (!MOODYCAMEL_NOEXCEPT_CTOR(T, U, new ((T*)nullptr) T(std::forward<U>(element)))) {
MOODYCAMEL_CONSTEXPR_IF (!MOODYCAMEL_NOEXCEPT_CTOR(T, U, new ((T*)nullptr) T(std::forward<U>(element)))) {
this->tailIndex.store(newTailIndex, std::memory_order_release);
return true;
}
Expand Down Expand Up @@ -2132,7 +2139,7 @@ class ConcurrentQueue
block = block->next;
}

if (MOODYCAMEL_NOEXCEPT_CTOR(T, decltype(*itemFirst), new ((T*)nullptr) T(details::deref_noexcept(itemFirst)))) {
MOODYCAMEL_CONSTEXPR_IF (MOODYCAMEL_NOEXCEPT_CTOR(T, decltype(*itemFirst), new ((T*)nullptr) T(details::deref_noexcept(itemFirst)))) {
blockIndex.load(std::memory_order_relaxed)->front.store((pr_blockIndexFront - 1) & (pr_blockIndexSize - 1), std::memory_order_release);
}
}
Expand All @@ -2147,11 +2154,11 @@ class ConcurrentQueue
this->tailBlock = firstAllocatedBlock;
}
while (true) {
auto stopIndex = (currentTailIndex & ~static_cast<index_t>(BLOCK_SIZE - 1)) + static_cast<index_t>(BLOCK_SIZE);
index_t stopIndex = (currentTailIndex & ~static_cast<index_t>(BLOCK_SIZE - 1)) + static_cast<index_t>(BLOCK_SIZE);
if (details::circular_less_than<index_t>(newTailIndex, stopIndex)) {
stopIndex = newTailIndex;
}
if (MOODYCAMEL_NOEXCEPT_CTOR(T, decltype(*itemFirst), new ((T*)nullptr) T(details::deref_noexcept(itemFirst)))) {
MOODYCAMEL_CONSTEXPR_IF (MOODYCAMEL_NOEXCEPT_CTOR(T, decltype(*itemFirst), new ((T*)nullptr) T(details::deref_noexcept(itemFirst)))) {
while (currentTailIndex != stopIndex) {
new ((*this->tailBlock)[currentTailIndex++]) T(*itemFirst++);
}
Expand Down Expand Up @@ -2213,8 +2220,9 @@ class ConcurrentQueue
this->tailBlock = this->tailBlock->next;
}

if (!MOODYCAMEL_NOEXCEPT_CTOR(T, decltype(*itemFirst), new ((T*)nullptr) T(details::deref_noexcept(itemFirst))) && firstAllocatedBlock != nullptr) {
blockIndex.load(std::memory_order_relaxed)->front.store((pr_blockIndexFront - 1) & (pr_blockIndexSize - 1), std::memory_order_release);
MOODYCAMEL_CONSTEXPR_IF (!MOODYCAMEL_NOEXCEPT_CTOR(T, decltype(*itemFirst), new ((T*)nullptr) T(details::deref_noexcept(itemFirst)))) {
if (firstAllocatedBlock != nullptr)
blockIndex.load(std::memory_order_relaxed)->front.store((pr_blockIndexFront - 1) & (pr_blockIndexSize - 1), std::memory_order_release);
}

this->tailIndex.store(newTailIndex, std::memory_order_release);
Expand Down Expand Up @@ -2258,7 +2266,7 @@ class ConcurrentQueue
auto index = firstIndex;
do {
auto firstIndexInBlock = index;
auto endIndex = (index & ~static_cast<index_t>(BLOCK_SIZE - 1)) + static_cast<index_t>(BLOCK_SIZE);
index_t endIndex = (index & ~static_cast<index_t>(BLOCK_SIZE - 1)) + static_cast<index_t>(BLOCK_SIZE);
endIndex = details::circular_less_than<index_t>(firstIndex + static_cast<index_t>(actualCount), endIndex) ? firstIndex + static_cast<index_t>(actualCount) : endIndex;
auto block = localBlockIndex->entries[indexIndex].block;
if (MOODYCAMEL_NOEXCEPT_ASSIGN(T, T&&, details::deref_noexcept(itemFirst) = std::move((*(*block)[index])))) {
Expand Down Expand Up @@ -2492,8 +2500,8 @@ class ConcurrentQueue
newBlock->owner = this;
#endif
newBlock->ConcurrentQueue::Block::template reset_empty<implicit_context>();
if (!MOODYCAMEL_NOEXCEPT_CTOR(T, U, new ((T*)nullptr) T(std::forward<U>(element)))) {

MOODYCAMEL_CONSTEXPR_IF (!MOODYCAMEL_NOEXCEPT_CTOR(T, U, new ((T*)nullptr) T(std::forward<U>(element)))) {
// May throw, try to insert now before we publish the fact that we have this new block
MOODYCAMEL_TRY {
new ((*newBlock)[currentTailIndex]) T(std::forward<U>(element));
Expand All @@ -2511,7 +2519,7 @@ class ConcurrentQueue

this->tailBlock = newBlock;

if (!MOODYCAMEL_NOEXCEPT_CTOR(T, U, new ((T*)nullptr) T(std::forward<U>(element)))) {
MOODYCAMEL_CONSTEXPR_IF (!MOODYCAMEL_NOEXCEPT_CTOR(T, U, new ((T*)nullptr) T(std::forward<U>(element)))) {
this->tailIndex.store(newTailIndex, std::memory_order_release);
return true;
}
Expand Down Expand Up @@ -2595,6 +2603,10 @@ class ConcurrentQueue
return false;
}

#ifdef _MSC_VER
#pragma warning(push)
#pragma warning(disable: 4706) // assignment within conditional expression
#endif
template<AllocationMode allocMode, typename It>
bool enqueue_bulk(It itemFirst, size_t count)
{
Expand Down Expand Up @@ -2630,6 +2642,7 @@ class ConcurrentQueue
auto head = this->headIndex.load(std::memory_order_relaxed);
assert(!details::circular_less_than<index_t>(currentTailIndex, head));
bool full = !details::circular_less_than<index_t>(head, currentTailIndex + BLOCK_SIZE) || (MAX_SUBQUEUE_SIZE != details::const_numeric_max<size_t>::value && (MAX_SUBQUEUE_SIZE == 0 || MAX_SUBQUEUE_SIZE - BLOCK_SIZE < currentTailIndex - head));

if (full || !(indexInserted = insert_block_index_entry<allocMode>(idxEntry, currentTailIndex)) || (newBlock = this->parent->ConcurrentQueue::template requisition_block<allocMode>()) == nullptr) {
// Index allocation or block allocation failed; revert any other allocations
// and index insertions done so far for this operation
Expand Down Expand Up @@ -2680,11 +2693,11 @@ class ConcurrentQueue
this->tailBlock = firstAllocatedBlock;
}
while (true) {
auto stopIndex = (currentTailIndex & ~static_cast<index_t>(BLOCK_SIZE - 1)) + static_cast<index_t>(BLOCK_SIZE);
index_t stopIndex = (currentTailIndex & ~static_cast<index_t>(BLOCK_SIZE - 1)) + static_cast<index_t>(BLOCK_SIZE);
if (details::circular_less_than<index_t>(newTailIndex, stopIndex)) {
stopIndex = newTailIndex;
}
if (MOODYCAMEL_NOEXCEPT_CTOR(T, decltype(*itemFirst), new ((T*)nullptr) T(details::deref_noexcept(itemFirst)))) {
MOODYCAMEL_CONSTEXPR_IF (MOODYCAMEL_NOEXCEPT_CTOR(T, decltype(*itemFirst), new ((T*)nullptr) T(details::deref_noexcept(itemFirst)))) {
while (currentTailIndex != stopIndex) {
new ((*this->tailBlock)[currentTailIndex++]) T(*itemFirst++);
}
Expand Down Expand Up @@ -2744,6 +2757,9 @@ class ConcurrentQueue
this->tailIndex.store(newTailIndex, std::memory_order_release);
return true;
}
#ifdef _MSC_VER
#pragma warning(pop)
#endif

template<typename It>
size_t dequeue_bulk(It& itemFirst, size_t max)
Expand Down Expand Up @@ -2775,7 +2791,7 @@ class ConcurrentQueue
auto indexIndex = get_block_index_index_for_index(index, localBlockIndex);
do {
auto blockStartIndex = index;
auto endIndex = (index & ~static_cast<index_t>(BLOCK_SIZE - 1)) + static_cast<index_t>(BLOCK_SIZE);
index_t endIndex = (index & ~static_cast<index_t>(BLOCK_SIZE - 1)) + static_cast<index_t>(BLOCK_SIZE);
endIndex = details::circular_less_than<index_t>(firstIndex + static_cast<index_t>(actualCount), endIndex) ? firstIndex + static_cast<index_t>(actualCount) : endIndex;

auto entry = localBlockIndex->index[indexIndex];
Expand Down Expand Up @@ -2873,7 +2889,7 @@ class ConcurrentQueue
if (localBlockIndex == nullptr) {
return false; // this can happen if new_block_index failed in the constructor
}
auto newTail = (localBlockIndex->tail.load(std::memory_order_relaxed) + 1) & (localBlockIndex->capacity - 1);
size_t newTail = (localBlockIndex->tail.load(std::memory_order_relaxed) + 1) & (localBlockIndex->capacity - 1);
idxEntry = localBlockIndex->index[newTail];
if (idxEntry->key.load(std::memory_order_relaxed) == INVALID_BLOCK_BASE ||
idxEntry->value.load(std::memory_order_relaxed) == nullptr) {
Expand Down Expand Up @@ -3443,7 +3459,7 @@ class ConcurrentQueue
}

auto newHash = new (raw) ImplicitProducerHash;
newHash->capacity = newCapacity;
newHash->capacity = (size_t)newCapacity;
newHash->entries = reinterpret_cast<ImplicitProducerKVP*>(details::align_for<ImplicitProducerKVP>(raw + sizeof(ImplicitProducerHash)));
for (size_t i = 0; i != newCapacity; ++i) {
new (newHash->entries + i) ImplicitProducerKVP;
Expand Down Expand Up @@ -3557,23 +3573,26 @@ class ConcurrentQueue
template<typename TAlign>
static inline void* aligned_malloc(size_t size)
{
if (std::alignment_of<TAlign>::value <= std::alignment_of<details::max_align_t>::value)
MOODYCAMEL_CONSTEXPR_IF (std::alignment_of<TAlign>::value <= std::alignment_of<details::max_align_t>::value)
return (Traits::malloc)(size);
size_t alignment = std::alignment_of<TAlign>::value;
void* raw = (Traits::malloc)(size + alignment - 1 + sizeof(void*));
if (!raw)
return nullptr;
char* ptr = details::align_for<TAlign>(reinterpret_cast<char*>(raw) + sizeof(void*));
*(reinterpret_cast<void**>(ptr) - 1) = raw;
return ptr;
else {
size_t alignment = std::alignment_of<TAlign>::value;
void* raw = (Traits::malloc)(size + alignment - 1 + sizeof(void*));
if (!raw)
return nullptr;
char* ptr = details::align_for<TAlign>(reinterpret_cast<char*>(raw) + sizeof(void*));
*(reinterpret_cast<void**>(ptr) - 1) = raw;
return ptr;
}
}

template<typename TAlign>
static inline void aligned_free(void* ptr)
{
if (std::alignment_of<TAlign>::value <= std::alignment_of<details::max_align_t>::value)
MOODYCAMEL_CONSTEXPR_IF (std::alignment_of<TAlign>::value <= std::alignment_of<details::max_align_t>::value)
return (Traits::free)(ptr);
(Traits::free)(ptr ? *(reinterpret_cast<void**>(ptr) - 1) : nullptr);
else
(Traits::free)(ptr ? *(reinterpret_cast<void**>(ptr) - 1) : nullptr);
}

template<typename U>
Expand Down Expand Up @@ -3679,15 +3698,15 @@ ConsumerToken::ConsumerToken(ConcurrentQueue<T, Traits>& queue)
: itemsConsumedFromCurrent(0), currentProducer(nullptr), desiredProducer(nullptr)
{
initialOffset = queue.nextExplicitConsumerId.fetch_add(1, std::memory_order_release);
lastKnownGlobalOffset = -1;
lastKnownGlobalOffset = (std::uint32_t)-1;
}

template<typename T, typename Traits>
ConsumerToken::ConsumerToken(BlockingConcurrentQueue<T, Traits>& queue)
: itemsConsumedFromCurrent(0), currentProducer(nullptr), desiredProducer(nullptr)
{
initialOffset = reinterpret_cast<ConcurrentQueue<T, Traits>*>(&queue)->nextExplicitConsumerId.fetch_add(1, std::memory_order_release);
lastKnownGlobalOffset = -1;
lastKnownGlobalOffset = (std::uint32_t)-1;
}

template<typename T, typename Traits>
Expand All @@ -3714,6 +3733,10 @@ inline void swap(typename ConcurrentQueue<T, Traits>::ImplicitProducerKVP& a, ty

}

#if defined(_MSC_VER) && (!defined(_HAS_CXX17) || !_HAS_CXX17)
#pragma warning(pop)
#endif

#if defined(__GNUC__)
#pragma GCC diagnostic pop
#endif
6 changes: 3 additions & 3 deletions tests/corealgos.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -277,7 +277,7 @@ struct ThreadLocal
auto raw = static_cast<char*>(corealgos_allocator::malloc(sizeof(InnerHash) + std::alignment_of<KeyValuePair>::value - 1 + sizeof(KeyValuePair) * newCapacity));
if (raw == nullptr) {
// Allocation failed
currentHashCount.fetch_add(-1, std::memory_order_relaxed);
currentHashCount.fetch_add((uint32_t)-1, std::memory_order_relaxed);
resizeInProgress.clear(std::memory_order_relaxed);
return nullptr;
}
Expand Down Expand Up @@ -434,15 +434,15 @@ struct FreeList
assert((head->freeListRefs.load(std::memory_order_relaxed) & SHOULD_BE_ON_FREELIST) == 0);

// Decrease refcount twice, once for our ref, and once for the list's ref
head->freeListRefs.fetch_add(-2, std::memory_order_release);
head->freeListRefs.fetch_add(-2u, std::memory_order_release);
return head;
}

// OK, the head must have changed on us, but we still need to decrease the refcount we
// increased.
// Note that we don't need to release any memory effects, but we do need to ensure that the reference
// count decrement happens-after the CAS on the head.
refs = prevHead->freeListRefs.fetch_add(-1, std::memory_order_acq_rel);
refs = prevHead->freeListRefs.fetch_add(-1u, std::memory_order_acq_rel);
if (refs == SHOULD_BE_ON_FREELIST + 1) {
add_knowing_refcount_is_zero(prevHead);
}
Expand Down
12 changes: 6 additions & 6 deletions tests/fuzztests/fuzztests.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -484,7 +484,7 @@ bool run_test(uint64_t seed, int iterations, test_type& out_type, const char*& o
count = q.try_dequeue_bulk(bulkData.begin(), bulkData.size());
}
for (std::size_t k = 0; k != count; ++k) {
auto item = bulkData[k];
item = bulkData[k];
ASSERT_OR_FAIL_THREAD((item & 0xFFFFFF) >= 0 && (item & 0xFFFFFF) < (int)largestOpCount);
ASSERT_OR_FAIL_THREAD((item & 0xFFFFFF) > lastItems[item >> 24]);
lastItems[item >> 24] = item & 0xFFFFFF;
Expand Down Expand Up @@ -784,12 +784,12 @@ int main(int argc, char** argv)
}
}

int result = 0;
int exitCode = 0;
test_type test;
const char* failReason;
if (singleSeed) {
if (!run_test(seed, SINGLE_SEED_ITERATIONS, test, failReason)) {
result = 1;
exitCode = 1;
std::ofstream fout(LOG_FILE, std::ios::app);
fout << test_names[test] << " failed: " << failReason << std::endl;
std::printf(" %s failed: %s\n", test_names[test], failReason);
Expand Down Expand Up @@ -818,7 +818,7 @@ int main(int argc, char** argv)
std::signal(SIGSEGV, signal_handler);
std::signal(SIGABRT, signal_handler);

int result;
bool result;
try {
result = run_test(seed, 2, test, failReason);
}
Expand All @@ -839,7 +839,7 @@ int main(int argc, char** argv)
std::signal(SIGABRT, SIG_DFL);

if (!result) {
result = 1;
exitCode = 1;
std::ofstream fout(LOG_FILE, std::ios::app);
fout << "*** Failure detected!\n Seed: " << std::hex << seed << "\n Test: " << test_names[test] << "\n Reason: " << failReason << std::endl;
std::printf("*** Failure detected!\n Seed: %08x%08x\n Test: %s\n Reason: %s\n", (uint32_t)(seed >> 32), (uint32_t)(seed), test_names[test], failReason);
Expand All @@ -863,5 +863,5 @@ int main(int argc, char** argv)
}
}

return result;
return exitCode;
}
Loading

0 comments on commit 9cfda6c

Please sign in to comment.