From a64722937d7edc159c0ba66b21f87610cf52006a Mon Sep 17 00:00:00 2001 From: Alexander Karatarakis Date: Fri, 15 Nov 2024 19:11:09 -0800 Subject: [PATCH 01/15] [clang-tidy-18] Fix missing include misc-include-cleaner --- test/enum_utils_test.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/test/enum_utils_test.cpp b/test/enum_utils_test.cpp index 460b0f25..902ac734 100644 --- a/test/enum_utils_test.cpp +++ b/test/enum_utils_test.cpp @@ -8,6 +8,7 @@ #include #include +#include #include #include #include From 0124e0bd3b77ea5c619ad6a2c024c86133d1a6aa Mon Sep 17 00:00:00 2001 From: Alexander Karatarakis Date: Fri, 15 Nov 2024 19:42:01 -0800 Subject: [PATCH 02/15] Clear out clang-tidy-19 diagnostics in preparation for incremental cleanup --- .clang-tidy | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/.clang-tidy b/.clang-tidy index 1506231e..5bb1dfb5 100644 --- a/.clang-tidy +++ b/.clang-tidy @@ -74,6 +74,18 @@ Checks: > -bugprone-easily-swappable-parameters, -bugprone-switch-missing-default-case, -bugprone-unchecked-optional-access, + -performance-unnecessary-copy-initialization, + -readability-container-size-empty, + -readability-enum-initial-value, + -readability-identifier-naming, + -readability-math-missing-parentheses, + -misc-use-internal-linkage, + -modernize-use-designated-initializers, + -modernize-use-equals-default, + -modernize-use-ranges, + -bugprone-crtp-constructor-accessibility, + -bugprone-return-const-ref-from-parameter, + -bugprone-use-after-move, # Turn all the warnings from the checks above into errors. WarningsAsErrors: "*" From 7c37e5c4648e4077c0a47005f5cc9263e6c4d879 Mon Sep 17 00:00:00 2001 From: Alexander Karatarakis Date: Fri, 15 Nov 2024 19:46:25 -0800 Subject: [PATCH 03/15] [clang-tidy-19] Enable readability-enum-initial-value ``` error: inital values in enum 'CustomValuesTestEnum1' are not consistent, consider explicit initialization of all, none or only the first enumerator readability-enum-initial-value,-warnings-as-errors] ``` --- .clang-tidy | 1 - test/enum_utils_test.cpp | 2 +- 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/.clang-tidy b/.clang-tidy index 5bb1dfb5..711aa626 100644 --- a/.clang-tidy +++ b/.clang-tidy @@ -76,7 +76,6 @@ Checks: > -bugprone-unchecked-optional-access, -performance-unnecessary-copy-initialization, -readability-container-size-empty, - -readability-enum-initial-value, -readability-identifier-naming, -readability-math-missing-parentheses, -misc-use-internal-linkage, diff --git a/test/enum_utils_test.cpp b/test/enum_utils_test.cpp index 902ac734..2967dc8f 100644 --- a/test/enum_utils_test.cpp +++ b/test/enum_utils_test.cpp @@ -43,7 +43,7 @@ namespace fixed_containers::rich_enums enum class CustomValuesTestEnum1 { ONE = 7, - TWO, + TWO = 8, FOUR = 12, THREE = 10, }; From 62366a8ed74b4a89af235c77a83e95a15fa7e442 Mon Sep 17 00:00:00 2001 From: Alexander Karatarakis Date: Fri, 15 Nov 2024 19:53:55 -0800 Subject: [PATCH 04/15] [clang-tidy-19] Enable misc-use-internal-linkage ``` error: function 'print_map_state' can be made static or moved into an anonymous namespace to enforce internal linkage [misc-use-internal-linkage,-warnings-as-errors] ``` --- .clang-tidy | 1 - test/fixed_robinhood_hashtable_test.cpp | 4 ++-- test/recursive_reflection_test.cpp | 5 +++-- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/.clang-tidy b/.clang-tidy index 711aa626..9f315864 100644 --- a/.clang-tidy +++ b/.clang-tidy @@ -78,7 +78,6 @@ Checks: > -readability-container-size-empty, -readability-identifier-naming, -readability-math-missing-parentheses, - -misc-use-internal-linkage, -modernize-use-designated-initializers, -modernize-use-equals-default, -modernize-use-ranges, diff --git a/test/fixed_robinhood_hashtable_test.cpp b/test/fixed_robinhood_hashtable_test.cpp index 5f697cdc..c3870b5f 100644 --- a/test/fixed_robinhood_hashtable_test.cpp +++ b/test/fixed_robinhood_hashtable_test.cpp @@ -41,8 +41,6 @@ static_assert(TriviallyCopyAssignable); static_assert(TriviallyMoveAssignable); static_assert(StandardLayout); -} // namespace - template [[maybe_unused]] void print_map_state(const T& map) { @@ -66,6 +64,8 @@ template } } +} // namespace + TEST(BucketOperations, DistAndFingerprint) { static_assert(IsStructuralType); diff --git a/test/recursive_reflection_test.cpp b/test/recursive_reflection_test.cpp index ef4ea875..45a6bc09 100644 --- a/test/recursive_reflection_test.cpp +++ b/test/recursive_reflection_test.cpp @@ -21,7 +21,8 @@ namespace fixed_containers::recursive_reflection { - +namespace +{ template constexpr std::size_t path_count_of(S&& instance = {}) { @@ -46,7 +47,7 @@ constexpr auto extract_paths_of(S&& instance = {}) [&](const PathNameChain&, F&& /*field*/) {}); return paths; } - +} // namespace } // namespace fixed_containers::recursive_reflection namespace fixed_containers::recursive_reflection From bc77ae3f4fc3a6e9028775d0305e8ff4250fe8bc Mon Sep 17 00:00:00 2001 From: Alexander Karatarakis Date: Fri, 15 Nov 2024 20:05:58 -0800 Subject: [PATCH 05/15] [clang-tidy-19] Enable modernize-use-equals-default ``` error: use '= default' to define a trivial copy-assignment operator [modernize-use-equals-default,-warnings-as-errors] ``` --- .clang-tidy | 1 - .../fixed_robinhood_hashtable.hpp | 18 +----------------- test/instance_counter.hpp | 6 ++++++ 3 files changed, 7 insertions(+), 18 deletions(-) diff --git a/.clang-tidy b/.clang-tidy index 9f315864..1c0c118d 100644 --- a/.clang-tidy +++ b/.clang-tidy @@ -79,7 +79,6 @@ Checks: > -readability-identifier-naming, -readability-math-missing-parentheses, -modernize-use-designated-initializers, - -modernize-use-equals-default, -modernize-use-ranges, -bugprone-crtp-constructor-accessibility, -bugprone-return-const-ref-from-parameter, diff --git a/include/fixed_containers/fixed_robinhood_hashtable.hpp b/include/fixed_containers/fixed_robinhood_hashtable.hpp index c331956a..d06a18dc 100644 --- a/include/fixed_containers/fixed_robinhood_hashtable.hpp +++ b/include/fixed_containers/fixed_robinhood_hashtable.hpp @@ -385,23 +385,7 @@ class FixedRobinhoodHashtable = default; constexpr FixedRobinhoodHashtable(FixedRobinhoodHashtable&& other) = default; - - constexpr FixedRobinhoodHashtable& operator=(const FixedRobinhoodHashtable& other) - requires IsReference - { - IMPLEMENTATION_DETAIL_DO_NOT_USE_value_storage_ = - other.IMPLEMENTATION_DETAIL_DO_NOT_USE_value_storage_; - IMPLEMENTATION_DETAIL_DO_NOT_USE_bucket_array_ = - other.IMPLEMENTATION_DETAIL_DO_NOT_USE_bucket_array_; - IMPLEMENTATION_DETAIL_DO_NOT_USE_hash_ = other.IMPLEMENTATION_DETAIL_DO_NOT_USE_hash_; - IMPLEMENTATION_DETAIL_DO_NOT_USE_key_equal_ = - other.IMPLEMENTATION_DETAIL_DO_NOT_USE_key_equal_; - return *this; - } - constexpr FixedRobinhoodHashtable& operator=(const FixedRobinhoodHashtable& other) - requires(!IsReference) - = default; - + constexpr FixedRobinhoodHashtable& operator=(const FixedRobinhoodHashtable& other) = default; constexpr FixedRobinhoodHashtable& operator=(FixedRobinhoodHashtable&& other) = default; }; diff --git a/test/instance_counter.hpp b/test/instance_counter.hpp index dd268a88..316263c3 100644 --- a/test/instance_counter.hpp +++ b/test/instance_counter.hpp @@ -13,6 +13,7 @@ class InstanceCounterNonTrivialAssignment { public: static int counter; // NOLINT(readability-identifier-naming) + static int ignored_field_to_make_constructors_and_assignment_ops_non_trivial_; using Self = InstanceCounterNonTrivialAssignment; private: @@ -37,11 +38,13 @@ class InstanceCounterNonTrivialAssignment InstanceCounterNonTrivialAssignment& operator=(const Self& other) { value_ = other.value_; + ignored_field_to_make_constructors_and_assignment_ops_non_trivial_++; return *this; } InstanceCounterNonTrivialAssignment& operator=(Self&& other) noexcept { value_ = other.value_; + ignored_field_to_make_constructors_and_assignment_ops_non_trivial_++; return *this; } ~InstanceCounterNonTrivialAssignment() { counter--; } @@ -53,6 +56,9 @@ class InstanceCounterNonTrivialAssignment }; template int InstanceCounterNonTrivialAssignment::counter = 0; +template +int InstanceCounterNonTrivialAssignment< + UniqueDifferentiator>::ignored_field_to_make_constructors_and_assignment_ops_non_trivial_ = 0; template class InstanceCounterTrivialAssignment From 5e0c0f8be18a9d21f09f4e719ec67ab1fe4fb68f Mon Sep 17 00:00:00 2001 From: Alexander Karatarakis Date: Tue, 19 Nov 2024 19:16:35 -0800 Subject: [PATCH 06/15] [clang-tidy-19] Enable modernize-use-designated-initializers ``` error: use designated initializer list to initialize 'Bucket' [modernize-use-designated-initializers,-warnings-as-errors] 92 | return {increment_dist(dist_and_fingerprint_), value_index_}; | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ ``` --- .clang-tidy | 1 - .../fixed_containers/fixed_red_black_tree.hpp | 5 +- .../fixed_robinhood_hashtable.hpp | 6 +- test/circular_integer_range_iterator_test.cpp | 105 ++++++++++-------- test/comparison_chain_test.cpp | 4 +- test/enums_test_common.hpp | 8 +- ...fixed_doubly_linked_list_raw_view_test.cpp | 22 ++-- test/in_out_test.cpp | 4 +- test/integer_range_test.cpp | 2 +- test/out_test.cpp | 4 +- test/string_literal_test.cpp | 2 +- 11 files changed, 91 insertions(+), 72 deletions(-) diff --git a/.clang-tidy b/.clang-tidy index 1c0c118d..5de7394e 100644 --- a/.clang-tidy +++ b/.clang-tidy @@ -78,7 +78,6 @@ Checks: > -readability-container-size-empty, -readability-identifier-naming, -readability-math-missing-parentheses, - -modernize-use-designated-initializers, -modernize-use-ranges, -bugprone-crtp-constructor-accessibility, -bugprone-return-const-ref-from-parameter, diff --git a/include/fixed_containers/fixed_red_black_tree.hpp b/include/fixed_containers/fixed_red_black_tree.hpp index cba08b51..125b7e8c 100644 --- a/include/fixed_containers/fixed_red_black_tree.hpp +++ b/include/fixed_containers/fixed_red_black_tree.hpp @@ -638,7 +638,7 @@ class FixedRedBlackTreeBase tree_storage().delete_at_and_return_repositioned_index(index); set_root_index(NULL_INDEX); set_size(0); - return {NULL_INDEX, NULL_INDEX}; + return {.successor = NULL_INDEX, .repositioned = NULL_INDEX}; } decrement_size(); @@ -730,7 +730,8 @@ class FixedRedBlackTreeBase const NodeIndex repositioned_index = tree_storage().delete_at_and_return_repositioned_index(index_to_delete); - SuccessorIndexAndRepositionedIndex ret{successor_index, repositioned_index}; + SuccessorIndexAndRepositionedIndex ret{.successor = successor_index, + .repositioned = repositioned_index}; if (repositioned_index != index_to_delete) { diff --git a/include/fixed_containers/fixed_robinhood_hashtable.hpp b/include/fixed_containers/fixed_robinhood_hashtable.hpp index d06a18dc..69094d9b 100644 --- a/include/fixed_containers/fixed_robinhood_hashtable.hpp +++ b/include/fixed_containers/fixed_robinhood_hashtable.hpp @@ -89,12 +89,14 @@ struct Bucket [[nodiscard]] constexpr Bucket plus_dist() const { - return {increment_dist(dist_and_fingerprint_), value_index_}; + return {.dist_and_fingerprint_ = increment_dist(dist_and_fingerprint_), + .value_index_ = value_index_}; } [[nodiscard]] constexpr Bucket minus_dist() const { - return {decrement_dist(dist_and_fingerprint_), value_index_}; + return {.dist_and_fingerprint_ = decrement_dist(dist_and_fingerprint_), + .value_index_ = value_index_}; } }; diff --git a/test/circular_integer_range_iterator_test.cpp b/test/circular_integer_range_iterator_test.cpp index cca6d1d2..77307b6e 100644 --- a/test/circular_integer_range_iterator_test.cpp +++ b/test/circular_integer_range_iterator_test.cpp @@ -34,9 +34,9 @@ TEST(CircularIntegerRangeIterator, StartAndFinishAreTheSameAsRange) { using ItType = CircularIntegerRangeIterator; static constexpr ItType IT1{IntegerRange::closed_open(0, 3), - StartingIntegerAndDistance{0, 0}}; + StartingIntegerAndDistance{.start = 0, .distance = 0}}; static constexpr ItType IT_END{IntegerRange::closed_open(0, 3), - StartingIntegerAndDistance{0, 3}}; + StartingIntegerAndDistance{.start = 0, .distance = 3}}; static_assert(2 == *std::prev(IT1, 1)); static_assert(0 == *std::next(IT1, 0)); static_assert(1 == *std::next(IT1, 1)); @@ -48,9 +48,9 @@ TEST(CircularIntegerRangeIterator, StartAndFinishAreTheSameAsRange) { using ItType = CircularIntegerRangeIterator; static constexpr ItType IT1{IntegerRange::closed_open(3, 6), - StartingIntegerAndDistance{3, 0}}; + StartingIntegerAndDistance{.start = 3, .distance = 0}}; static constexpr ItType IT_END{IntegerRange::closed_open(3, 6), - StartingIntegerAndDistance{3, 3}}; + StartingIntegerAndDistance{.start = 3, .distance = 3}}; static_assert(5 == *std::prev(IT1, 1)); static_assert(3 == *std::next(IT1, 0)); static_assert(4 == *std::next(IT1, 1)); @@ -66,9 +66,9 @@ TEST(CircularIntegerRangeIterator, WrapAround) { using ItType = CircularIntegerRangeIterator; static constexpr ItType IT1{IntegerRange::closed_open(0, 3), - StartingIntegerAndDistance{2, 0}}; + StartingIntegerAndDistance{.start = 2, .distance = 0}}; static constexpr ItType IT_END{IntegerRange::closed_open(0, 3), - StartingIntegerAndDistance{2, 3}}; + StartingIntegerAndDistance{.start = 2, .distance = 3}}; static_assert(1 == *std::prev(IT1, 1)); static_assert(2 == *std::next(IT1, 0)); static_assert(0 == *std::next(IT1, 1)); @@ -80,9 +80,9 @@ TEST(CircularIntegerRangeIterator, WrapAround) { using ItType = CircularIntegerRangeIterator; static constexpr ItType IT1{IntegerRange::closed_open(3, 6), - StartingIntegerAndDistance{5, 0}}; + StartingIntegerAndDistance{.start = 5, .distance = 0}}; static constexpr ItType IT_END{IntegerRange::closed_open(3, 6), - StartingIntegerAndDistance{5, 3}}; + StartingIntegerAndDistance{.start = 5, .distance = 3}}; static_assert(4 == *std::prev(IT1, 1)); static_assert(5 == *std::next(IT1, 0)); static_assert(3 == *std::next(IT1, 1)); @@ -97,9 +97,9 @@ TEST(CircularIntegerRangeIterator, CurrentIndexNotAtStart) { using ItType = CircularIntegerRangeIterator; static constexpr ItType IT1{IntegerRange::closed_open(0, 3), - StartingIntegerAndDistance{2, 1}}; + StartingIntegerAndDistance{.start = 2, .distance = 1}}; static constexpr ItType IT_END{IntegerRange::closed_open(0, 3), - StartingIntegerAndDistance{2, 3}}; + StartingIntegerAndDistance{.start = 2, .distance = 3}}; static_assert(1 == *std::prev(IT1, 2)); static_assert(2 == *std::prev(IT1, 1)); static_assert(0 == *std::next(IT1, 0)); @@ -111,9 +111,9 @@ TEST(CircularIntegerRangeIterator, CurrentIndexNotAtStart) { using ItType = CircularIntegerRangeIterator; static constexpr ItType IT1{IntegerRange::closed_open(3, 6), - StartingIntegerAndDistance{5, 2}}; + StartingIntegerAndDistance{.start = 5, .distance = 2}}; static constexpr ItType IT_END{IntegerRange::closed_open(3, 6), - StartingIntegerAndDistance{5, 3}}; + StartingIntegerAndDistance{.start = 5, .distance = 3}}; static_assert(4 == *std::prev(IT1, 3)); static_assert(5 == *std::prev(IT1, 2)); static_assert(3 == *std::prev(IT1, 1)); @@ -128,9 +128,9 @@ TEST(CircularIntegerRangeIterator, PartialAndWrapAroundAndCurrentIndexNotAtStart { using ItType = CircularIntegerRangeIterator; static constexpr ItType IT1{IntegerRange::closed_open(0, 11), - StartingIntegerAndDistance{10, 1}}; + StartingIntegerAndDistance{.start = 10, .distance = 1}}; static constexpr ItType IT_END{IntegerRange::closed_open(0, 11), - StartingIntegerAndDistance{10, 3}}; + StartingIntegerAndDistance{.start = 10, .distance = 3}}; static_assert(9 == *std::prev(IT1, 2)); static_assert(10 == *std::prev(IT1, 1)); static_assert(0 == *std::next(IT1, 0)); @@ -142,9 +142,9 @@ TEST(CircularIntegerRangeIterator, PartialAndWrapAroundAndCurrentIndexNotAtStart { using ItType = CircularIntegerRangeIterator; static constexpr ItType IT1{IntegerRange::closed_open(3, 11), - StartingIntegerAndDistance{10, 2}}; + StartingIntegerAndDistance{.start = 10, .distance = 2}}; static constexpr ItType IT_END{IntegerRange::closed_open(3, 11), - StartingIntegerAndDistance{10, 3}}; + StartingIntegerAndDistance{.start = 10, .distance = 3}}; static_assert(9 == *std::prev(IT1, 3)); static_assert(10 == *std::prev(IT1, 2)); static_assert(3 == *std::prev(IT1, 1)); @@ -159,14 +159,14 @@ TEST(CircularIntegerRangeIterator, RandomAccess) { using ItType = CircularIntegerRangeIterator; static constexpr ItType IT1{IntegerRange::closed_open(0, 3), - StartingIntegerAndDistance{2, 0}}; + StartingIntegerAndDistance{.start = 2, .distance = 0}}; static_assert(0 == IT1[1]); } { using ItType = CircularIntegerRangeIterator; static constexpr ItType IT1{IntegerRange::closed_open(3, 11), - StartingIntegerAndDistance{4, 2}}; + StartingIntegerAndDistance{.start = 4, .distance = 2}}; static_assert(8 == IT1[2]); } } @@ -177,10 +177,10 @@ TEST(CircularIntegerRangeIterator, Equality) { // Range static constexpr IntegerRange RANGE = IntegerRange::closed_open(0, 11); - static constexpr ItType IT1{RANGE, StartingIntegerAndDistance{10, 1}}; - static constexpr ItType IT2{RANGE, StartingIntegerAndDistance{10, 1}}; + static constexpr ItType IT1{RANGE, StartingIntegerAndDistance{.start = 10, .distance = 1}}; + static constexpr ItType IT2{RANGE, StartingIntegerAndDistance{.start = 10, .distance = 1}}; static constexpr ItType IT3{IntegerRange::closed_open(0, 999), - StartingIntegerAndDistance{10, 1}}; + StartingIntegerAndDistance{.start = 10, .distance = 1}}; static_assert(IT1 == IT2); EXPECT_DEATH(void(IT1 != IT3), ""); // Hard error if attempting to compare unrelated ranges @@ -188,10 +188,10 @@ TEST(CircularIntegerRangeIterator, Equality) { // Index static constexpr IntegerRange RANGE = IntegerRange::closed_open(0, 11); - static constexpr ItType IT1{RANGE, StartingIntegerAndDistance{10, 1}}; - static constexpr ItType IT2{RANGE, StartingIntegerAndDistance{9, 2}}; - static constexpr ItType IT3{RANGE, StartingIntegerAndDistance{0, 0}}; - static constexpr ItType IT4{RANGE, StartingIntegerAndDistance{0, 99}}; + static constexpr ItType IT1{RANGE, StartingIntegerAndDistance{.start = 10, .distance = 1}}; + static constexpr ItType IT2{RANGE, StartingIntegerAndDistance{.start = 9, .distance = 2}}; + static constexpr ItType IT3{RANGE, StartingIntegerAndDistance{.start = 0, .distance = 0}}; + static constexpr ItType IT4{RANGE, StartingIntegerAndDistance{.start = 0, .distance = 99}}; static_assert(*IT1 == 0); static_assert(*IT2 == 0); @@ -201,7 +201,8 @@ TEST(CircularIntegerRangeIterator, Equality) static_assert(IT1 != IT3); // Same index, but it wraps around, so not equal static_assert(IT1 != IT4); - static constexpr ItType IT_END{RANGE, StartingIntegerAndDistance{10, 3}}; + static constexpr ItType IT_END{RANGE, + StartingIntegerAndDistance{.start = 10, .distance = 3}}; static_assert(IT1 != IT_END); static_assert(std::next(IT1, 2) == IT_END); } @@ -213,10 +214,14 @@ TEST(CircularIntegerRangeIterator, Comparison) using ItType = CircularIntegerRangeIterator; { static constexpr IntegerRange RANGE = IntegerRange::closed_open(0, 11); - static constexpr ItType IT1{RANGE, StartingIntegerAndDistance{10, 1}}; - static constexpr ItType IT2{RANGE, StartingIntegerAndDistance{9, 2}}; - static constexpr ItType IT3{RANGE, StartingIntegerAndDistance{0, 0}}; - static constexpr ItType IT4{RANGE, StartingIntegerAndDistance{0, 100}}; + static constexpr ItType IT1{RANGE, + StartingIntegerAndDistance{.start = 10, .distance = 1}}; + static constexpr ItType IT2{RANGE, + StartingIntegerAndDistance{.start = 9, .distance = 2}}; + static constexpr ItType IT3{RANGE, + StartingIntegerAndDistance{.start = 0, .distance = 0}}; + static constexpr ItType IT4{RANGE, + StartingIntegerAndDistance{.start = 0, .distance = 100}}; static_assert(*IT1 == 0); static_assert(*IT2 == 0); @@ -238,10 +243,14 @@ TEST(CircularIntegerRangeIterator, Comparison) using ItType = CircularIntegerRangeIterator; { static constexpr IntegerRange RANGE = IntegerRange::closed_open(0, 11); - static constexpr ItType IT1{RANGE, StartingIntegerAndDistance{10, 1}}; - static constexpr ItType IT2{RANGE, StartingIntegerAndDistance{9, 2}}; - static constexpr ItType IT3{RANGE, StartingIntegerAndDistance{0, 0}}; - static constexpr ItType IT4{RANGE, StartingIntegerAndDistance{0, 100}}; + static constexpr ItType IT1{RANGE, + StartingIntegerAndDistance{.start = 10, .distance = 1}}; + static constexpr ItType IT2{RANGE, + StartingIntegerAndDistance{.start = 9, .distance = 2}}; + static constexpr ItType IT3{RANGE, + StartingIntegerAndDistance{.start = 0, .distance = 0}}; + static constexpr ItType IT4{RANGE, + StartingIntegerAndDistance{.start = 0, .distance = 100}}; static_assert(*IT1 == 10); static_assert(*IT2 == 10); @@ -266,10 +275,14 @@ TEST(CircularIntegerRangeIterator, OperatorMinus) using ItType = CircularIntegerRangeIterator; { static constexpr IntegerRange RANGE = IntegerRange::closed_open(0, 11); - static constexpr ItType IT1{RANGE, StartingIntegerAndDistance{10, 1}}; - static constexpr ItType IT2{RANGE, StartingIntegerAndDistance{9, 2}}; - static constexpr ItType IT3{RANGE, StartingIntegerAndDistance{0, 0}}; - static constexpr ItType IT4{RANGE, StartingIntegerAndDistance{0, 99}}; + static constexpr ItType IT1{RANGE, + StartingIntegerAndDistance{.start = 10, .distance = 1}}; + static constexpr ItType IT2{RANGE, + StartingIntegerAndDistance{.start = 9, .distance = 2}}; + static constexpr ItType IT3{RANGE, + StartingIntegerAndDistance{.start = 0, .distance = 0}}; + static constexpr ItType IT4{RANGE, + StartingIntegerAndDistance{.start = 0, .distance = 99}}; static_assert(*IT1 == 0); static_assert(*IT2 == 0); @@ -281,7 +294,7 @@ TEST(CircularIntegerRangeIterator, OperatorMinus) static_assert(99 == IT4 - IT3); static constexpr ItType IT_END{IntegerRange::closed_open(0, 11), - StartingIntegerAndDistance{10, 3}}; + StartingIntegerAndDistance{.start = 10, .distance = 3}}; static_assert(2 == IT_END - IT1); } } @@ -290,10 +303,14 @@ TEST(CircularIntegerRangeIterator, OperatorMinus) using ItType = CircularIntegerRangeIterator; { static constexpr IntegerRange RANGE = IntegerRange::closed_open(0, 11); - static constexpr ItType IT1{RANGE, StartingIntegerAndDistance{10, 1}}; - static constexpr ItType IT2{RANGE, StartingIntegerAndDistance{9, 2}}; - static constexpr ItType IT3{RANGE, StartingIntegerAndDistance{0, 0}}; - static constexpr ItType IT4{RANGE, StartingIntegerAndDistance{0, 99}}; + static constexpr ItType IT1{RANGE, + StartingIntegerAndDistance{.start = 10, .distance = 1}}; + static constexpr ItType IT2{RANGE, + StartingIntegerAndDistance{.start = 9, .distance = 2}}; + static constexpr ItType IT3{RANGE, + StartingIntegerAndDistance{.start = 0, .distance = 0}}; + static constexpr ItType IT4{RANGE, + StartingIntegerAndDistance{.start = 0, .distance = 99}}; static_assert(*IT1 == 10); static_assert(*IT2 == 10); @@ -305,7 +322,7 @@ TEST(CircularIntegerRangeIterator, OperatorMinus) static_assert(-99 == IT4 - IT3); static constexpr ItType IT_REND{IntegerRange::closed_open(0, 11), - StartingIntegerAndDistance{10, 0}}; + StartingIntegerAndDistance{.start = 10, .distance = 0}}; static_assert(1 == IT_REND - IT1); } } diff --git a/test/comparison_chain_test.cpp b/test/comparison_chain_test.cpp index 168695ca..734b73e3 100644 --- a/test/comparison_chain_test.cpp +++ b/test/comparison_chain_test.cpp @@ -68,8 +68,8 @@ TEST(ComparisonChain, SimpleTest) } TEST(ComparisonChain, ComparatorUsage) { - constexpr MyCompoundStruct STRUCT1{1, 2, 3, 4}; - constexpr MyCompoundStruct STRUCT2{1, 2, 5, 4}; + constexpr MyCompoundStruct STRUCT1{.a = 1, .b = 2, .c = 3, .d = 4}; + constexpr MyCompoundStruct STRUCT2{.a = 1, .b = 2, .c = 5, .d = 4}; static_assert(STRUCT1 < STRUCT2); } diff --git a/test/enums_test_common.hpp b/test/enums_test_common.hpp index cc4607d1..f0cf9081 100644 --- a/test/enums_test_common.hpp +++ b/test/enums_test_common.hpp @@ -89,10 +89,10 @@ struct TestRichEnum1Data inline constexpr auto TEST_RICH_ENUM_1_DATA = EnumMap::create_with_all_entries({ - {TestRichEnum1BackingEnum::C_ONE, {1, 1.0}}, - {TestRichEnum1BackingEnum::C_TWO, {2, 2.0}}, - {TestRichEnum1BackingEnum::C_THREE, {3, 3.0}}, - {TestRichEnum1BackingEnum::C_FOUR, {4, 4.0}}, + {TestRichEnum1BackingEnum::C_ONE, {.value = 1, .double_value = 1.0}}, + {TestRichEnum1BackingEnum::C_TWO, {.value = 2, .double_value = 2.0}}, + {TestRichEnum1BackingEnum::C_THREE, {.value = 3, .double_value = 3.0}}, + {TestRichEnum1BackingEnum::C_FOUR, {.value = 4, .double_value = 4.0}}, }); } // namespace detail diff --git a/test/fixed_doubly_linked_list_raw_view_test.cpp b/test/fixed_doubly_linked_list_raw_view_test.cpp index 51c46b76..a9d250ae 100644 --- a/test/fixed_doubly_linked_list_raw_view_test.cpp +++ b/test/fixed_doubly_linked_list_raw_view_test.cpp @@ -88,13 +88,13 @@ TEST(FixedDoublyLinkedListRawView, ViewOfStructList) FixedDoublyLinkedList list; const std::size_t first = - list.emplace_back_and_return_index(StructThatContainsPadding{'a', 123}); - list.emplace_back_and_return_index(StructThatContainsPadding{'b', 234}); - list.emplace_back_and_return_index(StructThatContainsPadding{'c', 345}); - list.emplace_front_and_return_index(StructThatContainsPadding{'Z', 321}); + list.emplace_back_and_return_index(StructThatContainsPadding{.a = 'a', .b = 123}); + list.emplace_back_and_return_index(StructThatContainsPadding{.a = 'b', .b = 234}); + list.emplace_back_and_return_index(StructThatContainsPadding{.a = 'c', .b = 345}); + list.emplace_front_and_return_index(StructThatContainsPadding{.a = 'Z', .b = 321}); list.delete_at_and_return_next_index(first); - list.emplace_front_and_return_index(StructThatContainsPadding{'Y', 432}); - list.emplace_back_and_return_index(StructThatContainsPadding{'d', 456}); + list.emplace_front_and_return_index(StructThatContainsPadding{.a = 'Y', .b = 432}); + list.emplace_back_and_return_index(StructThatContainsPadding{.a = 'd', .b = 456}); // list is Y, Z, b, c, d auto view = FixedDoublyLinkedListRawView(reinterpret_cast(&list), @@ -112,19 +112,19 @@ TEST(FixedDoublyLinkedListRawView, ViewOfStructList) auto iter = view.begin(); EXPECT_EQ(get_from_ptr(*iter), - (StructThatContainsPadding{'Y', 432})); + (StructThatContainsPadding{.a = 'Y', .b = 432})); iter = std::next(iter); EXPECT_EQ(get_from_ptr(*iter), - (StructThatContainsPadding{'Z', 321})); + (StructThatContainsPadding{.a = 'Z', .b = 321})); iter = std::next(iter); EXPECT_EQ(get_from_ptr(*iter), - (StructThatContainsPadding{'b', 234})); + (StructThatContainsPadding{.a = 'b', .b = 234})); iter = std::next(iter); EXPECT_EQ(get_from_ptr(*iter), - (StructThatContainsPadding{'c', 345})); + (StructThatContainsPadding{.a = 'c', .b = 345})); iter = std::next(iter); EXPECT_EQ(get_from_ptr(*iter), - (StructThatContainsPadding{'d', 456})); + (StructThatContainsPadding{.a = 'd', .b = 456})); iter = std::next(iter); EXPECT_EQ(iter, view.end()); } diff --git a/test/in_out_test.cpp b/test/in_out_test.cpp index 4ebcb421..12608148 100644 --- a/test/in_out_test.cpp +++ b/test/in_out_test.cpp @@ -52,7 +52,7 @@ TEST(InOut, Usage2) { constexpr SomeStruct RESULT = []() { - SomeStruct instance{10, 20}; + SomeStruct instance{.a = 10, .b = 20}; increment_struct(in_out{instance}); return instance; }(); @@ -62,7 +62,7 @@ TEST(InOut, Usage2) } { - SomeStruct instance{10, 20}; + SomeStruct instance{.a = 10, .b = 20}; increment_struct(in_out{instance}); EXPECT_EQ(11, instance.a); EXPECT_EQ(22, instance.b); diff --git a/test/integer_range_test.cpp b/test/integer_range_test.cpp index 40fd7daa..dda2b57b 100644 --- a/test/integer_range_test.cpp +++ b/test/integer_range_test.cpp @@ -43,7 +43,7 @@ TEST(IntegerRange, CompileTimeIntegerRange) TEST(StartingIntegerAndDistance, Simple) { - static constexpr StartingIntegerAndDistance VAL{3, 7}; + static constexpr StartingIntegerAndDistance VAL{.start = 3, .distance = 7}; static_assert(3 == VAL.start); static_assert(7 == VAL.distance); static_assert(3 == VAL.to_range().start_inclusive()); diff --git a/test/out_test.cpp b/test/out_test.cpp index 502aed18..bb0d2aa8 100644 --- a/test/out_test.cpp +++ b/test/out_test.cpp @@ -52,7 +52,7 @@ TEST(Out, Usage2) { constexpr SomeStruct RESULT = []() { - SomeStruct instance{0, 0}; + SomeStruct instance{.a = 0, .b = 0}; set_struct(out{instance}); return instance; }(); @@ -62,7 +62,7 @@ TEST(Out, Usage2) } { - SomeStruct instance{0, 0}; + SomeStruct instance{.a = 0, .b = 0}; set_struct(out{instance}); EXPECT_EQ(1, instance.a); EXPECT_EQ(2, instance.b); diff --git a/test/string_literal_test.cpp b/test/string_literal_test.cpp index fa04be1b..1c16e804 100644 --- a/test/string_literal_test.cpp +++ b/test/string_literal_test.cpp @@ -62,7 +62,7 @@ TEST(StringLiteral, CopyandMoveConstructor) StringLiteral b; }; - constexpr MyStruct VAL1{"foo", "bar"}; + constexpr MyStruct VAL1{.a = "foo", .b = "bar"}; static_assert(VAL1.a.as_view() == "foo"); static_assert(VAL1.b.as_view() == "bar"); From c83d168b06da973b04002e7c400294f10e079d75 Mon Sep 17 00:00:00 2001 From: Alexander Karatarakis Date: Tue, 19 Nov 2024 19:30:00 -0800 Subject: [PATCH 07/15] [clang-tidy-19] Enable readability-math-missing-parentheses ``` error: '*' has higher precedence than '-'; add parentheses to explicitly specify the order of operations [readability-math-missing-parentheses,-warnings-as-errors] ``` --- .clang-tidy | 1 - include/fixed_containers/circular_indexing.hpp | 2 +- .../fixed_containers/fixed_doubly_linked_list_raw_view.hpp | 2 +- include/fixed_containers/fixed_red_black_tree_types.hpp | 2 +- include/fixed_containers/fixed_red_black_tree_view.hpp | 2 +- test/struct_view_test.cpp | 6 +++--- test/tuples_test.cpp | 4 ++-- 7 files changed, 9 insertions(+), 10 deletions(-) diff --git a/.clang-tidy b/.clang-tidy index 5de7394e..7b62d464 100644 --- a/.clang-tidy +++ b/.clang-tidy @@ -77,7 +77,6 @@ Checks: > -performance-unnecessary-copy-initialization, -readability-container-size-empty, -readability-identifier-naming, - -readability-math-missing-parentheses, -modernize-use-ranges, -bugprone-crtp-constructor-accessibility, -bugprone-return-const-ref-from-parameter, diff --git a/include/fixed_containers/circular_indexing.hpp b/include/fixed_containers/circular_indexing.hpp index a1e1eed7..6ee5a23a 100644 --- a/include/fixed_containers/circular_indexing.hpp +++ b/include/fixed_containers/circular_indexing.hpp @@ -40,7 +40,7 @@ constexpr CyclesAndInteger increment_index_with_wraparound(const IntegerRangeTyp int_math::safe_subtract(new_index_unwrapped, range.start_inclusive()) .template cast(); const std::size_t quotient = adjust_to_zero / range_size; - const std::size_t remainder = adjust_to_zero - quotient * range_size; + const std::size_t remainder = adjust_to_zero - (quotient * range_size); return {.cycles = static_cast(quotient), .integer = remainder + range.start_inclusive()}; } diff --git a/include/fixed_containers/fixed_doubly_linked_list_raw_view.hpp b/include/fixed_containers/fixed_doubly_linked_list_raw_view.hpp index ce88de1b..c75e4411 100644 --- a/include/fixed_containers/fixed_doubly_linked_list_raw_view.hpp +++ b/include/fixed_containers/fixed_doubly_linked_list_raw_view.hpp @@ -89,7 +89,7 @@ class FixedDoublyLinkedListRawView { // the full PoolStorage will be aligned to the alignment of the element, which matters if it // aligns bigger than alignof(size_t) - std::size_t raw_size = max_elem_count_ * elem_size_bytes_ + sizeof(std::size_t); + std::size_t raw_size = (max_elem_count_ * elem_size_bytes_) + sizeof(std::size_t); if (raw_size % elem_align_bytes_ != 0) { raw_size += elem_align_bytes_ - (raw_size % elem_align_bytes_); diff --git a/include/fixed_containers/fixed_red_black_tree_types.hpp b/include/fixed_containers/fixed_red_black_tree_types.hpp index 786c11e2..d2396791 100644 --- a/include/fixed_containers/fixed_red_black_tree_types.hpp +++ b/include/fixed_containers/fixed_red_black_tree_types.hpp @@ -26,7 +26,7 @@ constexpr NodeColor COLOR_RED = true; // care about values 0 to MAXIMUM_SIZE. Furthermore, NULL_INDEX is at max(). class NodeIndexWithColorEmbeddedInTheMostSignificantBit { - static constexpr std::size_t SHIFT_TO_MOST_SIGNIFICANT_BIT = sizeof(NodeIndex) * 8ULL - 1ULL; + static constexpr std::size_t SHIFT_TO_MOST_SIGNIFICANT_BIT = (sizeof(NodeIndex) * 8ULL) - 1ULL; static constexpr NodeIndex MASK = 1ULL << SHIFT_TO_MOST_SIGNIFICANT_BIT; static constexpr NodeIndex LOCAL_NULL_INDEX = NULL_INDEX >> 1; diff --git a/include/fixed_containers/fixed_red_black_tree_view.hpp b/include/fixed_containers/fixed_red_black_tree_view.hpp index c9bc809a..2f279c98 100644 --- a/include/fixed_containers/fixed_red_black_tree_view.hpp +++ b/include/fixed_containers/fixed_red_black_tree_view.hpp @@ -16,7 +16,7 @@ template constexpr std::common_type_t align_up(M m_val, N n_val) { assert_or_abort(n_val > 0); - return m_val + n_val - 1 - (m_val + n_val - 1) % n_val; + return m_val + n_val - 1 - ((m_val + n_val - 1) % n_val); } class FixedRedBlackTreeRawView diff --git a/test/struct_view_test.cpp b/test/struct_view_test.cpp index ee3e1700..5c84cf82 100644 --- a/test/struct_view_test.cpp +++ b/test/struct_view_test.cpp @@ -510,11 +510,11 @@ TEST(StructView, GetPointerDistanceRecursiveWithArray) EXPECT_EQ(8 + 8, struct_view_detail::get_pointer_distance(array_test_super_struct_1, array_test_super_struct_1.arr)); - EXPECT_EQ(8 + 8 + TEST_ARRAY_SIZE * sizeof(ArrayTestSuperStructLayer2) + 8, + EXPECT_EQ(8 + 8 + (TEST_ARRAY_SIZE * sizeof(ArrayTestSuperStructLayer2)) + 8, struct_view_detail::get_pointer_distance(array_test_super_struct_1, array_test_super_struct_1.vec)); - EXPECT_EQ(8 + 8 + TEST_ARRAY_SIZE * sizeof(ArrayTestSuperStructLayer2) + 8 + - (8 + TEST_ARRAY_SIZE * sizeof(ArrayTestSuperStructLayer2)), + EXPECT_EQ(8 + 8 + (TEST_ARRAY_SIZE * sizeof(ArrayTestSuperStructLayer2)) + 8 + + (8 + (TEST_ARRAY_SIZE * sizeof(ArrayTestSuperStructLayer2))), struct_view_detail::get_pointer_distance(array_test_super_struct_1, array_test_super_struct_1.matrix)); } diff --git a/test/tuples_test.cpp b/test/tuples_test.cpp index 484ac95f..54510759 100644 --- a/test/tuples_test.cpp +++ b/test/tuples_test.cpp @@ -56,9 +56,9 @@ TEST(Tuples, AsTupleViewCodegenBranches) std::cout << " // clang-format off \n"; for (std::size_t group_id = 0; group_id < GROUP_COUNT; group_id++) { - const std::size_t starting_i = group_id * GROUP_SIZE + 1; + const std::size_t starting_i = (group_id * GROUP_SIZE) + 1; const std::size_t ending_i = - std::min(MAX_VARIABLE_COUNT, group_id * GROUP_SIZE + GROUP_SIZE); + std::min(MAX_VARIABLE_COUNT, (group_id * GROUP_SIZE) + GROUP_SIZE); std::cout << " else if constexpr(FIELD_COUNT <= " << ending_i << ") {\n"; From 34a07f4f828c8980a4b21e16a7a8639b73f4a708 Mon Sep 17 00:00:00 2001 From: Alexander Karatarakis Date: Tue, 19 Nov 2024 19:37:12 -0800 Subject: [PATCH 08/15] [clang-tidy-19] Enable modernize-use-ranges ``` error: use a ranges version of this algorithm [modernize-use-ranges,-warnings-as-errors] 1709 | var1.erase(std::remove_if(var1.begin(), var1.end(), [&](const auto&) { return true; }), | ^~~~~~~~~~~~~~ ~~~~~~~~~~~~ ~~~~~~~~~~ | std::ranges::remove_if var1 ``` --- .clang-tidy | 1 - test/enum_set_test.cpp | 7 ++----- test/fixed_list_test.cpp | 8 ++++++-- test/fixed_red_black_tree_test.cpp | 2 +- test/fixed_set_test.cpp | 7 ++----- test/fixed_string_test.cpp | 8 ++++++-- test/fixed_unordered_set_test.cpp | 7 ++----- test/fixed_vector_test.cpp | 8 ++++++-- 8 files changed, 25 insertions(+), 23 deletions(-) diff --git a/.clang-tidy b/.clang-tidy index 7b62d464..8e38eefd 100644 --- a/.clang-tidy +++ b/.clang-tidy @@ -77,7 +77,6 @@ Checks: > -performance-unnecessary-copy-initialization, -readability-container-size-empty, -readability-identifier-naming, - -modernize-use-ranges, -bugprone-crtp-constructor-accessibility, -bugprone-return-const-ref-from-parameter, -bugprone-use-after-move, diff --git a/test/enum_set_test.cpp b/test/enum_set_test.cpp index f2a200f4..26f44b76 100644 --- a/test/enum_set_test.cpp +++ b/test/enum_set_test.cpp @@ -704,11 +704,8 @@ TEST(EnumSet, SetIntersection) const EnumSet var2{TestEnum1::ONE}; EnumSet v_intersection; - std::set_intersection(var1.begin(), - var1.end(), - var2.begin(), - var2.end(), - std::inserter(v_intersection, v_intersection.begin())); + std::ranges::set_intersection( + var1, var2, std::inserter(v_intersection, v_intersection.begin())); return v_intersection; }(); diff --git a/test/fixed_list_test.cpp b/test/fixed_list_test.cpp index c99da573..0eec476b 100644 --- a/test/fixed_list_test.cpp +++ b/test/fixed_list_test.cpp @@ -1696,7 +1696,9 @@ TEST(FixedList, EraseEmpty) FixedList var1{}; // Don't Expect Death - var1.erase(std::remove_if(var1.begin(), var1.end(), [&](const auto&) { return true; }), + var1.erase(std::remove_if(var1.begin(), // NOLINT(modernize-use-ranges) + var1.end(), + [&](const auto&) { return true; }), var1.end()); EXPECT_DEATH(var1.erase(var1.begin()), ""); @@ -1706,7 +1708,9 @@ TEST(FixedList, EraseEmpty) std::list var1{}; // Don't Expect Death - var1.erase(std::remove_if(var1.begin(), var1.end(), [&](const auto&) { return true; }), + var1.erase(std::remove_if(var1.begin(), // NOLINT(modernize-use-ranges) + var1.end(), + [&](const auto&) { return true; }), var1.end()); // The iterator pos must be valid and dereferenceable. Thus the end() iterator (which is diff --git a/test/fixed_red_black_tree_test.cpp b/test/fixed_red_black_tree_test.cpp index 231b9aa7..5e45ffc5 100644 --- a/test/fixed_red_black_tree_test.cpp +++ b/test/fixed_red_black_tree_test.cpp @@ -1851,7 +1851,7 @@ TEST(FixedRedBlackTree, TreeMaxHeight) } // Descending Insertion - std::reverse(insertion_order.begin(), insertion_order.end()); + std::ranges::reverse(insertion_order); for (std::size_t i = 0; i < MAXIMUM_SIZE; i++) { bst[insertion_order[i]] = insertion_order[i]; diff --git a/test/fixed_set_test.cpp b/test/fixed_set_test.cpp index 69f06cf2..b3046676 100644 --- a/test/fixed_set_test.cpp +++ b/test/fixed_set_test.cpp @@ -776,11 +776,8 @@ TEST(FixedSet, SetIntersection) const FixedSet var2{1}; FixedSet v_intersection; - std::set_intersection(var1.begin(), - var1.end(), - var2.begin(), - var2.end(), - std::inserter(v_intersection, v_intersection.begin())); + std::ranges::set_intersection( + var1, var2, std::inserter(v_intersection, v_intersection.begin())); return v_intersection; }(); diff --git a/test/fixed_string_test.cpp b/test/fixed_string_test.cpp index 604efc51..eec005d1 100644 --- a/test/fixed_string_test.cpp +++ b/test/fixed_string_test.cpp @@ -995,7 +995,9 @@ TEST(FixedString, EraseEmpty) FixedString<3> var1{}; // Don't Expect Death - var1.erase(std::remove_if(var1.begin(), var1.end(), [&](const auto&) { return true; }), + var1.erase(std::remove_if(var1.begin(), // NOLINT(modernize-use-ranges) + var1.end(), + [&](const auto&) { return true; }), var1.end()); EXPECT_DEATH(var1.erase(var1.begin()), ""); @@ -1005,7 +1007,9 @@ TEST(FixedString, EraseEmpty) std::string var1{}; // Don't Expect Death - var1.erase(std::remove_if(var1.begin(), var1.end(), [&](const auto&) { return true; }), + var1.erase(std::remove_if(var1.begin(), // NOLINT(modernize-use-ranges) + var1.end(), + [&](const auto&) { return true; }), var1.end()); // The iterator pos must be valid and dereferenceable. Thus the end() iterator (which is diff --git a/test/fixed_unordered_set_test.cpp b/test/fixed_unordered_set_test.cpp index 5024d4a6..b48b6219 100644 --- a/test/fixed_unordered_set_test.cpp +++ b/test/fixed_unordered_set_test.cpp @@ -656,11 +656,8 @@ TEST(FixedUnorderedSet, SetIntersection) const FixedUnorderedSet var2{1}; FixedUnorderedSet v_intersection; - std::set_intersection(var1.begin(), - var1.end(), - var2.begin(), - var2.end(), - std::inserter(v_intersection, v_intersection.begin())); + std::ranges::set_intersection( + var1, var2, std::inserter(v_intersection, v_intersection.begin())); return v_intersection; }(); diff --git a/test/fixed_vector_test.cpp b/test/fixed_vector_test.cpp index 68a1469f..0c9c3a49 100644 --- a/test/fixed_vector_test.cpp +++ b/test/fixed_vector_test.cpp @@ -1719,7 +1719,9 @@ TEST(FixedVector, EraseEmpty) FixedVector var1{}; // Don't Expect Death - var1.erase(std::remove_if(var1.begin(), var1.end(), [&](const auto&) { return true; }), + var1.erase(std::remove_if(var1.begin(), // NOLINT(modernize-use-ranges) + var1.end(), + [&](const auto&) { return true; }), var1.end()); EXPECT_DEATH(var1.erase(var1.begin()), ""); @@ -1729,7 +1731,9 @@ TEST(FixedVector, EraseEmpty) std::vector var1{}; // Don't Expect Death - var1.erase(std::remove_if(var1.begin(), var1.end(), [&](const auto&) { return true; }), + var1.erase(std::remove_if(var1.begin(), // NOLINT(modernize-use-ranges) + var1.end(), + [&](const auto&) { return true; }), var1.end()); // The iterator pos must be valid and dereferenceable. Thus the end() iterator (which is From 382293a5e752b8afe15ad7e8fc82e9eb7ded96fb Mon Sep 17 00:00:00 2001 From: Alexander Karatarakis Date: Thu, 21 Nov 2024 18:25:14 -0800 Subject: [PATCH 09/15] [clang-tidy-19] Enable readability-identifier-naming --- .clang-tidy | 1 - include/fixed_containers/enum_utils.hpp | 8 ++++---- 2 files changed, 4 insertions(+), 5 deletions(-) diff --git a/.clang-tidy b/.clang-tidy index 8e38eefd..6142b8ce 100644 --- a/.clang-tidy +++ b/.clang-tidy @@ -76,7 +76,6 @@ Checks: > -bugprone-unchecked-optional-access, -performance-unnecessary-copy-initialization, -readability-container-size-empty, - -readability-identifier-naming, -bugprone-crtp-constructor-accessibility, -bugprone-return-const-ref-from-parameter, -bugprone-use-after-move, diff --git a/include/fixed_containers/enum_utils.hpp b/include/fixed_containers/enum_utils.hpp index 3fb5203c..f8d8f695 100644 --- a/include/fixed_containers/enum_utils.hpp +++ b/include/fixed_containers/enum_utils.hpp @@ -335,10 +335,10 @@ using RichEnumStorage = std::conditional_t Date: Sat, 23 Nov 2024 15:32:08 -0800 Subject: [PATCH 10/15] Introduce NoisyClass for debugging purposes --- BUILD.bazel | 17 +++++++++++++++++ CMakeLists.txt | 2 ++ test/noisy_class.hpp | 36 ++++++++++++++++++++++++++++++++++++ test/noisy_class_test.cpp | 7 +++++++ 4 files changed, 62 insertions(+) create mode 100644 test/noisy_class.hpp create mode 100644 test/noisy_class_test.cpp diff --git a/BUILD.bazel b/BUILD.bazel index 11681179..fa46f170 100644 --- a/BUILD.bazel +++ b/BUILD.bazel @@ -1618,6 +1618,23 @@ cc_test( copts = ["-std=c++20"], ) +cc_library( + name = "noisy_class", + srcs = ["test/noisy_class.hpp"], + copts = ["-std=c++20"], +) + +cc_test( + name = "noisy_class_test", + srcs = ["test/noisy_class_test.cpp"], + deps = [ + ":noisy_class", + "@com_google_googletest//:gtest", + "@com_google_googletest//:gtest_main", + ], + copts = ["-std=c++20"], +) + cc_test( name = "optional_reference_test", srcs = ["test/optional_reference_test.cpp"], diff --git a/CMakeLists.txt b/CMakeLists.txt index 3510fc38..118b6135 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -209,6 +209,8 @@ if(BUILD_TESTS) add_test_dependencies(macro_countermeasures_test) add_executable(memory_test test/memory_test.cpp) add_test_dependencies(memory_test) + add_executable(noisy_class_test test/noisy_class_test.cpp) + add_test_dependencies(noisy_class_test) add_executable(optional_reference_test test/optional_reference_test.cpp) add_test_dependencies(optional_reference_test) add_executable(out_test test/out_test.cpp) diff --git a/test/noisy_class.hpp b/test/noisy_class.hpp new file mode 100644 index 00000000..43fe5c92 --- /dev/null +++ b/test/noisy_class.hpp @@ -0,0 +1,36 @@ +#pragma once + +#include +#include +#include + +struct NoisyClass +{ + std::string s; + + NoisyClass() + : s() + { + std::cout << "default constructor" << '\n'; + } + NoisyClass(const std::string_view sv0) + : s(" ") + { + s.append(sv0); + std::cout << "constructor" << s << '\n'; + } + NoisyClass(const NoisyClass& /*unused*/) { std::cout << "copy constructor" << s << '\n'; } + NoisyClass(NoisyClass&& /*unused*/) noexcept { std::cout << "move constructor" << s << '\n'; } + NoisyClass& operator=(const NoisyClass& /*unused*/) + { + std::cout << "copy assignment operator" << s << '\n'; + return *this; + } + NoisyClass& operator=(NoisyClass&& /*unused*/) noexcept + { + std::cout << "move assignment operator" << s << '\n'; + return *this; + } + + ~NoisyClass() { std::cout << "destructor" << s << '\n'; } +}; diff --git a/test/noisy_class_test.cpp b/test/noisy_class_test.cpp new file mode 100644 index 00000000..af2aeaab --- /dev/null +++ b/test/noisy_class_test.cpp @@ -0,0 +1,7 @@ +#include "noisy_class.hpp" + +namespace fixed_containers +{ +// Simply ensure it compiles. +static_assert(sizeof(NoisyClass) > 1); +} // namespace fixed_containers From 2fddaf99d0d0af288e8bdc86816287ce1040449d Mon Sep 17 00:00:00 2001 From: Alexander Karatarakis Date: Mon, 25 Nov 2024 15:00:18 -0800 Subject: [PATCH 11/15] [clang-tidy-19] Enable bugprone-use-after-move ``` error: 'pair' used after it was forwarded [bugprone-use-after-move,-warnings-as-errors] 21 | return container.try_emplace(std::forward(pair).first, | ^ note: forward occurred here 22 | std::forward(pair).second); | ^ ``` --- .clang-tidy | 1 - include/fixed_containers/emplace.hpp | 7 ++-- .../fixed_containers/recursive_reflection.hpp | 36 ++++++++----------- include/fixed_containers/reflection.hpp | 9 +++-- include/fixed_containers/struct_view.hpp | 2 +- include/fixed_containers/tuples.hpp | 4 +-- test/recursive_reflection_test.cpp | 10 +++--- test/struct_view_test.cpp | 10 +++--- 8 files changed, 34 insertions(+), 45 deletions(-) diff --git a/.clang-tidy b/.clang-tidy index 6142b8ce..4cabb590 100644 --- a/.clang-tidy +++ b/.clang-tidy @@ -78,7 +78,6 @@ Checks: > -readability-container-size-empty, -bugprone-crtp-constructor-accessibility, -bugprone-return-const-ref-from-parameter, - -bugprone-use-after-move, # Turn all the warnings from the checks above into errors. WarningsAsErrors: "*" diff --git a/include/fixed_containers/emplace.hpp b/include/fixed_containers/emplace.hpp index 5b643864..e183a20f 100644 --- a/include/fixed_containers/emplace.hpp +++ b/include/fixed_containers/emplace.hpp @@ -17,9 +17,10 @@ constexpr std::pair emplace_in_terms_of_try_ if constexpr (sizeof...(Rest) == 0 && IsStdPair) { // Lambda to avoid compilation errors with .first/.second when passing a non-pair - return [&container](Pair&& pair) { - return container.try_emplace(std::forward(pair).first, - std::forward(pair).second); + return [&container](Pair&& pair) + { + return container.try_emplace(std::forward(pair.first), + std::forward(pair.second)); }(std::forward(first)); } else if constexpr (sizeof...(Rest) == 2 && diff --git a/include/fixed_containers/recursive_reflection.hpp b/include/fixed_containers/recursive_reflection.hpp index 7433cb00..f19ee17b 100644 --- a/include/fixed_containers/recursive_reflection.hpp +++ b/include/fixed_containers/recursive_reflection.hpp @@ -136,8 +136,8 @@ struct ReflectionHandler PostFunction&& post_fn, in_out chain) { - std::forward(pre_fn)(std::as_const(*chain), std::forward(instance)); - std::forward(post_fn)(std::as_const(*chain), std::forward(instance)); + std::forward(pre_fn)(std::as_const(*chain), instance); + std::forward(post_fn)(std::as_const(*chain), instance); } }; @@ -155,7 +155,7 @@ struct ReflectionHandler PostFunction&& post_fn, in_out chain) { - std::forward(pre_fn)(std::as_const(*chain), std::forward(instance)); + pre_fn(std::as_const(*chain), instance); chain->push_back(OPTIONAL_PATH_NAME); bool constructed{false}; if (!instance.has_value()) @@ -163,16 +163,14 @@ struct ReflectionHandler instance.emplace(); constructed = true; } - recursive_reflection::for_each_path_dfs_helper(instance.value(), - std::forward(pre_fn), - std::forward(post_fn), - in_out{*chain}); + recursive_reflection::for_each_path_dfs_helper( + instance.value(), pre_fn, post_fn, in_out{*chain}); if (constructed) { instance.reset(); } chain->pop_back(); - std::forward(post_fn)(std::as_const(*chain), std::forward(instance)); + post_fn(std::as_const(*chain), instance); } }; @@ -190,7 +188,7 @@ struct ReflectionHandler PostFunction&& post_fn, in_out chain) { - std::forward(pre_fn)(std::as_const(*chain), std::forward(instance)); + pre_fn(std::as_const(*chain), instance); chain->push_back(ITERABLE_PATH_NAME); bool constructed{false}; if (std::ranges::empty(instance)) @@ -198,16 +196,14 @@ struct ReflectionHandler std::ranges::construct_at(std::ranges::data(instance)); constructed = true; } - recursive_reflection::for_each_path_dfs_helper(*std::ranges::data(instance), - std::forward(pre_fn), - std::forward(post_fn), - in_out{*chain}); + recursive_reflection::for_each_path_dfs_helper( + *std::ranges::data(instance), pre_fn, post_fn, in_out{*chain}); if (constructed) { std::ranges::destroy_at(std::ranges::data(instance)); } chain->pop_back(); - std::forward(post_fn)(std::as_const(*chain), std::forward(instance)); + post_fn(std::as_const(*chain), instance); } }; @@ -225,19 +221,17 @@ struct ReflectionHandler PostFunction&& post_fn, in_out chain) { - std::forward(pre_fn)(std::as_const(*chain), std::forward(instance)); + pre_fn(std::as_const(*chain), instance); reflection::for_each_field( - std::forward(instance), + instance, [&pre_fn, &post_fn, &chain](const std::string_view& name, F& field) { chain->push_back(name); - recursive_reflection::for_each_path_dfs_helper(field, - std::forward(pre_fn), - std::forward(post_fn), - fixed_containers::in_out{*chain}); + recursive_reflection::for_each_path_dfs_helper( + field, pre_fn, post_fn, fixed_containers::in_out{*chain}); chain->pop_back(); }); - std::forward(post_fn)(std::as_const(*chain), std::forward(instance)); + post_fn(std::as_const(*chain), instance); } }; diff --git a/include/fixed_containers/reflection.hpp b/include/fixed_containers/reflection.hpp index 2dd600cd..98266042 100644 --- a/include/fixed_containers/reflection.hpp +++ b/include/fixed_containers/reflection.hpp @@ -158,7 +158,7 @@ constexpr void for_each_parsed_field_entry(const T& instance, Func func) const char* const fmt, Args&&... args) { - layer_tracker->update_layer(fmt, std::forward(args)...); + layer_tracker->update_layer(fmt, args...); if constexpr (sizeof...(args) >= 3) { auto as_tuple = std::forward_as_tuple(std::forward(args)...); @@ -241,10 +241,9 @@ constexpr void for_each_field(T&& instance, Func&& func) { constexpr const auto& FIELD_NAMES = field_names_of(); auto tuple_view = tuples::as_tuple_view(instance); - tuples::for_each_entry( - tuple_view, - [&func](std::size_t index, Field&& field) - { std::forward(func)(FIELD_NAMES.at(index), std::forward(field)); }); + tuples::for_each_entry(tuple_view, + [&func](std::size_t index, Field&& field) + { func(FIELD_NAMES.at(index), std::forward(field)); }); } } // namespace fixed_containers::reflection diff --git a/include/fixed_containers/struct_view.hpp b/include/fixed_containers/struct_view.hpp index 1c6b196c..52952e96 100644 --- a/include/fixed_containers/struct_view.hpp +++ b/include/fixed_containers/struct_view.hpp @@ -657,7 +657,7 @@ void for_each_field(const StructView& struct_view, { for (const auto& [path, path_properties] : struct_view.get_path_map_ref()) { - for_each_index_of_path(base_pointer, path, path_properties, std::forward(func)); + for_each_index_of_path(base_pointer, path, path_properties, func); } } diff --git a/include/fixed_containers/tuples.hpp b/include/fixed_containers/tuples.hpp index d91f1561..0a5fade1 100644 --- a/include/fixed_containers/tuples.hpp +++ b/include/fixed_containers/tuples.hpp @@ -28,7 +28,7 @@ template std::add_lvalue_reference_t>>>) constexpr void for_each_entry(Tuple&& tuple, Func&& func) { - std::apply([&func](auto&&... tuple_entries) { (std::forward(func)(tuple_entries), ...); }, + std::apply([&func](auto&&... tuple_entries) { (func(tuple_entries), ...); }, std::forward(tuple)); } @@ -43,7 +43,7 @@ constexpr void for_each_entry(Tuple&& tuple, Func&& func) for_each_entry(std::forward(tuple), [&func, index = static_cast(0)](auto& entry) mutable { - std::forward(func)(index, entry); + func(index, entry); ++index; }); } diff --git a/test/recursive_reflection_test.cpp b/test/recursive_reflection_test.cpp index 45a6bc09..013a920c 100644 --- a/test/recursive_reflection_test.cpp +++ b/test/recursive_reflection_test.cpp @@ -264,14 +264,12 @@ struct ReflectionHandler PostFunction&& post_fn, in_out chain) { - std::forward(pre_fn)(std::as_const(*chain), std::forward(instance)); + pre_fn(std::as_const(*chain), instance); chain->push_back("a_"); - recursive_reflection::for_each_path_dfs_helper(std::forward(instance).get_a(), - std::forward(pre_fn), - std::forward(post_fn), - fixed_containers::in_out{*chain}); + recursive_reflection::for_each_path_dfs_helper( + instance.get_a(), pre_fn, post_fn, fixed_containers::in_out{*chain}); chain->pop_back(); - std::forward(post_fn)(std::as_const(*chain), std::forward(instance)); + post_fn(std::as_const(*chain), instance); } }; diff --git a/test/struct_view_test.cpp b/test/struct_view_test.cpp index 5c84cf82..96ac7e66 100644 --- a/test/struct_view_test.cpp +++ b/test/struct_view_test.cpp @@ -811,14 +811,12 @@ struct ReflectionHandler PostFunction&& post_fn, in_out chain) { - std::forward(pre_fn)(std::as_const(*chain), std::forward(instance)); + pre_fn(std::as_const(*chain), instance); chain->push_back("a_"); - recursive_reflection::for_each_path_dfs_helper(std::forward(instance).get_a(), - std::forward(pre_fn), - std::forward(post_fn), - fixed_containers::in_out{*chain}); + recursive_reflection::for_each_path_dfs_helper( + instance.get_a(), pre_fn, post_fn, fixed_containers::in_out{*chain}); chain->pop_back(); - std::forward(post_fn)(std::as_const(*chain), std::forward(instance)); + post_fn(std::as_const(*chain), instance); } }; From cd4dfa5262370d3601675830bd819bbd30b71bde Mon Sep 17 00:00:00 2001 From: Alexander Karatarakis Date: Mon, 2 Dec 2024 17:00:27 -0800 Subject: [PATCH 12/15] Add mock_mutator() to instance counter helper type --- test/instance_counter.hpp | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/test/instance_counter.hpp b/test/instance_counter.hpp index 316263c3..de2b15ab 100644 --- a/test/instance_counter.hpp +++ b/test/instance_counter.hpp @@ -51,6 +51,10 @@ class InstanceCounterNonTrivialAssignment [[nodiscard]] int get() const { return value_; } + // To counter diagnostics like unused, variable can be const (`misc-const-correctness`), + // `performance-unnecessary-copy-initialization` + void mock_mutator() {} + bool operator==(const Self& other) const { return value_ == other.value_; } std::strong_ordering operator<=>(const Self& other) const { return value_ <=> other.value_; } }; @@ -92,6 +96,10 @@ class InstanceCounterTrivialAssignment [[nodiscard]] int get() const { return value_; } + // To counter diagnostics like unused, variable can be const (`misc-const-correctness`), + // `performance-unnecessary-copy-initialization` + void mock_mutator() {} + bool operator==(const Self& other) const { return value_ == other.value_; } std::strong_ordering operator<=>(const Self& other) const { return value_ <=> other.value_; } }; From 835030d11b29a239866612747c51911fd92e1bd4 Mon Sep 17 00:00:00 2001 From: Alexander Karatarakis Date: Mon, 2 Dec 2024 17:01:39 -0800 Subject: [PATCH 13/15] [clang-tidy-19] Enable performance-unnecessary-copy-initialization ``` error: local copy 'var2' of the variable 'var1' is never modified; consider avoiding the copy performance-unnecessary-copy-initialization,-warnings-as-errors] 1951 | const MapOfInstanceCounterType var2{var1}; | ^ | & ``` --- .clang-tidy | 1 - test/enum_map_test.cpp | 4 ++-- test/fixed_circular_deque_test.cpp | 4 ++-- test/fixed_deque_test.cpp | 4 ++-- test/fixed_list_test.cpp | 4 ++-- test/fixed_map_test.cpp | 4 ++-- test/fixed_unordered_map_test.cpp | 4 ++-- test/fixed_vector_test.cpp | 4 ++-- 8 files changed, 14 insertions(+), 15 deletions(-) diff --git a/.clang-tidy b/.clang-tidy index 4cabb590..1b1db571 100644 --- a/.clang-tidy +++ b/.clang-tidy @@ -74,7 +74,6 @@ Checks: > -bugprone-easily-swappable-parameters, -bugprone-switch-missing-default-case, -bugprone-unchecked-optional-access, - -performance-unnecessary-copy-initialization, -readability-container-size-empty, -bugprone-crtp-constructor-accessibility, -bugprone-return-const-ref-from-parameter, diff --git a/test/enum_map_test.cpp b/test/enum_map_test.cpp index 486a7beb..f53291ea 100644 --- a/test/enum_map_test.cpp +++ b/test/enum_map_test.cpp @@ -1846,8 +1846,8 @@ TYPED_TEST_P(EnumMapInstanceCheckFixture, EnumMapInstanceCheck) ASSERT_EQ(2, InstanceCounterType::counter); { // IMPORTANT SCOPE, don't remove. - const MapOfInstanceCounterType var2{var1}; - (void)var2; + MapOfInstanceCounterType var2{var1}; + var2.begin()->second.mock_mutator(); ASSERT_EQ(4, InstanceCounterType::counter); } ASSERT_EQ(2, InstanceCounterType::counter); diff --git a/test/fixed_circular_deque_test.cpp b/test/fixed_circular_deque_test.cpp index 669fbd01..654edc73 100644 --- a/test/fixed_circular_deque_test.cpp +++ b/test/fixed_circular_deque_test.cpp @@ -2543,8 +2543,8 @@ TYPED_TEST_P(FixedCircularDequeInstanceCheckFixture, FixedCircularDequeInstanceC ASSERT_EQ(2, InstanceCounterType::counter); { // IMPORTANT SCOPE, don't remove. - const CircularDequeOfInstanceCounterType var2{var1}; - (void)var2; + CircularDequeOfInstanceCounterType var2{var1}; + var2.back().mock_mutator(); ASSERT_EQ(4, InstanceCounterType::counter); } ASSERT_EQ(2, InstanceCounterType::counter); diff --git a/test/fixed_deque_test.cpp b/test/fixed_deque_test.cpp index a09df732..afb5f1eb 100644 --- a/test/fixed_deque_test.cpp +++ b/test/fixed_deque_test.cpp @@ -2305,8 +2305,8 @@ TYPED_TEST_P(FixedDequeInstanceCheckFixture, FixedDequeInstanceCheck) ASSERT_EQ(2, InstanceCounterType::counter); { // IMPORTANT SCOPE, don't remove. - const DequeOfInstanceCounterType var2{var1}; - (void)var2; + DequeOfInstanceCounterType var2{var1}; + var2.back().mock_mutator(); ASSERT_EQ(4, InstanceCounterType::counter); } ASSERT_EQ(2, InstanceCounterType::counter); diff --git a/test/fixed_list_test.cpp b/test/fixed_list_test.cpp index 0eec476b..dbfe526c 100644 --- a/test/fixed_list_test.cpp +++ b/test/fixed_list_test.cpp @@ -2200,8 +2200,8 @@ TYPED_TEST_P(FixedListInstanceCheckFixture, FixedListInstanceCheck) ASSERT_EQ(2, InstanceCounterType::counter); { // IMPORTANT SCOPE, don't remove. - const ListOfInstanceCounterType var2{var1}; - (void)var2; + ListOfInstanceCounterType var2{var1}; + var2.back().mock_mutator(); ASSERT_EQ(4, InstanceCounterType::counter); } ASSERT_EQ(2, InstanceCounterType::counter); diff --git a/test/fixed_map_test.cpp b/test/fixed_map_test.cpp index cf29ffa5..1d7b6b23 100644 --- a/test/fixed_map_test.cpp +++ b/test/fixed_map_test.cpp @@ -1948,8 +1948,8 @@ TYPED_TEST_P(FixedMapInstanceCheckFixture, FixedMapInstanceCheck) ASSERT_EQ(4, InstanceCounterType::counter); { // IMPORTANT SCOPE, don't remove. - const MapOfInstanceCounterType var2{var1}; - (void)var2; + MapOfInstanceCounterType var2{var1}; + var2.begin()->second.mock_mutator(); ASSERT_EQ(8, InstanceCounterType::counter); } ASSERT_EQ(4, InstanceCounterType::counter); diff --git a/test/fixed_unordered_map_test.cpp b/test/fixed_unordered_map_test.cpp index 273e3483..e373e2ed 100644 --- a/test/fixed_unordered_map_test.cpp +++ b/test/fixed_unordered_map_test.cpp @@ -1831,8 +1831,8 @@ TYPED_TEST_P(FixedUnorderedMapInstanceCheckFixture, FixedUnorderedMapInstanceChe ASSERT_EQ(4, InstanceCounterType::counter); { // IMPORTANT SCOPE, don't remove. - const MapOfInstanceCounterType var2{var1}; - (void)var2; + MapOfInstanceCounterType var2{var1}; + var2.begin()->second.mock_mutator(); ASSERT_EQ(8, InstanceCounterType::counter); } ASSERT_EQ(4, InstanceCounterType::counter); diff --git a/test/fixed_vector_test.cpp b/test/fixed_vector_test.cpp index 0c9c3a49..fecd4fdb 100644 --- a/test/fixed_vector_test.cpp +++ b/test/fixed_vector_test.cpp @@ -2195,8 +2195,8 @@ TYPED_TEST_P(FixedVectorInstanceCheckFixture, FixedVectorInstanceCheck) ASSERT_EQ(2, InstanceCounterType::counter); { // IMPORTANT SCOPE, don't remove. - const VectorOfInstanceCounterType var2{var1}; - (void)var2; + VectorOfInstanceCounterType var2{var1}; + var2.back().mock_mutator(); ASSERT_EQ(4, InstanceCounterType::counter); } ASSERT_EQ(2, InstanceCounterType::counter); From a49a642ef027d6720a340f560cee952b99bb31a1 Mon Sep 17 00:00:00 2001 From: Alexander Karatarakis Date: Mon, 2 Dec 2024 19:01:12 -0800 Subject: [PATCH 14/15] [OptionalReference] value_or() should not be callable with r-values Detected by clang-tidy-19's `bugprone-return-const-ref-from-parameter` --- .../fixed_containers/optional_reference.hpp | 7 +++++-- test/optional_reference_test.cpp | 20 +++++++++++++++++++ 2 files changed, 25 insertions(+), 2 deletions(-) diff --git a/include/fixed_containers/optional_reference.hpp b/include/fixed_containers/optional_reference.hpp index f4437ad1..db3e3584 100644 --- a/include/fixed_containers/optional_reference.hpp +++ b/include/fixed_containers/optional_reference.hpp @@ -7,6 +7,7 @@ #include #include #include +#include namespace fixed_containers { @@ -86,11 +87,13 @@ class OptionalReference return *val(); } - [[nodiscard]] constexpr reference value_or(reference default_value) const& + template + [[nodiscard]] constexpr reference value_or(U&& default_value) const& + requires(std::is_lvalue_reference_v) { if (!has_value()) { - return default_value; + return std::forward(default_value); } return *val(); diff --git a/test/optional_reference_test.cpp b/test/optional_reference_test.cpp index 93da0cec..2793bc29 100644 --- a/test/optional_reference_test.cpp +++ b/test/optional_reference_test.cpp @@ -97,6 +97,15 @@ TEST(OptionalReference, Value) } } +namespace +{ +template +constexpr bool value_or_is_callable_with_rvalue() +{ + return requires(T instance) { instance.value_or(Parameter{}); }; +} +} // namespace + TEST(OptionalReference, ValueOr) { { @@ -118,6 +127,17 @@ TEST(OptionalReference, ValueOr) const int& result = val1.value_or(fallback_value); EXPECT_EQ(5, result); } + { + /* + constexpr int ENTRY_1 = 5; + const OptionalReference val1(ENTRY_1); + const int& result = val1.value_or(77); // This should fail to compile + EXPECT_EQ(5, result); + */ + + static_assert(!value_or_is_callable_with_rvalue, int>(), + "`value_or() should not be callable with r-values"); + } } TEST(OptionalReference, DereferenceOperator) From 0298e75ec2a74ac85a6a4504a9c60ac7d9e2d838 Mon Sep 17 00:00:00 2001 From: Alexander Karatarakis Date: Mon, 2 Dec 2024 19:06:24 -0800 Subject: [PATCH 15/15] [clang-tidy-19] Enable bugprone-return-const-ref-from-parameter ``` error: returning a constant reference parameter may cause use-after-free when the parameter is constructed from a temporary [bugprone-return-const-ref-from-parameter,-warnings-as-errors] 122 | return value; | ^~~~~ ``` --- .clang-tidy | 1 - include/fixed_containers/optional_storage.hpp | 2 +- 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/.clang-tidy b/.clang-tidy index 1b1db571..828b7d5e 100644 --- a/.clang-tidy +++ b/.clang-tidy @@ -76,7 +76,6 @@ Checks: > -bugprone-unchecked-optional-access, -readability-container-size-empty, -bugprone-crtp-constructor-accessibility, - -bugprone-return-const-ref-from-parameter, # Turn all the warnings from the checks above into errors. WarningsAsErrors: "*" diff --git a/include/fixed_containers/optional_storage.hpp b/include/fixed_containers/optional_storage.hpp index dd96237c..ddfea536 100644 --- a/include/fixed_containers/optional_storage.hpp +++ b/include/fixed_containers/optional_storage.hpp @@ -119,7 +119,7 @@ constexpr auto& get(OptionalStorage& value) template constexpr T&& get(T&& value) { - return value; + return std::forward(value); } // "Transparent" here means there will be no wrapping for simple types.