Skip to content

Commit

Permalink
Fix clang-tidy warnings in std::memmove usage of amc/memory.h for tri…
Browse files Browse the repository at this point in the history
…vially relocatable types
  • Loading branch information
sjanel committed Feb 15, 2025
1 parent 6ee692e commit feab06f
Show file tree
Hide file tree
Showing 12 changed files with 43 additions and 48 deletions.
3 changes: 2 additions & 1 deletion .github/workflows/clang-format-check.yml
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,8 @@ jobs:
steps:
- uses: actions/checkout@v4
- name: Run clang-format style check for C/C++/Protobuf programs.
uses: jidicula/clang-format-action@v4.11.0
uses: jidicula/clang-format-action@v4.14.0
with:
check-path: ${{ matrix.path }}
fallback-style: "Google"
clang-format-version: '19'
6 changes: 3 additions & 3 deletions .github/workflows/ubuntu.yml
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ jobs:
runs-on: ubuntu-latest
strategy:
matrix:
compiler: ["15", "17"]
compiler: ["17", "19"]
steps:
- uses: actions/checkout@v4
- name: Install compiler
Expand All @@ -68,7 +68,7 @@ jobs:
runs-on: ubuntu-latest
strategy:
matrix:
compiler: ["16"]
compiler: ["20"]
steps:
- uses: actions/checkout@v4
- name: Install compiler
Expand All @@ -88,7 +88,7 @@ jobs:
strategy:
matrix:
standard: [11, 14, 17, 20]
compiler: [g++-11]
compiler: [g++-13]
steps:
- uses: actions/checkout@v4
- name: Install compiler
Expand Down
20 changes: 2 additions & 18 deletions .github/workflows/windows.yml
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,8 @@ on:
pull_request:

jobs:
msvc2019:
runs-on: windows-2019
msvc2022:
runs-on: windows-2022
strategy:
matrix:
build_type: [Debug, Release]
Expand All @@ -18,27 +18,11 @@ jobs:
- uses: actions/checkout@v4
- name: cmake
run: cmake -S . -B build -A ${{ matrix.architecture }}
if: matrix.build_type == 'Release'
- name: cmake
run: cmake -S . -B build -A ${{ matrix.architecture }}
if: matrix.build_type == 'Debug'
- name: build
run: cmake --build build --config ${{ matrix.build_type }} --parallel 2
- name: test
run: cd build && ctest -j 2 -C ${{ matrix.build_type }} --output-on-failure

msvc2019_latest:
runs-on: windows-2019

steps:
- uses: actions/checkout@v4
- name: cmake
run: cmake -S . -B build -DCMAKE_CXX_FLAGS="/permissive- /std:c++latest /utf-8 /W4"
- name: build
run: cmake --build build --config Release --parallel 2
- name: test
run: cd build && ctest -j 2 -C Release --output-on-failure

clang-cl-11:
runs-on: windows-latest
strategy:
Expand Down
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -17,3 +17,4 @@
*.vscode

**/build*/
.cache
2 changes: 1 addition & 1 deletion CMakeLists.txt
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
cmake_minimum_required (VERSION 3.14)

project(amc
VERSION 2.6.0.0
VERSION 2.5.2
DESCRIPTION "Header base library of C++ containers"
LANGUAGES CXX
)
Expand Down
4 changes: 2 additions & 2 deletions benchmark/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,8 @@ if (NOT benchmark_FOUND)

FetchContent_Declare(
googlebenchmark
GIT_REPOSITORY https://github.com/google/benchmark.git
GIT_TAG v1.8.3
URL https://github.com/google/benchmark/archive/refs/tags/v1.9.1.tar.gz
URL_HASH SHA256=32131c08ee31eeff2c8968d7e874f3cb648034377dfc32a4c377fa8796d84981
)

