Skip to content

Commit e8ca306

Browse files
authored
[ADT] Add a missing call to a unique_function destructor after move (#98747)
Right now immediately after moving the captured state, we 'disarm' the moved-from object's destructor by resetting the `RHS.CallbackAndInlineFlag`. This means that we never properly destroy the RHS object's captured state.
1 parent 87dd312 commit e8ca306

File tree

4 files changed

+29
-2
lines changed

4 files changed

+29
-2
lines changed

llvm/include/llvm/ADT/FunctionExtras.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -314,6 +314,7 @@ template <typename ReturnT, typename... ParamTs> class UniqueFunctionBase {
314314
// Non-trivial move, so dispatch to a type-erased implementation.
315315
getNonTrivialCallbacks()->MovePtr(getInlineStorage(),
316316
RHS.getInlineStorage());
317+
getNonTrivialCallbacks()->DestroyPtr(RHS.getInlineStorage());
317318
}
318319

319320
// Clear the old callback and inline flag to get back to as-if-null.

llvm/unittests/ADT/CountCopyAndMove.cpp

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,8 @@
1010

1111
using namespace llvm;
1212

13+
int CountCopyAndMove::DefaultConstructions = 0;
14+
int CountCopyAndMove::ValueConstructions = 0;
1315
int CountCopyAndMove::CopyConstructions = 0;
1416
int CountCopyAndMove::CopyAssignments = 0;
1517
int CountCopyAndMove::MoveConstructions = 0;

llvm/unittests/ADT/CountCopyAndMove.h

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,15 +12,17 @@
1212
namespace llvm {
1313

1414
struct CountCopyAndMove {
15+
static int DefaultConstructions;
16+
static int ValueConstructions;
1517
static int CopyConstructions;
1618
static int CopyAssignments;
1719
static int MoveConstructions;
1820
static int MoveAssignments;
1921
static int Destructions;
2022
int val;
2123

22-
CountCopyAndMove() = default;
23-
explicit CountCopyAndMove(int val) : val(val) {}
24+
CountCopyAndMove() { ++DefaultConstructions; }
25+
explicit CountCopyAndMove(int val) : val(val) { ++ValueConstructions; }
2426
CountCopyAndMove(const CountCopyAndMove &other) : val(other.val) {
2527
++CopyConstructions;
2628
}
@@ -40,13 +42,20 @@ struct CountCopyAndMove {
4042
~CountCopyAndMove() { ++Destructions; }
4143

4244
static void ResetCounts() {
45+
DefaultConstructions = 0;
46+
ValueConstructions = 0;
4347
CopyConstructions = 0;
4448
CopyAssignments = 0;
4549
MoveConstructions = 0;
4650
MoveAssignments = 0;
4751
Destructions = 0;
4852
}
4953

54+
static int TotalConstructions() {
55+
return DefaultConstructions + ValueConstructions + MoveConstructions +
56+
CopyConstructions;
57+
}
58+
5059
static int TotalCopies() { return CopyConstructions + CopyAssignments; }
5160

5261
static int TotalMoves() { return MoveConstructions + MoveAssignments; }

llvm/unittests/ADT/FunctionExtrasTest.cpp

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
//===----------------------------------------------------------------------===//
88

99
#include "llvm/ADT/FunctionExtras.h"
10+
#include "CountCopyAndMove.h"
1011
#include "gtest/gtest.h"
1112

1213
#include <memory>
@@ -329,4 +330,18 @@ TEST(UniqueFunctionTest, InlineStorageWorks) {
329330
UniqueFunctionWithInlineStorage(&UniqueFunctionWithInlineStorage);
330331
}
331332

333+
// Check that the moved-from captured state is properly destroyed during
334+
// move construction/assignment.
335+
TEST(UniqueFunctionTest, MovedFromStateIsDestroyedCorrectly) {
336+
CountCopyAndMove::ResetCounts();
337+
{
338+
unique_function<void()> CapturingFunction{
339+
[Counter = CountCopyAndMove{}] {}};
340+
unique_function<void()> CapturingFunctionMoved{
341+
std::move(CapturingFunction)};
342+
}
343+
EXPECT_EQ(CountCopyAndMove::TotalConstructions(),
344+
CountCopyAndMove::Destructions);
345+
}
346+
332347
} // anonymous namespace

0 commit comments

Comments
 (0)