Skip to content

Commit cc97853

Browse files
authored
Fix ArenaVector::swap (#7115)
This was never right for over a decade, and just never used I suppose... it should have been called "take" since it grabbed data from the other item and then set that other item to empty. Fix it so it swaps properly.
1 parent f7afec9 commit cc97853

3 files changed

Lines changed: 43 additions & 6 deletions

File tree

src/mixed_arena.h

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -263,12 +263,9 @@ template<typename SubType, typename T> class ArenaVectorBase {
263263
void operator=(SubType& other) { set(other); }
264264

265265
void swap(SubType& other) {
266-
data = other.data;
267-
usedElements = other.usedElements;
268-
allocatedElements = other.allocatedElements;
269-
270-
other.data = nullptr;
271-
other.usedElements = other.allocatedElements = 0;
266+
std::swap(data, other.data);
267+
std::swap(usedElements, other.usedElements);
268+
std::swap(allocatedElements, other.allocatedElements);
272269
}
273270

274271
// iteration

test/gtest/CMakeLists.txt

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ include_directories(../../third_party/googletest/googletest/include)
22
include_directories(../../src/wasm)
33

44
set(unittest_SOURCES
5+
arena.cpp
56
binary-reader.cpp
67
cfg.cpp
78
dfa_minimization.cpp

test/gtest/arena.cpp

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
1+
#include "mixed_arena.h"
2+
#include "gtest/gtest.h"
3+
4+
using ArenaTest = ::testing::Test;
5+
6+
TEST_F(ArenaTest, Swap) {
7+
MixedArena arena;
8+
9+
ArenaVector<int> a(arena);
10+
a.push_back(10);
11+
a.push_back(20);
12+
13+
ArenaVector<int> b(arena);
14+
15+
EXPECT_EQ(a.size(), 2U);
16+
EXPECT_EQ(b.size(), 0U);
17+
18+
a.swap(b);
19+
20+
EXPECT_EQ(a.size(), 0U);
21+
EXPECT_EQ(b.size(), 2U);
22+
23+
a.swap(b);
24+
25+
EXPECT_EQ(a.size(), 2U);
26+
EXPECT_EQ(b.size(), 0U);
27+
28+
// Now reverse a and b. The swap should be the same.
29+
30+
b.swap(a);
31+
32+
EXPECT_EQ(a.size(), 0U);
33+
EXPECT_EQ(b.size(), 2U);
34+
35+
b.swap(a);
36+
37+
EXPECT_EQ(a.size(), 2U);
38+
EXPECT_EQ(b.size(), 0U);
39+
}

0 commit comments

Comments
 (0)