Skip to content

Commit 94017f7

Browse files
xedinxedin
committed
[IRGen] Remove 'FieldNames' field from type context descriptor
All of the information contained by this field (list of property names) is already encoded as part of the field reflection metadata and is accessible via `swift_getFieldAt` runtime method.
1 parent f6be62d commit 94017f7

File tree

10 files changed

+56
-106
lines changed

10 files changed

+56
-106
lines changed

‎include/swift/Runtime/Metadata.h

Lines changed: 15 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -3154,9 +3154,11 @@ class TargetClassDescriptor final
31543154
}
31553155

31563156
public:
3157-
/// The field names. A doubly-null-terminated list of strings, whose
3158-
/// length and order is consistent with that of the field offset vector.
3159-
RelativeDirectPointer<const char, /*nullable*/ true> FieldNames;
3157+
/// Indicates if the type represented by this descriptor
3158+
/// supports reflection (C and Obj-C enums currently don't).
3159+
/// FIXME: This is temporarily left as 32-bit integer to avoid
3160+
/// changing layout of context descriptor.
3161+
uint32_t IsReflectable;
31603162

31613163
/// True if metadata records for this type have a field offset vector for
31623164
/// its stored properties.
@@ -3258,9 +3260,11 @@ class TargetStructDescriptor final
32583260
/// vector.
32593261
uint32_t FieldOffsetVectorOffset;
32603262

3261-
/// The field names. A doubly-null-terminated list of strings, whose
3262-
/// length and order is consistent with that of the field offset vector.
3263-
RelativeDirectPointer<const char, /*nullable*/ true> FieldNames;
3263+
/// Indicates if the type represented by this descriptor
3264+
/// supports reflection (C and Obj-C enums currently don't).
3265+
/// FIXME: This is temporarily left as 32-bit integer to avoid
3266+
/// changing layout of context descriptor.
3267+
uint32_t IsReflectable;
32643268

32653269
/// True if metadata records for this type have a field offset vector for
32663270
/// its stored properties.
@@ -3304,10 +3308,11 @@ class TargetEnumDescriptor final
33043308
/// The number of empty cases in the enum.
33053309
uint32_t NumEmptyCases;
33063310

3307-
/// The names of the cases. A doubly-null-terminated list of strings,
3308-
/// whose length is NumNonEmptyCases + NumEmptyCases. Cases are named in
3309-
/// tag order, non-empty cases first, followed by empty cases.
3310-
RelativeDirectPointer<const char, /*nullable*/ true> CaseNames;
3311+
/// Indicates if the type represented by this descriptor
3312+
/// supports reflection (C and Obj-C enums currently don't).
3313+
/// FIXME: This is temporarily left as 32-bit integer to avoid
3314+
/// changing layout of context descriptor.
3315+
uint32_t IsReflectable;
33113316

