[clang] Use `llvm::SmallVector` instead of `OwningArrayRef` in `VTableLayout`. (PR #168768)
David Stone via cfe-commits
cfe-commits at lists.llvm.org
Tue Nov 25 11:01:31 PST 2025
https://github.com/davidstone updated https://github.com/llvm/llvm-project/pull/168768
>From 8eae3a43847a4667d4660b30af4f6da980462ea6 Mon Sep 17 00:00:00 2001
From: David Stone <davidfromonline at gmail.com>
Date: Wed, 19 Nov 2025 12:58:07 -0700
Subject: [PATCH 1/3] Use `llvm::SmallVector` instead of `OwningArrayRef` in
`VTableLayout`.
This simplifies the code by removing the manual optimization for size == 1, and also gives us an optimization for other small sizes.
Accept a `llvm::SmallVector` by value for the constructor and move it into the destination, rather than accepting `ArrayRef` that we copy from. This also lets us not have to construct a reference to the elements of a `std::initializer_list`, which requires reading the implementation of the constructor to know whether it's safe.
Also explicitly document that the constructor requires the input indexes to have a size of at least 1.
---
clang/include/clang/AST/VTableBuilder.h | 32 +++++++------------------
clang/lib/AST/VTableBuilder.cpp | 17 ++++++-------
2 files changed, 16 insertions(+), 33 deletions(-)
diff --git a/clang/include/clang/AST/VTableBuilder.h b/clang/include/clang/AST/VTableBuilder.h
index e1efe8cddcc5e..58f6fcbc1270e 100644
--- a/clang/include/clang/AST/VTableBuilder.h
+++ b/clang/include/clang/AST/VTableBuilder.h
@@ -246,12 +246,12 @@ class VTableLayout {
// point for a given vtable index.
typedef llvm::SmallVector<unsigned, 4> AddressPointsIndexMapTy;
+ using VTableIndicesTy = llvm::SmallVector<std::size_t>;
+
private:
- // Stores the component indices of the first component of each virtual table in
- // the virtual table group. To save a little memory in the common case where
- // the vtable group contains a single vtable, an empty vector here represents
- // the vector {0}.
- OwningArrayRef<size_t> VTableIndices;
+ // Stores the component indices of the first component of each virtual table
+ // in the virtual table group.
+ VTableIndicesTy VTableIndices;
OwningArrayRef<VTableComponent> VTableComponents;
@@ -265,7 +265,8 @@ class VTableLayout {
AddressPointsIndexMapTy AddressPointIndices;
public:
- VTableLayout(ArrayRef<size_t> VTableIndices,
+ // Requires `!VTableIndicies.empty()`
+ VTableLayout(VTableIndicesTy VTableIndices,
ArrayRef<VTableComponent> VTableComponents,
ArrayRef<VTableThunkTy> VTableThunks,
const AddressPointsMapTy &AddressPoints);
@@ -292,26 +293,11 @@ class VTableLayout {
return AddressPointIndices;
}
- size_t getNumVTables() const {
- if (VTableIndices.empty())
- return 1;
- return VTableIndices.size();
- }
+ size_t getNumVTables() const { return VTableIndices.size(); }
- size_t getVTableOffset(size_t i) const {
- if (VTableIndices.empty()) {
- assert(i == 0);
- return 0;
- }
- return VTableIndices[i];
- }
+ size_t getVTableOffset(size_t i) const { return VTableIndices[i]; }
size_t getVTableSize(size_t i) const {
- if (VTableIndices.empty()) {
- assert(i == 0);
- return vtable_components().size();
- }
-
size_t thisIndex = VTableIndices[i];
size_t nextIndex = (i + 1 == VTableIndices.size())
? vtable_components().size()
diff --git a/clang/lib/AST/VTableBuilder.cpp b/clang/lib/AST/VTableBuilder.cpp
index 9951126c2c3a3..be20aebfc382b 100644
--- a/clang/lib/AST/VTableBuilder.cpp
+++ b/clang/lib/AST/VTableBuilder.cpp
@@ -999,7 +999,7 @@ class ItaniumVTableBuilder {
public:
/// Component indices of the first component of each of the vtables in the
/// vtable group.
- SmallVector<size_t, 4> VTableIndices;
+ VTableLayout::VTableIndicesTy VTableIndices;
ItaniumVTableBuilder(ItaniumVTableContext &VTables,
const CXXRecordDecl *MostDerivedClass,
@@ -2306,18 +2306,15 @@ MakeAddressPointIndices(const VTableLayout::AddressPointsMapTy &addressPoints,
return indexMap;
}
-VTableLayout::VTableLayout(ArrayRef<size_t> VTableIndices,
+VTableLayout::VTableLayout(VTableIndicesTy VTableIndices,
ArrayRef<VTableComponent> VTableComponents,
ArrayRef<VTableThunkTy> VTableThunks,
const AddressPointsMapTy &AddressPoints)
- : VTableComponents(VTableComponents), VTableThunks(VTableThunks),
+ : VTableIndices(std::move(VTableIndices)),
+ VTableComponents(VTableComponents), VTableThunks(VTableThunks),
AddressPoints(AddressPoints), AddressPointIndices(MakeAddressPointIndices(
AddressPoints, VTableIndices.size())) {
- if (VTableIndices.size() <= 1)
- assert(VTableIndices.size() == 1 && VTableIndices[0] == 0);
- else
- this->VTableIndices = OwningArrayRef<size_t>(VTableIndices);
-
+ assert(!VTableIndices.empty() && "VTableLayout requires at least one index.");
llvm::sort(this->VTableThunks, [](const VTableLayout::VTableThunkTy &LHS,
const VTableLayout::VTableThunkTy &RHS) {
assert((LHS.first != RHS.first || LHS.second == RHS.second) &&
@@ -3730,8 +3727,8 @@ void MicrosoftVTableContext::computeVTableRelatedInformation(
SmallVector<VTableLayout::VTableThunkTy, 1> VTableThunks(
Builder.vtable_thunks_begin(), Builder.vtable_thunks_end());
VFTableLayouts[id] = std::make_unique<VTableLayout>(
- ArrayRef<size_t>{0}, Builder.vtable_components(), VTableThunks,
- EmptyAddressPointsMap);
+ VTableLayout::VTableIndicesTy{0}, Builder.vtable_components(),
+ VTableThunks, EmptyAddressPointsMap);
Thunks.insert(Builder.thunks_begin(), Builder.thunks_end());
const ASTRecordLayout &Layout = Context.getASTRecordLayout(RD);
>From 7945528b8c4607ecf12d8075f4e25e5b5380f604 Mon Sep 17 00:00:00 2001
From: David Stone <davidfromonline at gmail.com>
Date: Wed, 19 Nov 2025 15:14:25 -0700
Subject: [PATCH 2/3] The parameter shadows the member variable, so we need to
qualify member access with `this->`
---
clang/lib/AST/VTableBuilder.cpp | 8 +++++---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/clang/lib/AST/VTableBuilder.cpp b/clang/lib/AST/VTableBuilder.cpp
index be20aebfc382b..922d43bb3f239 100644
--- a/clang/lib/AST/VTableBuilder.cpp
+++ b/clang/lib/AST/VTableBuilder.cpp
@@ -2312,9 +2312,11 @@ VTableLayout::VTableLayout(VTableIndicesTy VTableIndices,
const AddressPointsMapTy &AddressPoints)
: VTableIndices(std::move(VTableIndices)),
VTableComponents(VTableComponents), VTableThunks(VTableThunks),
- AddressPoints(AddressPoints), AddressPointIndices(MakeAddressPointIndices(
- AddressPoints, VTableIndices.size())) {
- assert(!VTableIndices.empty() && "VTableLayout requires at least one index.");
+ AddressPoints(AddressPoints),
+ AddressPointIndices(
+ MakeAddressPointIndices(AddressPoints, this->VTableIndices.size())) {
+ assert(!this->VTableIndices.empty() &&
+ "VTableLayout requires at least one index.");
llvm::sort(this->VTableThunks, [](const VTableLayout::VTableThunkTy &LHS,
const VTableLayout::VTableThunkTy &RHS) {
assert((LHS.first != RHS.first || LHS.second == RHS.second) &&
>From 73310e9b67d21b6fecd35332dc52aa5b91b7be41 Mon Sep 17 00:00:00 2001
From: David Stone <davidfromonline at gmail.com>
Date: Tue, 25 Nov 2025 11:58:37 -0700
Subject: [PATCH 3/3] Assert the first index is 0
---
clang/include/clang/AST/VTableBuilder.h | 2 +-
clang/lib/AST/VTableBuilder.cpp | 2 ++
2 files changed, 3 insertions(+), 1 deletion(-)
diff --git a/clang/include/clang/AST/VTableBuilder.h b/clang/include/clang/AST/VTableBuilder.h
index 58f6fcbc1270e..cec3d8aee0650 100644
--- a/clang/include/clang/AST/VTableBuilder.h
+++ b/clang/include/clang/AST/VTableBuilder.h
@@ -265,7 +265,7 @@ class VTableLayout {
AddressPointsIndexMapTy AddressPointIndices;
public:
- // Requires `!VTableIndicies.empty()`
+ // Requires `VTableIndices.front() == 0`
VTableLayout(VTableIndicesTy VTableIndices,
ArrayRef<VTableComponent> VTableComponents,
ArrayRef<VTableThunkTy> VTableThunks,
diff --git a/clang/lib/AST/VTableBuilder.cpp b/clang/lib/AST/VTableBuilder.cpp
index 922d43bb3f239..d97f10cb9d1a1 100644
--- a/clang/lib/AST/VTableBuilder.cpp
+++ b/clang/lib/AST/VTableBuilder.cpp
@@ -2317,6 +2317,8 @@ VTableLayout::VTableLayout(VTableIndicesTy VTableIndices,
MakeAddressPointIndices(AddressPoints, this->VTableIndices.size())) {
assert(!this->VTableIndices.empty() &&
"VTableLayout requires at least one index.");
+ assert(this->VTableIndices.front() == 0 &&
+ "VTableLayout requires the first index is 0.");
llvm::sort(this->VTableThunks, [](const VTableLayout::VTableThunkTy &LHS,
const VTableLayout::VTableThunkTy &RHS) {
assert((LHS.first != RHS.first || LHS.second == RHS.second) &&
More information about the cfe-commits
mailing list