[libc-commits] [libc] [libc] Use Block as a byte-backed proxy (PR #201001)

[Daniel Thornburgh via libc-commits]

================
@@ -90,48 +98,69 @@ using cpp::optional;
 ///
 /// The next offset of a block matches the previous offset of its next block.
 /// The first block in a list is denoted by having a previous offset of `0`.
-class Block {
-  // Masks for the contents of the next_ field.
+class BlockRef {
+  // Masks for the contents of the next field.
   static constexpr size_t PREV_FREE_MASK = 1 << 0;
   static constexpr size_t LAST_MASK = 1 << 1;
   static constexpr size_t SIZE_MASK = ~(PREV_FREE_MASK | LAST_MASK);
 
+  // Header field offsets. The previous offset is only meaningful when this
+  // block's PREV_FREE_MASK bit is set in the next field.
+  static constexpr size_t PREV_OFFSET = 0;
+  static constexpr size_t NEXT_OFFSET = sizeof(size_t);
+
 public:
+  static constexpr size_t HEADER_SIZE = 2 * sizeof(size_t);
+
   // To ensure block sizes have two lower unused bits, ensure usable space is
   // always aligned to at least 4 bytes. (The distances between usable spaces,
   // the outer size, is then always also 4-aligned.)
   static constexpr size_t MIN_ALIGN = cpp::max(size_t{4}, alignof(max_align_t));
-  // No copy or move.
-  Block(const Block &other) = delete;
-  Block &operator=(const Block &other) = delete;
+
+  LIBC_INLINE constexpr BlockRef() = default;
+  LIBC_INLINE explicit constexpr BlockRef(cpp::byte *ptr) : self(ptr) {}
+  LIBC_INLINE explicit constexpr operator bool() const {
+    return self != nullptr;
+  }
+  LIBC_INLINE constexpr bool operator==(BlockRef other) const {
+    return self == other.self;
+  }
+  LIBC_INLINE constexpr bool operator!=(BlockRef other) const {
+    return !(*this == other);
+  }
+
+  LIBC_INLINE cpp::byte *data() const { return self; }
----------------
mysterymath wrote:

Definitely shouldn't be called self; this is a smell.

A question is what "data" should represent... I'm actually kind of inclined to think of data() as being something that should replace usable_space(), and I think that would probably be a more ergonomic name for the concept too. What is the data that is in the block? It's not the block headers; it's the contained data!

As for the pointer we keep... something like "ptr" (mirroring the constructor param), "start" (separate from begin(), which would probably point to data()), maybe? We could also call this "header", since it is a pointer to the block header, as we've defined it by HEADER_SIZE. In any case, we should be consistent with the constructor param.

https://github.com/llvm/llvm-project/pull/201001