Skip to content

Commit 8de2a79

Browse files
mysterymathmysterymath
committed
Address michaelrj-google review comments
1 parent f7a7520 commit 8de2a79

File tree

6 files changed

+50
-47
lines changed

6 files changed

+50
-47
lines changed

‎libc/src/__support/freelist_heap.h‎

Lines changed: 9 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@ extern "C" cpp::byte __llvm_libc_heap_limit;
2828
using cpp::optional;
2929
using cpp::span;
3030

31-
inline constexpr bool IsPow2(size_t x) { return x && (x & (x - 1)) == 0; }
31+
LIBC_INLINE constexpr bool IsPow2(size_t x) { return x && (x & (x - 1)) == 0; }
3232

3333
class FreeListHeap {
3434
public:
@@ -74,7 +74,7 @@ template <size_t BUFF_SIZE> class FreeListHeapBuffer : public FreeListHeap {
7474
cpp::byte buffer[BUFF_SIZE];
7575
};
7676

77-
inline void FreeListHeap::init() {
77+
LIBC_INLINE void FreeListHeap::init() {
7878
LIBC_ASSERT(!is_initialized && "duplicate initialization");
7979
auto result = Block<>::init(region());
8080
Block<> *block = *result;
@@ -83,7 +83,7 @@ inline void FreeListHeap::init() {
8383
is_initialized = true;
8484
}
8585

86-
inline void *FreeListHeap::allocate_impl(size_t alignment, size_t size) {
86+
LIBC_INLINE void *FreeListHeap::allocate_impl(size_t alignment, size_t size) {
8787
if (size == 0)
8888
return nullptr;
8989

@@ -111,11 +111,12 @@ inline void *FreeListHeap::allocate_impl(size_t alignment, size_t size) {
111111
return block_info.block->usable_space();
112112
}
113113

114-
inline void *FreeListHeap::allocate(size_t size) {
114+
LIBC_INLINE void *FreeListHeap::allocate(size_t size) {
115115
return allocate_impl(alignof(max_align_t), size);
116116
}
117117

118-
inline void *FreeListHeap::aligned_allocate(size_t alignment, size_t size) {
118+
LIBC_INLINE void *FreeListHeap::aligned_allocate(size_t alignment,
119+
size_t size) {
119120
// The alignment must be an integral power of two.
120121
if (!IsPow2(alignment))
121122
return nullptr;
@@ -127,7 +128,7 @@ inline void *FreeListHeap::aligned_allocate(size_t alignment, size_t size) {
127128
return allocate_impl(alignment, size);
128129
}
129130

130-
inline void FreeListHeap::free(void *ptr) {
131+
LIBC_INLINE void FreeListHeap::free(void *ptr) {
131132
cpp::byte *bytes = static_cast<cpp::byte *>(ptr);
132133

133134
LIBC_ASSERT(is_valid_ptr(bytes) && "Invalid pointer");
@@ -157,7 +158,7 @@ inline void FreeListHeap::free(void *ptr) {
157158

158159
// Follows constract of the C standard realloc() function
159160
// If ptr is free'd, will return nullptr.
160-
inline void *FreeListHeap::realloc(void *ptr, size_t size) {
161+
LIBC_INLINE void *FreeListHeap::realloc(void *ptr, size_t size) {
161162
if (size == 0) {
162163
free(ptr);
163164
return nullptr;
@@ -192,7 +193,7 @@ inline void *FreeListHeap::realloc(void *ptr, size_t size) {
192193
return new_ptr;
193194
}
194195

195-
inline void *FreeListHeap::calloc(size_t num, size_t size) {
196+
LIBC_INLINE void *FreeListHeap::calloc(size_t num, size_t size) {
196197
void *ptr = allocate(num * size);
197198
if (ptr != nullptr)
198199
LIBC_NAMESPACE::inline_memset(ptr, 0, num * size);

‎libc/src/__support/freetrie.h‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,4 @@
1-
//===-- Interface for freetrie
2-
//--------------------------------------------===//freetrie.h
1+
//===-- Interface for freetrie --------------------------------------------===//
32
//
43
// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
54
// See https://llvm.org/LICENSE.txt for license information.

‎libc/test/src/__support/freelist_heap_test.cpp‎

Lines changed: 16 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -13,9 +13,11 @@
1313
#include "src/string/memcpy.h"
1414
#include "test/UnitTest/Test.h"
1515

16-
namespace LIBC_NAMESPACE_DECL {
17-
1816
using LIBC_NAMESPACE::freelist_heap;
17+
using LIBC_NAMESPACE::FreeListHeap;
18+
using LIBC_NAMESPACE::FreeListHeapBuffer;
19+
using LIBC_NAMESPACE::cpp::byte;
20+
using LIBC_NAMESPACE::cpp::span;
1921

2022
// Similar to `LlvmLibcBlockTest` in block_test.cpp, we'd like to run the same
2123
// tests independently for different parameters. In this case, we'd like to test
@@ -28,7 +30,8 @@ using LIBC_NAMESPACE::freelist_heap;
2830
// made in tests leak and aren't free'd. This is fine for the purposes of this
2931
// test file.
3032
#define TEST_FOR_EACH_ALLOCATOR(TestCase, BufferSize) \
31-
class LlvmLibcFreeListHeapTest##TestCase : public testing::Test { \
33+
class LlvmLibcFreeListHeapTest##TestCase \
34+
: public LIBC_NAMESPACE::testing::Test { \
3235
public: \
3336
FreeListHeapBuffer<BufferSize> fake_global_buffer; \
3437
void SetUp() override { \
@@ -38,8 +41,7 @@ using LIBC_NAMESPACE::freelist_heap;
3841
void RunTest(FreeListHeap &allocator, [[maybe_unused]] size_t N); \
3942
}; \
4043
TEST_F(LlvmLibcFreeListHeapTest##TestCase, TestCase) { \
41-
alignas(FreeListHeap::BlockType) \
42-
cpp::byte buf[BufferSize] = {cpp::byte(0)}; \
44+
alignas(FreeListHeap::BlockType) byte buf[BufferSize] = {byte(0)}; \
4345
FreeListHeap allocator(buf); \
4446
RunTest(allocator, BufferSize); \
4547
RunTest(*freelist_heap, freelist_heap->region().size()); \
@@ -92,7 +94,7 @@ TEST_FOR_EACH_ALLOCATOR(ReturnsNullWhenAllocationTooLarge, 2048) {
9294
// is used for other test cases and we don't explicitly free them.
9395
TEST(LlvmLibcFreeListHeap, ReturnsNullWhenFull) {
9496
constexpr size_t N = 2048;
95-
alignas(FreeListHeap::BlockType) cpp::byte buf[N] = {cpp::byte(0)};
97+
alignas(FreeListHeap::BlockType) byte buf[N] = {byte(0)};
9698

9799
FreeListHeap allocator(buf);
98100

@@ -134,9 +136,9 @@ TEST_FOR_EACH_ALLOCATOR(ReallocHasSameContent, 2048) {
134136
constexpr size_t ALLOC_SIZE = sizeof(int);
135137
constexpr size_t kNewAllocSize = sizeof(int) * 2;
136138
// Data inside the allocated block.
137-
cpp::byte data1[ALLOC_SIZE];
139+
byte data1[ALLOC_SIZE];
138140
// Data inside the reallocated block.
139-
cpp::byte data2[ALLOC_SIZE];
141+
byte data2[ALLOC_SIZE];
140142

141143
int *ptr1 = reinterpret_cast<int *>(allocator.allocate(ALLOC_SIZE));
142144
*ptr1 = 42;
@@ -188,10 +190,9 @@ TEST_FOR_EACH_ALLOCATOR(CanCalloc, 2048) {
188190
constexpr size_t ALLOC_SIZE = 128;
189191
constexpr size_t NUM = 4;
190192
constexpr int size = NUM * ALLOC_SIZE;
191-
constexpr cpp::byte zero{0};
193+
constexpr byte zero{0};
192194

193-
cpp::byte *ptr1 =
194-
reinterpret_cast<cpp::byte *>(allocator.calloc(NUM, ALLOC_SIZE));
195+
byte *ptr1 = reinterpret_cast<byte *>(allocator.calloc(NUM, ALLOC_SIZE));
195196

196197
// calloc'd content is zero.
197198
for (int i = 0; i < size; i++) {
@@ -203,10 +204,9 @@ TEST_FOR_EACH_ALLOCATOR(CanCallocWeirdSize, 2048) {
203204
constexpr size_t ALLOC_SIZE = 143;
204205
constexpr size_t NUM = 3;
205206
constexpr int size = NUM * ALLOC_SIZE;
206-
constexpr cpp::byte zero{0};
207+
constexpr byte zero{0};
207208

208-
cpp::byte *ptr1 =
209-
reinterpret_cast<cpp::byte *>(allocator.calloc(NUM, ALLOC_SIZE));
209+
byte *ptr1 = reinterpret_cast<byte *>(allocator.calloc(NUM, ALLOC_SIZE));
210210

211211
// calloc'd content is zero.
212212
for (int i = 0; i < size; i++) {
@@ -247,11 +247,11 @@ TEST_FOR_EACH_ALLOCATOR(AlignedAlloc, 2048) {
247247
TEST(LlvmLibcFreeListHeap, AlignedAllocOnlyBlockTypeAligned) {
248248
constexpr size_t BUFFER_SIZE = 4096;
249249
constexpr size_t BUFFER_ALIGNMENT = alignof(FreeListHeap::BlockType) * 2;
250-
alignas(BUFFER_ALIGNMENT) cpp::byte buf[BUFFER_SIZE] = {cpp::byte(0)};
250+
alignas(BUFFER_ALIGNMENT) byte buf[BUFFER_SIZE] = {byte(0)};
251251

252252
// Ensure the underlying buffer is at most aligned to the block type.
253253
FreeListHeap allocator(
254-
span<cpp::byte>(buf).subspan(alignof(FreeListHeap::BlockType)));
254+
span<byte>(buf).subspan(alignof(FreeListHeap::BlockType)));
255255

256256
constexpr size_t ALIGNMENTS[] = {1, 2, 4, 8, 16, 32, 64, 128, 256};
257257
constexpr size_t SIZE_SCALES[] = {1, 2, 3, 4, 5};
@@ -289,5 +289,3 @@ TEST_FOR_EACH_ALLOCATOR(InvalidAlignedAllocAlignment, 2048) {
289289
ptr = allocator.aligned_allocate(0, 8);
290290
EXPECT_EQ(ptr, static_cast<void *>(nullptr));
291291
}
292-
293-
} // namespace LIBC_NAMESPACE_DECL

‎libc/test/src/__support/freelist_test.cpp‎

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -11,15 +11,18 @@
1111
#include "src/__support/freelist.h"
1212
#include "test/UnitTest/Test.h"
1313

14-
namespace LIBC_NAMESPACE_DECL {
14+
using LIBC_NAMESPACE::Block;
15+
using LIBC_NAMESPACE::FreeList;
16+
using LIBC_NAMESPACE::cpp::byte;
17+
using LIBC_NAMESPACE::cpp::optional;
1518

1619
TEST(LlvmLibcFreeList, FreeList) {
17-
cpp::byte mem1[1024];
20+
byte mem1[1024];
1821
optional<Block<> *> maybeBlock = Block<>::init(mem1);
1922
ASSERT_TRUE(maybeBlock.has_value());
2023
Block<> *block1 = *maybeBlock;
2124

22-
cpp::byte mem2[1024];
25+
byte mem2[1024];
2326
maybeBlock = Block<>::init(mem2);
2427
ASSERT_TRUE(maybeBlock.has_value());
2528
Block<> *block2 = *maybeBlock;
@@ -46,5 +49,3 @@ TEST(LlvmLibcFreeList, FreeList) {
4649
list.pop();
4750
ASSERT_TRUE(list.empty());
4851
}
49-
50-
} // namespace LIBC_NAMESPACE_DECL

‎libc/test/src/__support/freestore_test.cpp‎

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -11,11 +11,16 @@
1111
#include "src/__support/freestore.h"
1212
#include "test/UnitTest/Test.h"
1313

14-
namespace LIBC_NAMESPACE_DECL {
14+
using LIBC_NAMESPACE::Block;
15+
using LIBC_NAMESPACE::FreeList;
16+
using LIBC_NAMESPACE::FreeStore;
17+
using LIBC_NAMESPACE::FreeTrie;
18+
using LIBC_NAMESPACE::cpp::byte;
19+
using LIBC_NAMESPACE::cpp::optional;
1520

1621
// Inserting or removing blocks too small to be tracked does nothing.
1722
TEST(LlvmLibcFreeStore, TooSmall) {
18-
cpp::byte mem[1024];
23+
byte mem[1024];
1924
optional<Block<> *> maybeBlock = Block<>::init(mem);
2025
ASSERT_TRUE(maybeBlock.has_value());
2126
Block<> *too_small = *maybeBlock;
@@ -33,7 +38,7 @@ TEST(LlvmLibcFreeStore, TooSmall) {
3338
}
3439

3540
TEST(LlvmLibcFreeStore, RemoveBestFit) {
36-
cpp::byte mem[1024];
41+
byte mem[1024];
3742
optional<Block<> *> maybeBlock = Block<>::init(mem);
3843
ASSERT_TRUE(maybeBlock.has_value());
3944

@@ -72,7 +77,7 @@ TEST(LlvmLibcFreeStore, RemoveBestFit) {
7277
}
7378

7479
TEST(LlvmLibcFreeStore, Remove) {
75-
cpp::byte mem[1024];
80+
byte mem[1024];
7681
optional<Block<> *> maybeBlock = Block<>::init(mem);
7782
ASSERT_TRUE(maybeBlock.has_value());
7883

@@ -94,5 +99,3 @@ TEST(LlvmLibcFreeStore, Remove) {
9499
ASSERT_EQ(store.remove_best_fit(small->inner_size()),
95100
static_cast<Block<> *>(nullptr));
96101
}
97-
98-
} // namespace LIBC_NAMESPACE_DECL

‎libc/test/src/__support/freetrie_test.cpp‎

Lines changed: 9 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -11,13 +11,16 @@
1111
#include "src/__support/freetrie.h"
1212
#include "test/UnitTest/Test.h"
1313

14-
namespace LIBC_NAMESPACE_DECL {
14+
using LIBC_NAMESPACE::Block;
15+
using LIBC_NAMESPACE::FreeTrie;
16+
using LIBC_NAMESPACE::cpp::byte;
17+
using LIBC_NAMESPACE::cpp::optional;
1518

1619
TEST(LlvmLibcFreeTrie, FindBestFitRoot) {
1720
FreeTrie trie({0, 4096});
1821
EXPECT_EQ(trie.find_best_fit(123), static_cast<FreeTrie::Node *>(nullptr));
1922

20-
cpp::byte mem[1024];
23+
byte mem[1024];
2124
optional<Block<> *> maybeBlock = Block<>::init(mem);
2225
ASSERT_TRUE(maybeBlock.has_value());
2326
Block<> *block = *maybeBlock;
@@ -33,7 +36,7 @@ TEST(LlvmLibcFreeTrie, FindBestFitRoot) {
3336
}
3437

3538
TEST(LlvmLibcFreeTrie, FindBestFitLower) {
36-
cpp::byte mem[4096];
39+
byte mem[4096];
3740
optional<Block<> *> maybeBlock = Block<>::init(mem);
3841
ASSERT_TRUE(maybeBlock.has_value());
3942
Block<> *lower = *maybeBlock;
@@ -49,7 +52,7 @@ TEST(LlvmLibcFreeTrie, FindBestFitLower) {
4952
}
5053

5154
TEST(LlvmLibcFreeTrie, FindBestFitUpper) {
52-
cpp::byte mem[4096];
55+
byte mem[4096];
5356
optional<Block<> *> maybeBlock = Block<>::init(mem);
5457
ASSERT_TRUE(maybeBlock.has_value());
5558
Block<> *root = *maybeBlock;
@@ -67,7 +70,7 @@ TEST(LlvmLibcFreeTrie, FindBestFitUpper) {
6770
}
6871

6972
TEST(LlvmLibcFreeTrie, FindBestFitLowerAndUpper) {
70-
cpp::byte mem[4096];
73+
byte mem[4096];
7174
optional<Block<> *> maybeBlock = Block<>::init(mem);
7275
ASSERT_TRUE(maybeBlock.has_value());
7376
Block<> *root = *maybeBlock;
@@ -91,7 +94,7 @@ TEST(LlvmLibcFreeTrie, FindBestFitLowerAndUpper) {
9194
}
9295

9396
TEST(LlvmLibcFreeTrie, Remove) {
94-
cpp::byte mem[4096];
97+
byte mem[4096];
9598
optional<Block<> *> maybeBlock = Block<>::init(mem);
9699
ASSERT_TRUE(maybeBlock.has_value());
97100
Block<> *small1 = *maybeBlock;
@@ -120,5 +123,3 @@ TEST(LlvmLibcFreeTrie, Remove) {
120123
trie.remove(trie.find_best_fit(small1->inner_size()));
121124
EXPECT_EQ(trie.find_best_fit(large->inner_size())->block(), large);
122125
}
123-
124-
} // namespace LIBC_NAMESPACE_DECL

0 commit comments

Comments
 (0)