[llvm] [orc-rt] Nest StringPool handle types (PR #216615)
Lang Hames via llvm-commits
llvm-commits at lists.llvm.org
Sun Aug 16 16:30:14 PDT 2026
https://github.com/lhames created https://github.com/llvm/llvm-project/pull/216615
Move the handle types into StringPool as nested members:
PooledStringPtr -> StringPool::Ptr
NonOwningPooledStringPtr -> StringPool::WeakPtr
PooledStringPtrBase -> StringPool::PtrBase
StringPoolEntryUnsafe -> StringPool::EntryUnsafe
Also tightens the WeakPtr -> Ptr contract: reconstructing a Ptr is only well-defined while another Ptr keeps the entry alive.
>From baeb675d857c66db24a7d9a696664889bdfaf6ff Mon Sep 17 00:00:00 2001
From: Lang Hames <lhames at gmail.com>
Date: Mon, 17 Aug 2026 09:17:19 +1000
Subject: [PATCH] [orc-rt] Nest StringPool handle types
Move the handle types into StringPool as nested members:
PooledStringPtr -> StringPool::Ptr
NonOwningPooledStringPtr -> StringPool::WeakPtr
PooledStringPtrBase -> StringPool::PtrBase
StringPoolEntryUnsafe -> StringPool::EntryUnsafe
Also tightens the WeakPtr -> Ptr contract: reconstructing a Ptr is only
well-defined while another Ptr keeps the entry alive.
---
orc-rt/include/orc-rt/StringPool.h | 147 ++++++++++++++--------------
orc-rt/test/unit/StringPoolTest.cpp | 55 +++++------
2 files changed, 98 insertions(+), 104 deletions(-)
diff --git a/orc-rt/include/orc-rt/StringPool.h b/orc-rt/include/orc-rt/StringPool.h
index 85014da2b2e54..d84d8b2cf6d5e 100644
--- a/orc-rt/include/orc-rt/StringPool.h
+++ b/orc-rt/include/orc-rt/StringPool.h
@@ -24,15 +24,12 @@
namespace orc_rt {
-class PooledStringPtr;
-class NonOwningPooledStringPtr;
-
/// Interns strings (e.g. symbol names, paths) behind ref-counted handles. An
-/// entry is kept alive as long as at least one PooledStringPtr refers to it;
+/// entry is kept alive as long as at least one StringPool::Ptr refers to it;
/// clearDeadEntries() reclaims entries with no owners left.
///
/// intern() and clearDeadEntries() may be called concurrently from any
-/// number of threads. Copying, moving, and destroying a PooledStringPtr
+/// number of threads. Copying, moving, and destroying a StringPool::Ptr
/// requires no lock -- only the atomic refcount in that ptr's own entry is
/// touched.
class StringPool {
@@ -43,15 +40,20 @@ class StringPool {
public:
using PoolEntry = PoolMap::value_type;
+ class PtrBase;
+ class Ptr;
+ class WeakPtr;
+ class EntryUnsafe;
+
StringPool() = default;
StringPool(const StringPool &) = delete;
StringPool &operator=(const StringPool &) = delete;
~StringPool();
- /// Returns the PooledStringPtr for S, interning a copy on first reference.
- PooledStringPtr intern(std::string_view S);
+ /// Returns the Ptr for S, interning a copy on first reference.
+ Ptr intern(std::string_view S);
- /// Erase entries with no remaining PooledStringPtr owners.
+ /// Erase entries with no remaining Ptr owners.
void clearDeadEntries();
/// Returns true if this pool has no entries.
@@ -62,66 +64,62 @@ class StringPool {
PoolMap Pool;
};
-/// Common base for PooledStringPtr and NonOwningPooledStringPtr: bool
-/// conversion, dereference, and comparison.
+/// Common base for StringPool::Ptr and StringPool::WeakPtr: bool conversion,
+/// dereference, and comparison.
///
/// Comparisons and hashing are pointer-identity, scoped to whichever
/// StringPool produced the handle -- handles from different pools are never
/// equal, even for identical text.
-class PooledStringPtrBase {
- friend class StringPoolEntryUnsafe;
+class StringPool::PtrBase {
+ friend class EntryUnsafe;
public:
- PooledStringPtrBase() = default;
- PooledStringPtrBase(std::nullptr_t) noexcept {}
+ PtrBase() = default;
+ PtrBase(std::nullptr_t) noexcept {}
explicit operator bool() const noexcept { return E != nullptr; }
const std::string &operator*() const noexcept { return E->first; }
- friend bool operator==(PooledStringPtrBase LHS,
- PooledStringPtrBase RHS) noexcept {
+ friend bool operator==(PtrBase LHS, PtrBase RHS) noexcept {
return LHS.E == RHS.E;
}
- friend bool operator!=(PooledStringPtrBase LHS,
- PooledStringPtrBase RHS) noexcept {
+ friend bool operator!=(PtrBase LHS, PtrBase RHS) noexcept {
return !(LHS == RHS);
}
// Pointer-order only; not stable across runs (ASLR). Fine as a map/set key
// ordering, not for anything user-visible.
- friend bool operator<(PooledStringPtrBase LHS,
- PooledStringPtrBase RHS) noexcept {
+ friend bool operator<(PtrBase LHS, PtrBase RHS) noexcept {
return LHS.E < RHS.E;
}
protected:
using PoolEntry = StringPool::PoolEntry;
- explicit PooledStringPtrBase(PoolEntry *E) noexcept : E(E) {}
+ explicit PtrBase(PoolEntry *E) noexcept : E(E) {}
PoolEntry *E = nullptr;
};
/// An owning, ref-counted handle to a string interned in some StringPool.
-class PooledStringPtr : public PooledStringPtrBase {
+class StringPool::Ptr : public StringPool::PtrBase {
friend class StringPool;
public:
- PooledStringPtr() = default;
- PooledStringPtr(std::nullptr_t) noexcept {}
-
- /// Constructs an owning handle from a non-owning one, incrementing the
- /// refcount. Other must be backed by an entry that some PooledStringPtr is
- /// already keeping alive -- constructing from a NonOwningPooledStringPtr
- /// whose entry has already been reclaimed by clearDeadEntries() is
- /// undefined behavior.
- explicit PooledStringPtr(NonOwningPooledStringPtr Other) noexcept;
-
- PooledStringPtr(const PooledStringPtr &Other) noexcept
- : PooledStringPtrBase(Other.E) {
- incRef();
- }
+ Ptr() = default;
+ Ptr(std::nullptr_t) noexcept {}
+
+ /// Constructs an owning handle from a weak one, incrementing the refcount.
+ /// Other's entry must currently have a nonzero refcount (i.e. some other
+ /// Ptr is known to be keeping it alive right now) -- constructing from a
+ /// WeakPtr whose entry's refcount has already reached zero is undefined
+ /// behavior, whether or not clearDeadEntries() has actually run yet.
+ /// Reclamation can happen on any thread as soon as the count hits zero, so
+ /// there is no safe window to observe "zero but not yet reclaimed".
+ explicit Ptr(WeakPtr Other) noexcept;
- PooledStringPtr &operator=(const PooledStringPtr &Other) noexcept {
+ Ptr(const Ptr &Other) noexcept : PtrBase(Other.E) { incRef(); }
+
+ Ptr &operator=(const Ptr &Other) noexcept {
if (this != &Other) {
decRef();
E = Other.E;
@@ -130,21 +128,19 @@ class PooledStringPtr : public PooledStringPtrBase {
return *this;
}
- PooledStringPtr(PooledStringPtr &&Other) noexcept { std::swap(E, Other.E); }
+ Ptr(Ptr &&Other) noexcept { std::swap(E, Other.E); }
- PooledStringPtr &operator=(PooledStringPtr &&Other) noexcept {
+ Ptr &operator=(Ptr &&Other) noexcept {
decRef();
E = nullptr;
std::swap(E, Other.E);
return *this;
}
- ~PooledStringPtr() { decRef(); }
+ ~Ptr() { decRef(); }
private:
- explicit PooledStringPtr(PoolEntry *E) noexcept : PooledStringPtrBase(E) {
- incRef();
- }
+ explicit Ptr(PoolEntry *E) noexcept : PtrBase(E) { incRef(); }
void incRef() noexcept {
if (E)
@@ -153,65 +149,64 @@ class PooledStringPtr : public PooledStringPtrBase {
void decRef() noexcept {
if (E) {
- assert(E->second.load() != 0 && "double-release of PooledStringPtr");
+ assert(E->second.load() != 0 && "double-release of StringPool::Ptr");
--E->second;
}
}
};
-/// A non-owning handle to a string interned in some StringPool.
+/// A non-owning (weak) handle to a string interned in some StringPool.
///
-/// Comparable and hashable interchangeably with PooledStringPtr (both wrap the
-/// same underlying entry pointer), but copying a NonOwningPooledStringPtr never
-/// touches the refcount, so it's cheaper to pass around than a PooledStringPtr.
-/// It is silently invalidated if the entry's refcount drops to zero and is
-/// reclaimed by clearDeadEntries(), so only use it where a corresponding
-/// PooledStringPtr is known to be keeping the entry alive -- e.g. as a lookup
-/// key into a table whose values (or a side table) hold the owning
-/// PooledStringPtr for that same entry.
-class NonOwningPooledStringPtr : public PooledStringPtrBase {
+/// Comparable and hashable interchangeably with StringPool::Ptr (both wrap the
+/// same underlying entry pointer), but copying a WeakPtr never touches the
+/// refcount, so it's cheaper to pass around than a Ptr. It is invalidated the
+/// instant the entry's refcount drops to zero (not when clearDeadEntries() next
+/// happens to run, which may be arbitrarily later on another thread) so only
+/// dereference, compare, or reconstruct a Ptr from a WeakPtr where a
+/// corresponding Ptr is known to be keeping the entry alive,
+/// e.g. as a lookup key into a table whose values (or a side table) hold the
+/// owning Ptr for that same entry.
+class StringPool::WeakPtr : public StringPool::PtrBase {
public:
- NonOwningPooledStringPtr() = default;
- NonOwningPooledStringPtr(std::nullptr_t) noexcept {}
- explicit NonOwningPooledStringPtr(const PooledStringPtr &Other) noexcept
- : PooledStringPtrBase(Other) {}
+ WeakPtr() = default;
+ WeakPtr(std::nullptr_t) noexcept {}
+ explicit WeakPtr(const Ptr &Other) noexcept : PtrBase(Other) {}
};
/// Provides unsafe (refcount-bypassing) access to the pool-entry pointer
-/// underlying a PooledStringPtrBase. Used to implement std::hash and C API
-/// operations. Not intended for general use.
-class StringPoolEntryUnsafe {
+/// underlying a StringPool::PtrBase. Used to implement std::hash, and
+/// intended to grow C API support (retain/release/take-ownership operations
+/// on an opaque pool-entry token) as that need arises.
+class StringPool::EntryUnsafe {
public:
using PoolEntry = StringPool::PoolEntry;
/// Extracts the pool-entry pointer from S without affecting its refcount.
- static StringPoolEntryUnsafe from(const PooledStringPtrBase &S) {
- return StringPoolEntryUnsafe(S.E);
- }
+ static EntryUnsafe from(const PtrBase &S) { return EntryUnsafe(S.E); }
const void *rawPtr() const { return E; }
private:
- StringPoolEntryUnsafe(PoolEntry *E) : E(E) {}
+ EntryUnsafe(PoolEntry *E) : E(E) {}
PoolEntry *E = nullptr;
};
-inline PooledStringPtr::PooledStringPtr(NonOwningPooledStringPtr Other) noexcept
- : PooledStringPtrBase(Other) {
+inline StringPool::Ptr::Ptr(StringPool::WeakPtr Other) noexcept
+ : PtrBase(Other) {
incRef();
}
inline StringPool::~StringPool() {
#ifndef NDEBUG
clearDeadEntries();
- assert(Pool.empty() && "Dangling PooledStringPtr at StringPool destruction");
+ assert(Pool.empty() && "Dangling StringPool::Ptr at StringPool destruction");
#endif
}
-inline PooledStringPtr StringPool::intern(std::string_view S) {
+inline StringPool::Ptr StringPool::intern(std::string_view S) {
std::scoped_lock<std::mutex> Lock(M);
auto [I, Added] = Pool.try_emplace(std::string(S), 0);
- return PooledStringPtr(&*I);
+ return Ptr(&*I);
}
inline void StringPool::clearDeadEntries() {
@@ -231,17 +226,17 @@ inline bool StringPool::empty() const {
} // namespace orc_rt
namespace std {
-template <> struct hash<orc_rt::PooledStringPtr> {
- size_t operator()(const orc_rt::PooledStringPtr &S) const noexcept {
+template <> struct hash<orc_rt::StringPool::Ptr> {
+ size_t operator()(const orc_rt::StringPool::Ptr &S) const noexcept {
return hash<const void *>()(
- orc_rt::StringPoolEntryUnsafe::from(S).rawPtr());
+ orc_rt::StringPool::EntryUnsafe::from(S).rawPtr());
}
};
-template <> struct hash<orc_rt::NonOwningPooledStringPtr> {
- size_t operator()(const orc_rt::NonOwningPooledStringPtr &S) const noexcept {
+template <> struct hash<orc_rt::StringPool::WeakPtr> {
+ size_t operator()(const orc_rt::StringPool::WeakPtr &S) const noexcept {
return hash<const void *>()(
- orc_rt::StringPoolEntryUnsafe::from(S).rawPtr());
+ orc_rt::StringPool::EntryUnsafe::from(S).rawPtr());
}
};
} // namespace std
diff --git a/orc-rt/test/unit/StringPoolTest.cpp b/orc-rt/test/unit/StringPoolTest.cpp
index 85b866d12a275..bb8d92e32969f 100644
--- a/orc-rt/test/unit/StringPoolTest.cpp
+++ b/orc-rt/test/unit/StringPoolTest.cpp
@@ -53,14 +53,14 @@ TEST(StringPoolTest, DifferentPoolsAreDistinct) {
}
TEST(StringPoolTest, DefaultConstructedIsNull) {
- PooledStringPtr Null;
+ StringPool::Ptr Null;
EXPECT_FALSE(Null);
- EXPECT_EQ(Null, PooledStringPtr(nullptr));
+ EXPECT_EQ(Null, StringPool::Ptr(nullptr));
}
TEST(StringPoolTest, CopyKeepsEntryAlive) {
StringPool SP;
- PooledStringPtr Copy;
+ StringPool::Ptr Copy;
{
auto Foo = SP.intern("foo");
Copy = Foo;
@@ -103,40 +103,39 @@ TEST(StringPoolTest, MoveLeavesSourceNull) {
EXPECT_EQ(*Moved, "foo");
}
-TEST(StringPoolTest, NonOwningPtrComparesEqualToOwning) {
+TEST(StringPoolTest, WeakPtrComparesEqualToOwning) {
StringPool SP;
auto Foo = SP.intern("foo");
- NonOwningPooledStringPtr NonOwningFoo(Foo);
- EXPECT_EQ(Foo, NonOwningFoo);
- EXPECT_EQ(*NonOwningFoo, "foo");
+ StringPool::WeakPtr WeakFoo(Foo);
+ EXPECT_EQ(Foo, WeakFoo);
+ EXPECT_EQ(*WeakFoo, "foo");
}
-TEST(StringPoolTest, NonOwningPtrDoesNotKeepEntryAlive) {
+TEST(StringPoolTest, WeakPtrDoesNotKeepEntryAlive) {
StringPool SP;
- NonOwningPooledStringPtr NonOwningFoo;
+ StringPool::WeakPtr WeakFoo;
{
auto Foo = SP.intern("foo");
- NonOwningFoo = NonOwningPooledStringPtr(Foo);
+ WeakFoo = StringPool::WeakPtr(Foo);
}
// Foo has been destroyed and was the only owner, so the entry should be
- // reclaimed even though NonOwningFoo still points at it.
+ // reclaimed even though WeakFoo still points at it.
SP.clearDeadEntries();
EXPECT_TRUE(SP.empty());
}
-TEST(StringPoolTest, ConstructOwningFromNonOwningIncrementsRefcount) {
+TEST(StringPoolTest, ConstructOwningFromWeakWhileStillAlive) {
StringPool SP;
- NonOwningPooledStringPtr NonOwningFoo;
- {
- auto Foo = SP.intern("foo");
- NonOwningFoo = NonOwningPooledStringPtr(Foo);
- }
- // The entry's refcount is now zero, but it has not yet been reclaimed by
- // clearDeadEntries(), so re-deriving an owning ptr from NonOwningFoo here
- // is well-defined and should keep the entry alive.
- PooledStringPtr Reowned(NonOwningFoo);
- SP.clearDeadEntries();
- ASSERT_FALSE(SP.empty());
+ auto Foo = SP.intern("foo");
+ StringPool::WeakPtr WeakFoo(Foo);
+ // Foo is still alive here, so the entry's refcount is nonzero and
+ // reconstructing an owning Ptr from WeakFoo is well-defined. Constructing
+ // from a WeakPtr whose entry's refcount has already reached zero is
+ // undefined behavior -- there is no safe way to detect that case from the
+ // WeakPtr side, since reclamation can happen on another thread as soon as
+ // the count hits zero.
+ StringPool::Ptr Reowned(WeakFoo);
+ EXPECT_EQ(Reowned, Foo);
EXPECT_EQ(*Reowned, "foo");
}
@@ -146,7 +145,7 @@ TEST(StringPoolTest, UsableAsUnorderedSetKey) {
auto Foo2 = SP.intern("foo");
auto Bar = SP.intern("bar");
- std::unordered_set<PooledStringPtr> S;
+ std::unordered_set<StringPool::Ptr> S;
S.insert(Foo1);
S.insert(Foo2);
S.insert(Bar);
@@ -156,11 +155,11 @@ TEST(StringPoolTest, UsableAsUnorderedSetKey) {
EXPECT_TRUE(S.count(Bar));
}
-TEST(StringPoolTest, OwningAndNonOwningHashInterchangeably) {
+TEST(StringPoolTest, OwningAndWeakHashInterchangeably) {
StringPool SP;
auto Foo = SP.intern("foo");
- NonOwningPooledStringPtr NonOwningFoo(Foo);
+ StringPool::WeakPtr WeakFoo(Foo);
- EXPECT_EQ(std::hash<PooledStringPtr>()(Foo),
- std::hash<NonOwningPooledStringPtr>()(NonOwningFoo));
+ EXPECT_EQ(std::hash<StringPool::Ptr>()(Foo),
+ std::hash<StringPool::WeakPtr>()(WeakFoo));
}
More information about the llvm-commits
mailing list