33123317
uint32_t getNumPayloadCases() const {
33133318
return NumPayloadCasesAndPayloadSizeOffset & 0x00FFFFFFU;

‎lib/IRGen/GenEnum.cpp

Lines changed: 5 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -158,21 +158,7 @@ void irgen::EnumImplStrategy::initializeFromParams(IRGenFunction &IGF,
158158
TI->initializeWithTake(IGF, dest, src, T, isOutlined);
159159
}
160160

161-
llvm::Constant *EnumImplStrategy::emitCaseNames() const {
162-
// Build the list of case names, payload followed by no-payload.
163-
llvm::SmallString<64> fieldNames;
164-
for (auto &payloadCase : getElementsWithPayload()) {
165-
fieldNames.append(payloadCase.decl->getName().str());
166-
fieldNames.push_back('\0');
167-
}
168-
for (auto &noPayloadCase : getElementsWithNoPayload()) {
169-
fieldNames.append(noPayloadCase.decl->getName().str());
170-
fieldNames.push_back('\0');
171-
}
172-
// The final null terminator is provided by getAddrOfGlobalString.
173-
return IGM.getAddrOfGlobalString(fieldNames,
174-
/*willBeRelativelyAddressed*/ true);
175-
}
161+
bool EnumImplStrategy::isReflectable() const { return true; }
176162

177163
unsigned EnumImplStrategy::getPayloadSizeForMetadata() const {
178164
llvm_unreachable("don't need payload size for this enum kind");
@@ -1109,11 +1095,11 @@ namespace {
11091095
llvm_unreachable("no extra inhabitants");
11101096
}
11111097

1112-
llvm::Constant *emitCaseNames() const override {
1098+
bool isReflectable() const override {
11131099
// C enums have arbitrary values and we don't preserve the mapping
1114-
// between the case and raw value at runtime, so don't emit any
1115-
// case names at all so that reflection can give up in this case.
1116-
return nullptr;
1100+
// between the case and raw value at runtime, so don't mark it as
1101+
// reflectable.
1102+
return false;
11171103
}
11181104
};
11191105

‎lib/IRGen/GenEnum.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -224,7 +224,7 @@ class EnumImplStrategy {
224224
}
225225

226226
/// Emit field names for enum reflection.
227-
virtual llvm::Constant *emitCaseNames() const;
227+
virtual bool isReflectable() const;
228228

229229
/// \brief Return the bits used for discriminators for payload cases.
230230
///

‎lib/IRGen/GenMeta.cpp

Lines changed: 9 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -2779,17 +2779,12 @@ namespace {
27792779
}
27802780

27812781
void addLayoutInfo() {
2782-
// Build the field name list.
2783-
llvm::SmallString<64> fieldNames;
2784-
unsigned numFields = getFieldNameString(getType()->getStoredProperties(),
2785-
fieldNames);
2786-
2787-
B.addInt32(numFields);
2782+
auto properties = getType()->getStoredProperties();
2783+
B.addInt32(std::distance(properties.begin(), properties.end()));
27882784
B.addInt32(FieldVectorOffset / IGM.getPointerSize());
2789-
B.addRelativeAddress(IGM.getAddrOfGlobalString(fieldNames,
2790-
/*willBeRelativelyAddressed*/ true));
2785+
B.addInt32(1); // struct always reflectable
27912786

2792-
addFieldTypes(IGM, getType(), getType()->getStoredProperties());
2787+
addFieldTypes(IGM, getType(), properties);
27932788
}
27942789

27952790
uint16_t getKindSpecificFlags() {
@@ -2844,8 +2839,7 @@ namespace {
28442839
B.addInt32(numPayloads | (PayloadSizeOffsetInWords << 24));
28452840
// # empty cases
28462841
B.addInt32(strategy.getElementsWithNoPayload().size());
2847-
2848-
B.addRelativeAddressOrNull(strategy.emitCaseNames());
2842+
B.addInt32(strategy.isReflectable());
28492843

28502844
addFieldTypes(IGM, strategy.getElementsWithPayload());
28512845
}
@@ -2981,17 +2975,12 @@ namespace {
29812975
}
29822976

29832977
void addLayoutInfo() {
2984-
// Build the field name list.
2985-
llvm::SmallString<64> fieldNames;
2986-
unsigned numFields = getFieldNameString(getType()->getStoredProperties(),
2987-
fieldNames);
2988-
2989-
B.addInt32(numFields);
2978+
auto properties = getType()->getStoredProperties();
2979+
B.addInt32(std::distance(properties.begin(), properties.end()));
29902980
B.addInt32(FieldVectorOffset / IGM.getPointerSize());
2991-
B.addRelativeAddress(IGM.getAddrOfGlobalString(fieldNames,
2992-
/*willBeRelativelyAddressed*/ true));
2981+
B.addInt32(1); // class is always reflectable
29932982

2994-
addFieldTypes(IGM, getType(), getType()->getStoredProperties());
2983+
addFieldTypes(IGM, getType(), properties);
29952984
}
29962985
};
29972986
} // end anonymous namespace

‎stdlib/public/runtime/MetadataLookup.cpp

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1054,8 +1054,12 @@ void swift::swift_getFieldAt(
10541054
auto getFieldAt = [&](const FieldDescriptor &descriptor) {
10551055
auto &field = descriptor.getFields()[index];
10561056
auto name = field.getFieldName(0);
1057-
auto type = field.getMangledTypeName(0);
10581057

1058+
// Enum cases don't always have types.
1059+
if (!field.hasMangledTypeName()) {
1060+
callback(name, FieldType().withIndirect(field.isIndirectCase()));
1061+
return;
1062+
}
10591063

10601064
std::vector<const ContextDescriptor *> descriptorPath;
10611065
{
@@ -1070,7 +1074,8 @@ void swift::swift_getFieldAt(
10701074
}
10711075

10721076
auto typeInfo = _getTypeByMangledName(
1073-
type, [&](unsigned depth, unsigned index) -> const Metadata * {
1077+
field.getMangledTypeName(0),
1078+
[&](unsigned depth, unsigned index) -> const Metadata * {
10741079
if (depth >= descriptorPath.size())
10751080
return nullptr;
10761081

‎stdlib/public/runtime/Reflection.mm

Lines changed: 19 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -446,19 +446,6 @@ void swift_TupleMirror_subscript(String *outString,
446446
new (outMirror) Mirror(reflect(owner, eltData, elt.Type));
447447
}
448448

449-
// Get a field name from a doubly-null-terminated list.
450-
static const char *getFieldName(const char *fieldNames, size_t i) {
451-
const char *fieldName = fieldNames;
452-
for (size_t j = 0; j < i; ++j) {
453-
size_t len = strlen(fieldName);
454-
assert(len != 0);
455-
fieldName += len + 1;
456-
}
457-
458-
return fieldName;
459-
}
460-
461-
462449
static bool loadSpecialReferenceStorage(HeapObject *owner,
463450
OpaqueValue *fieldData,
464451
const FieldType fieldType,
@@ -581,17 +568,14 @@ static bool isEnumReflectable(const Metadata *type) {
581568
const auto &Description = *Enum->getDescription();
582569

583570
// No metadata for C and @objc enums yet
584-
if (Description.CaseNames == nullptr)
585-
return false;
586-
587-
return true;
571+
return Description.IsReflectable;
588572
}
589573

590-
static void getEnumMirrorInfo(const OpaqueValue *value,
591-
const Metadata *type,
592-
unsigned *tagPtr,
593-
const Metadata **payloadTypePtr,
594-
bool *indirectPtr) {
574+
static const char *getEnumMirrorInfo(const OpaqueValue *value,
575+
const Metadata *type,
576+
unsigned *tagPtr = nullptr,
577+
const Metadata **payloadTypePtr = nullptr,
578+
bool *indirectPtr = nullptr) {
595579
const auto Enum = static_cast<const EnumMetadata *>(type);
596580
const auto &Description = *Enum->getDescription();
597581

@@ -606,19 +590,21 @@ static void getEnumMirrorInfo(const OpaqueValue *value,
606590
const Metadata *payloadType = nullptr;
607591
bool indirect = false;
608592

609-
if (static_cast<unsigned>(tag) < payloadCases) {
610-
swift_getFieldAt(type, tag, [&](llvm::StringRef caseName, FieldType info) {
611-
payloadType = info.getType();
612-
indirect = info.isIndirect();
613-
});
614-
}
593+
const char *caseName = nullptr;
594+
swift_getFieldAt(type, tag, [&](llvm::StringRef name, FieldType info) {
595+
caseName = name.data();
596+
payloadType = info.getType();
597+
indirect = info.isIndirect();
598+
});
615599

616600
if (tagPtr)
617601
*tagPtr = tag;
618602
if (payloadTypePtr)
619603
*payloadTypePtr = payloadType;
620604
if (indirectPtr)
621605
*indirectPtr = indirect;
606+
607+
return caseName;
622608
}
623609

624610
// internal func _swift_EnumMirror_caseName(
@@ -634,15 +620,11 @@ static void getEnumMirrorInfo(const OpaqueValue *value,
634620
return nullptr;
635621
}
636622

637-
const auto Enum = static_cast<const EnumMetadata *>(type);
638-
const auto &Description = *Enum->getDescription();
639-
640-
unsigned tag;
641-
getEnumMirrorInfo(value, type, &tag, nullptr, nullptr);
623+
auto caseName = getEnumMirrorInfo(value, type);
642624

643625
SWIFT_CC_PLUSONE_GUARD(swift_release(owner));
644626

645-
return getFieldName(Description.CaseNames, tag);
627+
return caseName;
646628
}
647629

648630
// internal func _getEnumCaseName<T>(_ value: T) -> UnsafePointer<CChar>?
@@ -687,7 +669,7 @@ intptr_t swift_EnumMirror_count(HeapObject *owner,
687669
}
688670

689671
const Metadata *payloadType;
690-
getEnumMirrorInfo(value, type, nullptr, &payloadType, nullptr);
672+
(void)getEnumMirrorInfo(value, type, nullptr, &payloadType, nullptr);
691673
SWIFT_CC_PLUSONE_GUARD(swift_release(owner));
692674
return (payloadType != nullptr) ? 1 : 0;
693675
}
@@ -709,7 +691,7 @@ void swift_EnumMirror_subscript(String *outString,
709691
const Metadata *payloadType;
710692
bool indirect;
711693

712-
getEnumMirrorInfo(value, type, &tag, &payloadType, &indirect);
694+
auto caseName = getEnumMirrorInfo(value, type, &tag, &payloadType, &indirect);
713695

714696
// Copy the enum payload into a box
715697
const Metadata *boxType = (indirect ? &METADATA_SYM(Bo).base : payloadType);
@@ -733,7 +715,7 @@ void swift_EnumMirror_subscript(String *outString,
733715
swift_release(pair.object);
734716
}
735717

736-
new (outString) String(getFieldName(Description.CaseNames, tag));
718+
new (outString) String(caseName);
737719
new (outMirror) Mirror(reflect(owner, value, payloadType));
738720
}
739721

‎test/IRGen/class_resilience.swift

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,6 @@
2727
// CHECK: @"$S16class_resilience30ClassWithIndirectResilientEnumC5colors5Int32VvpWvd" = hidden constant [[INT]] {{12|24}}
2828

2929
// CHECK: [[RESILIENTCHILD_NAME:@.*]] = private constant [15 x i8] c"ResilientChild\00"
30-
// CHECK: [[RESILIENTCHILD_FIELDS:@.*]] = private constant [7 x i8] c"field\00\00"
3130

3231
// CHECK: @"$S16class_resilience14ResilientChildCMn" = {{(protected )?}}constant <{{.*}}> <{
3332
// -- flags: class, unique, has vtable, has resilient superclass
@@ -38,8 +37,6 @@
3837
// CHECK-SAME: i32 1,
3938
// -- field offset vector offset
4039
// CHECK-SAME: i32 3,
41-
// -- field names,
42-
// CHECK-SAME: [7 x i8]* [[RESILIENTCHILD_FIELDS]]
4340
// CHECK-SAME: }>
4441

4542
// CHECK: @"$S16class_resilience14ResilientChildCMo" = {{(protected )?}}global [[INT]] 0

‎test/IRGen/enum.sil

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -104,7 +104,6 @@ import Swift
104104
// The witness table pattern includes extra inhabitant witness
105105
// implementations which are used if the instance has extra inhabitants.
106106
// FIXME: Strings should be unnamed_addr. rdar://problem/22674524
107-
// CHECK: [[DYNAMICSINGLETON_FIELD_NAMES:@.*]] = private constant [7 x i8] c"value\00\00"
108107
// CHECK: @"$S4enum16DynamicSingletonOWV" =
109108
// CHECK-SAME: i8* null
110109
// CHECK-SAME: i8* bitcast (void (%swift.opaque*, i32, %swift.type*)* @"$S4enum16DynamicSingletonOwxs" to i8*)
@@ -117,8 +116,6 @@ import Swift
117116
// CHECK-SAME: i32 1,
118117
// -- No empty cases
119118
// CHECK-SAME: i32 0,
120-
// -- Case names
121-
// CHECK-SAME: [[DYNAMICSINGLETON_FIELD_NAMES]]
122119
// -- Case type accessor
123120
// CHECK-SAME: i32 2,
124121
// -- generic parameters, requirements, key, extra

‎test/IRGen/generic_classes.sil

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,6 @@ import Swift
1313
// -- offset of RootGeneric<T>.x
1414
// FIXME: Strings should be unnamed_addr. rdar://problem/22674524
1515
// CHECK: [[ROOTGENERIC_NAME:@.*]] = private constant [12 x i8] c"RootGeneric\00"
16-
// CHECK: [[ROOTGENERIC_FIELDS:@.*]] = private constant [7 x i8] c"x\00y\00z\00\00"
1716

1817
// CHECK-LABEL: @"$S15generic_classes11RootGenericCMn" =
1918
// -- flags: class, generic, unique, has vtable
@@ -24,8 +23,6 @@ import Swift
2423
// CHECK-SAME: i32 3,
2524
// -- field offset vector offset
2625
// CHECK-SAME: i32 15,
27-
// -- field names
28-
// CHECK-SAME: [7 x i8]* [[ROOTGENERIC_FIELDS]]
2926
// -- generic parameter vector offset
3027
// CHECK-SAME: i32 10,
3128
// -- generic parameters, requirements, key arguments, extra arguments
@@ -63,8 +60,6 @@ import Swift
6360
// CHECK-SAME: i32 3,
6461
// -- -- field offset vector offset
6562
// CHECK-SAME: i32 11,
66-
// -- field names
67-
// CHECK-SAME: [7 x i8]* [[ROOTGENERIC_FIELDS]]
6863
// CHECK-SAME: }>
6964

7065
// CHECK: @"$S15generic_classes14RootNonGenericCMf" = internal global <{ {{.*}} }> <{

‎test/IRGen/generic_structs.sil

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,6 @@ import Builtin
2020

2121
// FIXME: Strings should be unnamed_addr. rdar://problem/22674524
2222
// CHECK: [[SINGLEDYNAMIC_NAME:@.*]] = private constant [14 x i8] c"SingleDynamic\00"
23-
// CHECK: [[SINGLEDYNAMIC_FIELDS:@.*]] = private constant [3 x i8] c"x\00\00"
2423
// CHECK: @"$S15generic_structs13SingleDynamicVMn" = hidden constant
2524
// -- flags: struct, unique, generic
2625
// CHECK-SAME: <i32 0x0000_00D1>
@@ -30,8 +29,6 @@ import Builtin
3029
// CHECK-SAME: i32 1,
3130
// -- field offset vector offset
3231
// CHECK-SAME: i32 3,
33-
// -- field names
34-
// CHECK-SAME: [3 x i8]* [[SINGLEDYNAMIC_FIELDS]]
3532
// -- generic parameter vector offset
3633
// CHECK-SAME: i32 2,
3734
// -- generic params, requirements, key args, extra args
@@ -53,7 +50,6 @@ import Builtin
5350
// -- Nominal type descriptor for generic struct with protocol requirements
5451
// FIXME: Strings should be unnamed_addr. rdar://problem/22674524
5552
// CHECK: [[DYNAMICWITHREQUIREMENTS_NAME:@.*]] = private constant [24 x i8] c"DynamicWithRequirements\00"
56-
// CHECK: [[DYNAMICWITHREQUIREMENTS_FIELDS:@.*]] = private constant [5 x i8] c"x\00y\00\00"
5753
// CHECK: @"$S15generic_structs23DynamicWithRequirementsVMn" = hidden constant <{ {{.*}} i32 }> <{
5854
// -- flags: struct, unique, generic
5955
// CHECK-SAME: <i32 0x0000_00D1>
@@ -63,8 +59,6 @@ import Builtin
6359
// CHECK-SAME: i32 2,
6460
// -- field offset vector offset
6561
// CHECK-SAME: i32 6,
66-
// -- field names
67-
// CHECK-SAME: [5 x i8]* [[DYNAMICWITHREQUIREMENTS_FIELDS]]
6862
// -- generic parameter vector offset
6963
// CHECK-SAME: i32 2,
7064
// -- generic params, requirements, key args, extra args

0 commit comments

Comments
 (0)