[clang] 26aa20a - Use range-based for to iterate over fields in record layout, NFCI (#122029)
via cfe-commits
cfe-commits at lists.llvm.org
Thu Jan 9 11:21:07 PST 2025
Author: Reid Kleckner
Date: 2025-01-09T11:21:03-08:00
New Revision: 26aa20a3dd82e2ff5855bee04b22b35f6b1f026f
URL: https://github.com/llvm/llvm-project/commit/26aa20a3dd82e2ff5855bee04b22b35f6b1f026f
DIFF: https://github.com/llvm/llvm-project/commit/26aa20a3dd82e2ff5855bee04b22b35f6b1f026f.diff
LOG: Use range-based for to iterate over fields in record layout, NFCI (#122029)
Move the common case of FieldDecl::getFieldIndex() inline to mitigate
the cost of removing the extra `FieldNo` induction variable.
Also rename isNoUniqueAddress parameter to isNonVirtualBaseType, which
appears to be more accurate. I think the current name is just a
consequence of autocomplete gone wrong.
Added:
Modified:
clang/include/clang/AST/Decl.h
clang/lib/AST/Decl.cpp
clang/lib/AST/RecordLayoutBuilder.cpp
clang/lib/CodeGen/CGRecordLayoutBuilder.cpp
Removed:
################################################################################
diff --git a/clang/include/clang/AST/Decl.h b/clang/include/clang/AST/Decl.h
index 67ee0bb412692a..16fc98aa1a57f3 100644
--- a/clang/include/clang/AST/Decl.h
+++ b/clang/include/clang/AST/Decl.h
@@ -3115,8 +3115,20 @@ class FieldDecl : public DeclaratorDecl, public Mergeable<FieldDecl> {
/// Returns the index of this field within its record,
/// as appropriate for passing to ASTRecordLayout::getFieldOffset.
- unsigned getFieldIndex() const;
+ unsigned getFieldIndex() const {
+ const FieldDecl *Canonical = getCanonicalDecl();
+ if (Canonical->CachedFieldIndex == 0) {
+ Canonical->setCachedFieldIndex();
+ assert(Canonical->CachedFieldIndex != 0);
+ }
+ return Canonical->CachedFieldIndex - 1;
+ }
+private:
+ /// Set CachedFieldIndex to the index of this field plus one.
+ void setCachedFieldIndex() const;
+
+public:
/// Determines whether this field is mutable (C++ only).
bool isMutable() const { return Mutable; }
diff --git a/clang/lib/AST/Decl.cpp b/clang/lib/AST/Decl.cpp
index 741e908cf9bc56..97e23dd1aaa920 100644
--- a/clang/lib/AST/Decl.cpp
+++ b/clang/lib/AST/Decl.cpp
@@ -4651,12 +4651,9 @@ bool FieldDecl::isPotentiallyOverlapping() const {
return hasAttr<NoUniqueAddressAttr>() && getType()->getAsCXXRecordDecl();
}
-unsigned FieldDecl::getFieldIndex() const {
- const FieldDecl *Canonical = getCanonicalDecl();
- if (Canonical != this)
- return Canonical->getFieldIndex();
-
- if (CachedFieldIndex) return CachedFieldIndex - 1;
+void FieldDecl::setCachedFieldIndex() const {
+ assert(this == getCanonicalDecl() &&
+ "should be called on the canonical decl");
unsigned Index = 0;
const RecordDecl *RD = getParent()->getDefinition();
@@ -4670,7 +4667,6 @@ unsigned FieldDecl::getFieldIndex() const {
}
assert(CachedFieldIndex && "failed to find field in parent");
- return CachedFieldIndex - 1;
}
SourceRange FieldDecl::getSourceRange() const {
diff --git a/clang/lib/AST/RecordLayoutBuilder.cpp b/clang/lib/AST/RecordLayoutBuilder.cpp
index f749d3a705fc99..4493dd00d26d82 100644
--- a/clang/lib/AST/RecordLayoutBuilder.cpp
+++ b/clang/lib/AST/RecordLayoutBuilder.cpp
@@ -138,9 +138,9 @@ class EmptySubobjectMap {
return Offset <= MaxEmptyClassOffset;
}
- CharUnits
- getFieldOffset(const ASTRecordLayout &Layout, unsigned FieldNo) const {
- uint64_t FieldOffset = Layout.getFieldOffset(FieldNo);
+ CharUnits getFieldOffset(const ASTRecordLayout &Layout,
+ const FieldDecl *Field) const {
+ uint64_t FieldOffset = Layout.getFieldOffset(Field->getFieldIndex());
assert(FieldOffset % CharWidth == 0 &&
"Field offset not at char boundary!");
@@ -298,14 +298,12 @@ EmptySubobjectMap::CanPlaceBaseSubobjectAtOffset(const BaseSubobjectInfo *Info,
}
// Traverse all member variables.
- unsigned FieldNo = 0;
- for (CXXRecordDecl::field_iterator I = Info->Class->field_begin(),
- E = Info->Class->field_end(); I != E; ++I, ++FieldNo) {
- if (I->isBitField())
+ for (const FieldDecl *Field : Info->Class->fields()) {
+ if (Field->isBitField())
continue;
- CharUnits FieldOffset = Offset + getFieldOffset(Layout, FieldNo);
- if (!CanPlaceFieldSubobjectAtOffset(*I, FieldOffset))
+ CharUnits FieldOffset = Offset + getFieldOffset(Layout, Field);
+ if (!CanPlaceFieldSubobjectAtOffset(Field, FieldOffset))
return false;
}
@@ -345,14 +343,12 @@ void EmptySubobjectMap::UpdateEmptyBaseSubobjects(const BaseSubobjectInfo *Info,
}
// Traverse all member variables.
- unsigned FieldNo = 0;
- for (CXXRecordDecl::field_iterator I = Info->Class->field_begin(),
- E = Info->Class->field_end(); I != E; ++I, ++FieldNo) {
- if (I->isBitField())
+ for (const FieldDecl *Field : Info->Class->fields()) {
+ if (Field->isBitField())
continue;
- CharUnits FieldOffset = Offset + getFieldOffset(Layout, FieldNo);
- UpdateEmptyFieldSubobjects(*I, FieldOffset, PlacingEmptyBase);
+ CharUnits FieldOffset = Offset + getFieldOffset(Layout, Field);
+ UpdateEmptyFieldSubobjects(Field, FieldOffset, PlacingEmptyBase);
}
}
@@ -410,15 +406,12 @@ EmptySubobjectMap::CanPlaceFieldSubobjectAtOffset(const CXXRecordDecl *RD,
}
// Traverse all member variables.
- unsigned FieldNo = 0;
- for (CXXRecordDecl::field_iterator I = RD->field_begin(), E = RD->field_end();
- I != E; ++I, ++FieldNo) {
- if (I->isBitField())
+ for (const FieldDecl *Field : RD->fields()) {
+ if (Field->isBitField())
continue;
- CharUnits FieldOffset = Offset + getFieldOffset(Layout, FieldNo);
-
- if (!CanPlaceFieldSubobjectAtOffset(*I, FieldOffset))
+ CharUnits FieldOffset = Offset + getFieldOffset(Layout, Field);
+ if (!CanPlaceFieldSubobjectAtOffset(Field, FieldOffset))
return false;
}
@@ -465,9 +458,8 @@ EmptySubobjectMap::CanPlaceFieldSubobjectAtOffset(const FieldDecl *FD,
return true;
}
-bool
-EmptySubobjectMap::CanPlaceFieldAtOffset(const FieldDecl *FD,
- CharUnits Offset) {
+bool EmptySubobjectMap::CanPlaceFieldAtOffset(const FieldDecl *FD,
+ CharUnits Offset) {
if (!CanPlaceFieldSubobjectAtOffset(FD, Offset))
return false;
@@ -521,15 +513,12 @@ void EmptySubobjectMap::UpdateEmptyFieldSubobjects(
}
// Traverse all member variables.
- unsigned FieldNo = 0;
- for (CXXRecordDecl::field_iterator I = RD->field_begin(), E = RD->field_end();
- I != E; ++I, ++FieldNo) {
- if (I->isBitField())
+ for (const FieldDecl *Field : RD->fields()) {
+ if (Field->isBitField())
continue;
- CharUnits FieldOffset = Offset + getFieldOffset(Layout, FieldNo);
-
- UpdateEmptyFieldSubobjects(*I, FieldOffset, PlacingOverlappingField);
+ CharUnits FieldOffset = Offset + getFieldOffset(Layout, Field);
+ UpdateEmptyFieldSubobjects(Field, FieldOffset, PlacingOverlappingField);
}
}
@@ -1455,10 +1444,8 @@ void ItaniumRecordLayoutBuilder::LayoutFields(const RecordDecl *D) {
bool InsertExtraPadding = D->mayInsertExtraPadding(/*EmitRemark=*/true);
bool HasFlexibleArrayMember = D->hasFlexibleArrayMember();
for (auto I = D->field_begin(), End = D->field_end(); I != End; ++I) {
- auto Next(I);
- ++Next;
- LayoutField(*I,
- InsertExtraPadding && (Next != End || !HasFlexibleArrayMember));
+ LayoutField(*I, InsertExtraPadding &&
+ (std::next(I) != End || !HasFlexibleArrayMember));
}
}
@@ -3672,35 +3659,33 @@ static void DumpRecordLayout(raw_ostream &OS, const RecordDecl *RD,
}
// Dump fields.
- uint64_t FieldNo = 0;
- for (RecordDecl::field_iterator I = RD->field_begin(),
- E = RD->field_end(); I != E; ++I, ++FieldNo) {
- const FieldDecl &Field = **I;
- uint64_t LocalFieldOffsetInBits = Layout.getFieldOffset(FieldNo);
+ for (const FieldDecl *Field : RD->fields()) {
+ uint64_t LocalFieldOffsetInBits =
+ Layout.getFieldOffset(Field->getFieldIndex());
CharUnits FieldOffset =
Offset + C.toCharUnitsFromBits(LocalFieldOffsetInBits);
// Recursively dump fields of record type.
- if (auto RT = Field.getType()->getAs<RecordType>()) {
+ if (auto RT = Field->getType()->getAs<RecordType>()) {
DumpRecordLayout(OS, RT->getDecl(), C, FieldOffset, IndentLevel,
- Field.getName().data(),
+ Field->getName().data(),
/*PrintSizeInfo=*/false,
/*IncludeVirtualBases=*/true);
continue;
}
- if (Field.isBitField()) {
+ if (Field->isBitField()) {
uint64_t LocalFieldByteOffsetInBits = C.toBits(FieldOffset - Offset);
unsigned Begin = LocalFieldOffsetInBits - LocalFieldByteOffsetInBits;
- unsigned Width = Field.getBitWidthValue(C);
+ unsigned Width = Field->getBitWidthValue(C);
PrintBitFieldOffset(OS, FieldOffset, Begin, Width, IndentLevel);
} else {
PrintOffset(OS, FieldOffset, IndentLevel);
}
const QualType &FieldType = C.getLangOpts().DumpRecordLayoutsCanonical
- ? Field.getType().getCanonicalType()
- : Field.getType();
- OS << FieldType << ' ' << Field << '\n';
+ ? Field->getType().getCanonicalType()
+ : Field->getType();
+ OS << FieldType << ' ' << *Field << '\n';
}
// Dump virtual bases.
diff --git a/clang/lib/CodeGen/CGRecordLayoutBuilder.cpp b/clang/lib/CodeGen/CGRecordLayoutBuilder.cpp
index ea44e6f21f3c86..209ef236f0d5dc 100644
--- a/clang/lib/CodeGen/CGRecordLayoutBuilder.cpp
+++ b/clang/lib/CodeGen/CGRecordLayoutBuilder.cpp
@@ -182,7 +182,7 @@ struct CGRecordLowering {
llvm::Type *StorageType);
/// Lowers an ASTRecordLayout to a llvm type.
void lower(bool NonVirtualBaseType);
- void lowerUnion(bool isNoUniqueAddress);
+ void lowerUnion(bool isNonVirtualBaseType);
void accumulateFields(bool isNonVirtualBaseType);
RecordDecl::field_iterator
accumulateBitFields(bool isNonVirtualBaseType,
@@ -310,9 +310,9 @@ void CGRecordLowering::lower(bool NVBaseType) {
computeVolatileBitfields();
}
-void CGRecordLowering::lowerUnion(bool isNoUniqueAddress) {
+void CGRecordLowering::lowerUnion(bool isNonVirtualBaseType) {
CharUnits LayoutSize =
- isNoUniqueAddress ? Layout.getDataSize() : Layout.getSize();
+ isNonVirtualBaseType ? Layout.getDataSize() : Layout.getSize();
llvm::Type *StorageType = nullptr;
bool SeenNamedMember = false;
// Iterate through the fields setting bitFieldInfo and the Fields array. Also
More information about the cfe-commits
mailing list