FetchContent_MakeAvailable(googlebenchmark)
Expand Down
2 changes: 1 addition & 1 deletion benchmark/vectors_benchmark.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -167,7 +167,7 @@ void Growing(benchmark::State &state) {
using SizeType = typename VecType::size_type;
TypeStats::_stats = TypeStats();

const SizeType kMaxSize = 10000U;
const SizeType kMaxSize = 1000000U;
TypeStats::_stats.start();
for (auto _ : state) {
VecType v;
Expand Down
8 changes: 4 additions & 4 deletions include/amc/flatset.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -87,8 +87,8 @@ class FlatSet : private Compare {
value_type &value() { return _optV.value(); }
const value_type &value() const { return _optV.value(); }

void swap(node_type &o) noexcept(
std::is_nothrow_move_constructible<T>::value &&amc::is_nothrow_swappable<T>::value) {
void swap(node_type &o) noexcept(std::is_nothrow_move_constructible<T>::value &&
amc::is_nothrow_swappable<T>::value) {
_optV.swap(o._optV);
}

Expand Down Expand Up @@ -117,8 +117,8 @@ class FlatSet : private Compare {
value_compare value_comp() const { return *this; }
allocator_type get_allocator() const { return _sortedVector.get_allocator(); }

FlatSet() noexcept(std::is_nothrow_default_constructible<Compare>::value
&&std::is_nothrow_default_constructible<VecType>::value) = default;
FlatSet() noexcept(std::is_nothrow_default_constructible<Compare>::value &&
std::is_nothrow_default_constructible<VecType>::value) = default;

explicit FlatSet(const Compare &comp, const Alloc &alloc = Alloc()) : Compare(comp), _sortedVector(alloc) {}

Expand Down
17 changes: 10 additions & 7 deletions include/amc/memory.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -276,7 +276,8 @@ template <class InputIt, class OutputIt>
inline OutputIt uninitialized_copy_impl(InputIt first, InputIt last, OutputIt dest, MemMoveInALoop) {
for (; first != last; ++dest, void(), ++first) {
using ValueType = typename std::iterator_traits<InputIt>::value_type;
std::memcpy(std::addressof(*dest), std::addressof(*first), sizeof(ValueType));
::std::memcpy(static_cast<void *>(std::addressof(*dest)), static_cast<const void *>(std::addressof(*first)),
sizeof(ValueType));
}
return dest;
}
Expand All @@ -287,7 +288,7 @@ inline OutputIt uninitialized_copy_impl(InputIt first, InputIt last, OutputIt de
typename std::iterator_traits<InputIt>::difference_type count = last - first;
if (count > 0) {
// We can use memcpy here as for uninitialized_copy, we know that ranges do not overlap
std::memcpy(dest, first, count * sizeof(ValueType));
::std::memcpy(static_cast<void *>(dest), static_cast<const void *>(first), count * sizeof(ValueType));
}
return dest + count;
}
Expand All @@ -310,7 +311,8 @@ template <class InputIt, class Size, class OutputIt>
inline OutputIt uninitialized_copy_n_impl(InputIt first, Size count, OutputIt dest, MemMoveInALoop) {
for (; count > 0; ++dest, void(), ++first, --count) {
using ValueType = typename std::iterator_traits<InputIt>::value_type;
std::memcpy(std::addressof(*dest), std::addressof(*first), sizeof(ValueType));
::std::memcpy(static_cast<void *>(std::addressof(*dest)), static_cast<const void *>(std::addressof(*first)),
sizeof(ValueType));
}
return dest;
}
Expand All @@ -320,7 +322,7 @@ inline OutputIt uninitialized_copy_n_impl(InputIt first, Size count, OutputIt de
using ValueType = typename std::iterator_traits<InputIt>::value_type;
// We can use memcpy here as for uninitialized_copy, we know that ranges do not overlap
if (count > 0) {
std::memcpy(dest, first, count * sizeof(ValueType));
::std::memcpy(static_cast<void *>(dest), static_cast<const void *>(first), count * sizeof(ValueType));
}
return dest + count;
}
Expand Down Expand Up @@ -359,7 +361,8 @@ inline std::pair<InputIt, OutputIt> uninitialized_move_n_impl(InputIt first, Siz
MemMoveInALoop) {
for (; count > 0; ++dest, void(), ++first, --count) {
using ValueType = typename std::iterator_traits<InputIt>::value_type;
std::memcpy(std::addressof(*dest), std::addressof(*first), sizeof(ValueType));
::std::memcpy(static_cast<void *>(std::addressof(*dest)), static_cast<const void *>(std::addressof(*first)),
sizeof(ValueType));
}
return std::pair<InputIt, OutputIt>(first, dest);
}
Expand Down Expand Up @@ -423,7 +426,7 @@ inline T *relocate_at_impl(T *elem, T *dest, MemMove) {
AMC_PUSH_WARNING
AMC_DISABLE_WARNING("-Wclass-memaccess")
#endif
std::memmove(dest, elem, sizeof(T));
::std::memmove(static_cast<void *>(dest), static_cast<const void *>(elem), sizeof(T));
#if __GNUC__ >= 8
AMC_POP_WARNING
#endif
Expand Down Expand Up @@ -488,7 +491,7 @@ inline std::pair<InputIt, OutputIt> uninitialized_relocate_n_impl(InputIt first,
AMC_PUSH_WARNING
AMC_DISABLE_WARNING("-Wclass-memaccess")
#endif
std::memmove(dest, first, count * sizeof(ValueType));
::std::memmove(static_cast<void *>(dest), static_cast<const void *>(first), count * sizeof(ValueType));
#if __GNUC__ >= 8
AMC_POP_WARNING
#endif
Expand Down
23 changes: 14 additions & 9 deletions include/amc/smallset.hpp
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
#pragma once

#include <algorithm>
#include <functional>
#include <initializer_list>
#include <iterator>
Expand Down Expand Up @@ -183,15 +184,19 @@ class SmallSet {
node_type &operator=(const node_type &) = delete;
node_type &operator=(node_type &&o) noexcept(std::is_nothrow_move_assignable<value_type>::value) = default;

~node_type() = default;

bool empty() const noexcept { return !_optV.has_value(); }

explicit operator bool() const noexcept { return _optV.has_value(); }

allocator_type get_allocator() const { return static_cast<allocator_type>(*this); }

value_type &value() { return _optV.value(); }
const value_type &value() const { return _optV.value(); }

void swap(node_type &o) noexcept(
std::is_nothrow_move_constructible<T>::value &&amc::is_nothrow_swappable<T>::value) {
void swap(node_type &o) noexcept(std::is_nothrow_move_constructible<T>::value &&
amc::is_nothrow_swappable<T>::value) {
_optV.swap(o._optV);
}

Expand Down Expand Up @@ -220,8 +225,8 @@ class SmallSet {

allocator_type get_allocator() const { return _set.get_allocator(); }

SmallSet() noexcept(std::is_nothrow_default_constructible<VecType>::value
&&std::is_nothrow_default_constructible<SetType>::value) = default;
SmallSet() noexcept(std::is_nothrow_default_constructible<VecType>::value &&
std::is_nothrow_default_constructible<SetType>::value) = default;

explicit SmallSet(const Compare &comp, const Alloc &alloc = Alloc()) : _set(comp, alloc) {}

Expand Down Expand Up @@ -419,8 +424,8 @@ class SmallSet {
: iterator(_set.erase(first.toSetIt(), last.toSetIt()));
}

void swap(SmallSet &o) noexcept(noexcept(std::declval<VecType>().swap(std::declval<VecType &>())) &&noexcept(
std::declval<SetType>().swap(std::declval<SetType &>()))) {
void swap(SmallSet &o) noexcept(noexcept(std::declval<VecType>().swap(std::declval<VecType &>())) &&
noexcept(std::declval<SetType>().swap(std::declval<SetType &>()))) {
_vec.swap(o._vec);
_set.swap(o._set);
}
Expand Down Expand Up @@ -521,17 +526,17 @@ class SmallSet {
if (o.isSmall()) {
// We are both small, we need to sort both containers
auto oSortedPtrs = ComputeSortedPtrVec(o._vec);
return amc::lexicographical_compare_three_way(sortedPtrs.begin(), sortedPtrs.end(), oSortedPtrs.begin(),
return std::lexicographical_compare_three_way(sortedPtrs.begin(), sortedPtrs.end(), oSortedPtrs.begin(),
oSortedPtrs.end(), Comp());
}
// we are small: as we do not order elements in the small container, we need to sort them.
return amc::lexicographical_compare_three_way(sortedPtrs.begin(), sortedPtrs.end(), o._set.begin(), o._set.end(),
return std::lexicographical_compare_three_way(sortedPtrs.begin(), sortedPtrs.end(), o._set.begin(), o._set.end(),
Comp());
}
if (o.isSmall()) {
// other is small: as we do not order elements in the small container, we need to sort them.
auto oSortedPtrs = ComputeSortedPtrVec(o._vec);
return amc::lexicographical_compare_three_way(_set.begin(), _set.end(), oSortedPtrs.begin(), oSortedPtrs.end(),
return std::lexicographical_compare_three_way(_set.begin(), _set.end(), oSortedPtrs.begin(), oSortedPtrs.end(),
Comp());
}
return _set <=> o._set;
Expand Down
4 changes: 2 additions & 2 deletions test/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,8 @@ if (NOT GTest_FOUND)

FetchContent_Declare(
googletest
GIT_REPOSITORY https://github.com/google/googletest.git
GIT_TAG v1.14.0
URL https://github.com/google/googletest/archive/refs/tags/v1.16.0.tar.gz
URL_HASH SHA256=78c676fc63881529bf97bf9d45948d905a66833fbfa5318ea2cd7478cb98f399
)

# For Windows: Prevent overriding the parent project's compiler/linker settings
Expand Down
1 change: 1 addition & 0 deletions test/vectors_test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -482,6 +482,7 @@ TEST(VectorTest, TrickyPushBack) {
TEST(VectorTest, SizeTypeNoIntegerOverflowFixedCapacityVector) {
using IntVector = FixedCapacityVector<int, 255U>;
using ExceptionType = std::out_of_range;

static_assert(sizeof(IntVector::size_type) == 1U, "");
IntVector v(250);
constexpr int kTab[] = {1, 2, 3, 4, 5, 6};
Expand Down

0 comments on commit feab06f

Please sign in to comment.