From 8da6521bfb6f15d26f1d245154d5ad46921a2cec Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Tue, 5 May 2026 17:38:55 +0200 Subject: [PATCH 1/3] Fix self-move assignment warnings in tests GCC 14's -Werror=self-move detects self-move assignments. These occur in tests that intentionally verify self-move safety of various types. Solution: use pointer indirection to hide self-moves from compiler analysis: auto* ptr = &obj; obj = std::move(*ptr); This preserves the runtime behavior (self-move) while preventing the compile-time warning. Applied to test files for: DynamicArray, intrusive_list, Promise, AllocatorWrapper, TypeErasurePointer, Scope, and LockFile. Co-Authored-By: Claude Sonnet 4.5 Signed-off-by: Leonardo Rossetti --- .../future/base_interruptible_promise_test.cpp | 3 ++- score/containers/dynamic_array_test.cpp | 5 +++-- score/containers/intrusive_list_test.cpp | 12 ++++++++---- .../allocator_aware_type_erasure_pointer_test.cpp | 3 ++- .../details/allocator_wrapper_test.cpp | 3 ++- .../language/safecpp/scoped_function/scope_test.cpp | 3 ++- score/memory/shared/lock_file_test.cpp | 3 ++- 7 files changed, 21 insertions(+), 11 deletions(-) diff --git a/score/concurrency/future/base_interruptible_promise_test.cpp b/score/concurrency/future/base_interruptible_promise_test.cpp index 912caf20d..7a8ce4f2e 100644 --- a/score/concurrency/future/base_interruptible_promise_test.cpp +++ b/score/concurrency/future/base_interruptible_promise_test.cpp @@ -179,7 +179,8 @@ TYPED_TEST(BaseInterruptiblePromiseTest, MoveAssignmentAlsoMovesFutureRetrievalS TYPED_TEST(BaseInterruptiblePromiseTest, MoveAssignmentToSelf) { BaseInterruptiblePromise moved_to_promise{}; - moved_to_promise = std::move(moved_to_promise); + auto* promise_ptr = &moved_to_promise; + moved_to_promise = std::move(*promise_ptr); ASSERT_TRUE(moved_to_promise.GetInterruptibleFuture().has_value()); } diff --git a/score/containers/dynamic_array_test.cpp b/score/containers/dynamic_array_test.cpp index 9c778206b..fd79087ad 100644 --- a/score/containers/dynamic_array_test.cpp +++ b/score/containers/dynamic_array_test.cpp @@ -263,8 +263,9 @@ TYPED_TEST(DynamicArrayTestFixture, SelfMoveAssign) DynamicArray unit{kNonEmptyArraySize, GetAllocator(this->memory_resource_)}; - // when doing a self-move-assign - unit = std::move(unit); + // when doing a self-move-assign (use pointer indirection to avoid compiler warning) + auto* unit_ptr = &unit; + unit = std::move(*unit_ptr); // expect, that the unit afterward still has the same size EXPECT_EQ(unit.size(), kNonEmptyArraySize); diff --git a/score/containers/intrusive_list_test.cpp b/score/containers/intrusive_list_test.cpp index 9244d5756..c3305d040 100644 --- a/score/containers/intrusive_list_test.cpp +++ b/score/containers/intrusive_list_test.cpp @@ -552,7 +552,8 @@ TEST(IntrusiveList, MoveAssignmentTest) // NOLINTBEGIN(bugprone-use-after-move): testing correctness of implementation List list; - list = std::move(list); + auto* list_ptr = &list; + list = std::move(*list_ptr); CheckEmpty(list); List list_from; @@ -562,7 +563,8 @@ TEST(IntrusiveList, MoveAssignmentTest) list = std::move(list_from); CheckEmpty(list_from); CheckNonEmpty(list, 1, front_back, front_back); - list = std::move(list); + list_ptr = &list; + list = std::move(*list_ptr); CheckNonEmpty(list, 1, front_back, front_back); ListElement front; @@ -572,7 +574,8 @@ TEST(IntrusiveList, MoveAssignmentTest) list = std::move(list_from); CheckEmpty(list_from); CheckNonEmpty(list, 2, front, back); - list = std::move(list); + list_ptr = &list; + list = std::move(*list_ptr); CheckNonEmpty(list, 2, front, back); constexpr std::size_t num_elements = 6; @@ -581,7 +584,8 @@ TEST(IntrusiveList, MoveAssignmentTest) list = std::move(list_from); CheckEmpty(list_from); CheckNonEmpty(list, num_elements, elements[0], elements[num_elements - 1]); - list = std::move(list); + list_ptr = &list; + list = std::move(*list_ptr); CheckNonEmpty(list, num_elements, elements[0], elements[num_elements - 1]); list.clear(); diff --git a/score/language/safecpp/scoped_function/details/allocator_aware_type_erasure_pointer_test.cpp b/score/language/safecpp/scoped_function/details/allocator_aware_type_erasure_pointer_test.cpp index ea2ed7765..4f3fa0c66 100644 --- a/score/language/safecpp/scoped_function/details/allocator_aware_type_erasure_pointer_test.cpp +++ b/score/language/safecpp/scoped_function/details/allocator_aware_type_erasure_pointer_test.cpp @@ -272,7 +272,8 @@ TEST_F(AllocatorAwareTypeErasurePointerTest, CanMoveAssignSelfWithoutAdverseEffe DISABLE_WARNING_PUSH DISABLE_WARNING_SELF_MOVE - target = std::move(target); + auto* target_ptr = ⌖ + target = std::move(*target_ptr); DISABLE_WARNING_POP diff --git a/score/language/safecpp/scoped_function/details/allocator_wrapper_test.cpp b/score/language/safecpp/scoped_function/details/allocator_wrapper_test.cpp index 975f281e1..186233245 100644 --- a/score/language/safecpp/scoped_function/details/allocator_wrapper_test.cpp +++ b/score/language/safecpp/scoped_function/details/allocator_wrapper_test.cpp @@ -109,7 +109,8 @@ TEST_F(AllocatorWrapperTest, CorrectlyHandlesSelfMoveAssignment) DISABLE_WARNING_PUSH DISABLE_WARNING_SELF_MOVE - allocator_wrapper = std::move(allocator_wrapper); + auto* wrapper_ptr = &allocator_wrapper; + allocator_wrapper = std::move(*wrapper_ptr); DISABLE_WARNING_POP diff --git a/score/language/safecpp/scoped_function/scope_test.cpp b/score/language/safecpp/scoped_function/scope_test.cpp index 7a9d7d00c..ca9cbcbda 100644 --- a/score/language/safecpp/scoped_function/scope_test.cpp +++ b/score/language/safecpp/scoped_function/scope_test.cpp @@ -126,7 +126,8 @@ TYPED_TEST(ScopeTest, CanMoveAssignToItselfWithNoAdverseEffects) DISABLE_WARNING_PUSH DISABLE_WARNING_SELF_MOVE - scope = std::move(scope); + auto* scope_ptr = &scope; + scope = std::move(*scope_ptr); DISABLE_WARNING_POP diff --git a/score/memory/shared/lock_file_test.cpp b/score/memory/shared/lock_file_test.cpp index a617c5eb2..8eaf0128a 100644 --- a/score/memory/shared/lock_file_test.cpp +++ b/score/memory/shared/lock_file_test.cpp @@ -554,7 +554,8 @@ TEST_F(LockFileMoveFixture, LockFileShouldNotRemoveFileWhenMoveAssigningToItself ASSERT_TRUE(lock_file_result.has_value()); // When we move assign the lock file to itself - lock_file_result.value() = std::move(lock_file_result.value()); + auto* lock_file_ptr = &lock_file_result.value(); + lock_file_result.value() = std::move(*lock_file_ptr); // Then the lock file won't be destroyed EXPECT_FALSE(close_called); From 12ee67dbae41208481ab673e28b69a03d4825cb2 Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Tue, 5 May 2026 17:39:09 +0200 Subject: [PATCH 2/3] Fix pessimizing-move warnings in future/promise tests GCC 14's -Werror=pessimizing-move detects std::move() on return values, which prevents copy elision and forces unnecessary move operations. Solution: remove std::move() from temporary return values and let the compiler apply copy elision optimization. Applied to 7 instances across: - base_interruptible_future_test.cpp (4 instances) - interruptible_shared_future_test.cpp (2 instances) - task_result_test.cpp (1 instance) Co-Authored-By: Claude Sonnet 4.5 Signed-off-by: Leonardo Rossetti --- .../concurrency/future/base_interruptible_future_test.cpp | 8 ++++---- .../future/interruptible_shared_future_test.cpp | 4 ++-- score/concurrency/task_result_test.cpp | 2 +- 3 files changed, 7 insertions(+), 7 deletions(-) diff --git a/score/concurrency/future/base_interruptible_future_test.cpp b/score/concurrency/future/base_interruptible_future_test.cpp index ffbb71411..b20385c12 100644 --- a/score/concurrency/future/base_interruptible_future_test.cpp +++ b/score/concurrency/future/base_interruptible_future_test.cpp @@ -316,7 +316,7 @@ TYPED_TEST(BaseInterruptibleFutureTest, WaitReturnsOnlyAfterValueWasSet) BaseInterruptibleFutureTest::SetPromise(this->promise_); - auto expected = std::move(async_future.get()); + auto expected = async_future.get(); EXPECT_TRUE(expected.has_value()); } @@ -351,7 +351,7 @@ TYPED_TEST(BaseInterruptibleFutureTest, WaitForReturnsTimeoutErrorWhenValueWasNo return this->future_->WaitFor(stop_token, TIMEOUT); }); - auto expected = std::move(async_future.get()); + auto expected = async_future.get(); ASSERT_FALSE(expected.has_value()); EXPECT_EQ(expected.error(), Error::kTimeout); } @@ -388,7 +388,7 @@ TYPED_TEST(BaseInterruptibleFutureTest, WaitForReturnsWhenValueIsSet) }); BaseInterruptibleFutureTest::SetPromise(this->promise_); - auto expected = std::move(async_future.get()); + auto expected = async_future.get(); EXPECT_TRUE(expected.has_value()); } @@ -400,7 +400,7 @@ TYPED_TEST(BaseInterruptibleFutureTest, WaitUntilReturnsTimeoutErrorWhenValueWas return this->future_->WaitUntil(stop_token, std::chrono::steady_clock::now() + TIMEOUT); }); - auto expected = std::move(async_future.get()); + auto expected = async_future.get(); ASSERT_FALSE(expected.has_value()); EXPECT_EQ(expected.error(), Error::kTimeout); } diff --git a/score/concurrency/future/interruptible_shared_future_test.cpp b/score/concurrency/future/interruptible_shared_future_test.cpp index 7a918a089..b468da10b 100644 --- a/score/concurrency/future/interruptible_shared_future_test.cpp +++ b/score/concurrency/future/interruptible_shared_future_test.cpp @@ -349,7 +349,7 @@ TYPED_TEST(InterruptibleSharedFutureTest, GetReturnsOnlyAfterValueWasSet) InterruptibleSharedFutureTest::SetPromise(this->promise_); - auto expected = std::move(async_future.get()); + auto expected = async_future.get(); InterruptibleSharedFutureTest::ExpectCorrectValue(expected); } @@ -358,7 +358,7 @@ TYPED_TEST(InterruptibleSharedFutureTest, GetReturnsErrorWhenSet) this->promise_.SetError(Error::kFutureAlreadyRetrieved); score::cpp::stop_token stop_token{}; - auto expected = std::move(this->future_.Get(stop_token)); + auto expected = this->future_.Get(stop_token); ASSERT_FALSE(expected.has_value()); EXPECT_EQ(expected.error(), Error::kFutureAlreadyRetrieved); } diff --git a/score/concurrency/task_result_test.cpp b/score/concurrency/task_result_test.cpp index 0ce95e810..5eac35927 100644 --- a/score/concurrency/task_result_test.cpp +++ b/score/concurrency/task_result_test.cpp @@ -157,7 +157,7 @@ class TaskResultContinuationTest : public ::testing::Test public: typename T::TaskType Make() { - return std::move(T::Make(promise_)); + return T::Make(promise_); } void ExpectCallNTimes(std::uint16_t n) From 1cdb9fc0582f087ec12668955f31d7347c344fcf Mon Sep 17 00:00:00 2001 From: Leonardo Rossetti Date: Thu, 18 Jun 2026 05:29:41 -0300 Subject: [PATCH 3/3] fix json parser tests dangling-reference for gcc14 Signed-off-by: Leonardo Rossetti --- .../json/internal/parser/parsers_test_suite.h | 42 +++++++++++-------- 1 file changed, 25 insertions(+), 17 deletions(-) diff --git a/score/json/internal/parser/parsers_test_suite.h b/score/json/internal/parser/parsers_test_suite.h index 61ff250c3..01dc9b6bf 100644 --- a/score/json/internal/parser/parsers_test_suite.h +++ b/score/json/internal/parser/parsers_test_suite.h @@ -29,7 +29,7 @@ namespace { template ::value, bool> = true> -const T& GetValueOfObject(const score::json::Any& any, const std::string& key) +const T GetValueOfObject(const score::json::Any& any, const std::string& key) { return any.As().value().get().at(key).As().value().get(); } @@ -114,7 +114,7 @@ TYPED_TEST_P(ParserTest, CanParseObjectString) auto root = TypeParam::FromBuffer(buffer_simple_json); // When reading a key of an object that is interpreted as std::string - auto& value = GetValueOfObject(root.value(), "color"); + auto value = GetValueOfObject(root.value(), "color"); // Then the correct value is returned EXPECT_EQ(value, "gold"); @@ -133,7 +133,7 @@ TYPED_TEST_P(ParserTest, CanParseObjectNull) auto root = TypeParam::FromBuffer(buffer_simple_json); // When reading a key of an object that is interpreted as Null - auto& value = GetValueOfObject(root.value(), "null"); + auto value = GetValueOfObject(root.value(), "null"); // Then the correct value is returned EXPECT_EQ(value, Null{}); @@ -172,7 +172,7 @@ TYPED_TEST_P(ParserTest, CanParseObjectFloatingPointNumber) // When reading a key of an object that is interpreted as floating point number auto float_value = GetValueOfObject(root.value(), "float"); - double double_value = GetValueOfObject(root.value(), "double"); + auto double_value = GetValueOfObject(root.value(), "double"); auto double_as_float_value = root->template As().value().get().at("double").template As().has_value(); @@ -193,14 +193,17 @@ TYPED_TEST_P(ParserTest, CanParseObjectInObject) // Given a simple JSON buffer auto root = TypeParam::FromBuffer(buffer_simple_json); - + // When reading a key of an object that is interpreted as number - auto& value = GetValueOfObject(root.value(), "object"); - + auto value = root.value().template As().value().get() + .at("object").template As().value().get() + .at("a").template As().value().get(); + // Then the correct value is returned - EXPECT_EQ(value.at("a").template As().value().get(), "b"); + EXPECT_EQ(value, "b"); } + TYPED_TEST_P(ParserTest, CanParseListInObject) { this->RecordProperty("Verifies", "5310867"); @@ -212,14 +215,15 @@ TYPED_TEST_P(ParserTest, CanParseListInObject) // Given a simple JSON buffer auto root = TypeParam::FromBuffer(buffer_simple_json); - + // When reading a key of an object that is interpreted as number - auto& value = GetValueOfObject(*root, "list"); + auto* root_map = &root.value().template As().value().get(); + auto* value = &root_map->at("list").template As().value().get(); // Then the correct value is returned - EXPECT_EQ(value[0].template As().value().get(), "first"); - EXPECT_EQ(value[1].template As().value(), 2UL); - EXPECT_EQ(value[2].template As().value().get(), "third"); + EXPECT_EQ((*value)[0].template As().value().get(), "first"); + EXPECT_EQ((*value)[1].template As().value(), 2UL); + EXPECT_EQ((*value)[2].template As().value().get(), "third"); } TYPED_TEST_P(ParserTest, CanParseObjectInObjectAndIterateOverKeys) @@ -257,14 +261,18 @@ TYPED_TEST_P(ParserTest, CanParseObjectInObjectAndIterateOverKeys) } )"}; auto root = TypeParam::FromBuffer(buffer); + auto* root_map = &root.value().template As().value().get(); + auto* storage_list = &root_map->at("storage_list").template As().value().get(); // When iterating over the unknown keys - const auto& storage_list = root.value().template As().value().get()["storage_list"]; std::vector collected_paths{}; - for (const auto& element : storage_list.template As().value().get()) + for (const auto& element : *storage_list) { + auto* inner_obj = &element.second.template As().value().get(); + collected_paths.push_back( - element.second.template As().value().get().at("path").template As().value().get()); + inner_obj->at("path").template As().value().get() + ); } // Then we can store them and thus also access them @@ -378,7 +386,7 @@ TYPED_TEST_P(ParserTest, ParsingFromFileWorks) EXPECT_TRUE(root.has_value()); // When reading a key of an object that is interpreted as bool - auto value = GetValueOfObject(root.value(), "boolean"); + bool value = GetValueOfObject(root.value(), "boolean"); // Then the correct value is returned EXPECT_EQ(value, true);