Skip to content

Commit e2cf23d

Browse files
committed
Reflection: Don't emit builtin descriptors for imported classes
Previously we would emit both a builtin descriptor and field descriptor for imported classes, but we only need the latter. Untangle some code and fix a crash with imported Objective-C generics in the process. Fixes <rdar://problem/26498484>.
1 parent 72e3086 commit e2cf23d

File tree

8 files changed

+105
-130
lines changed

8 files changed

+105
-130
lines changed

include/swift/Reflection/Records.h

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -104,14 +104,25 @@ struct FieldRecordIterator {
104104
};
105105

106106
enum class FieldDescriptorKind : uint16_t {
107+
// Swift nominal types.
107108
Struct,
108109
Class,
109110
Enum,
111+
112+
// A Swift opaque protocol. There are no fields, just a record for the
113+
// type itself.
110114
Protocol,
115+
116+
// A Swift class-bound protocol.
111117
ClassProtocol,
118+
119+
// An Objective-C protocol, which may be imported or defined in Swift.
112120
ObjCProtocol,
113-
ObjCClass,
114-
Imported
121+
122+
// An Objective-C class, which may be imported or defined in Swift.
123+
// In the former case, field type metadata is not emitted, and
124+
// must be obtained from the Objective-C runtime.
125+
ObjCClass
115126
};
116127

117128
// Field descriptors contain a collection of field records for a single

lib/IRGen/GenReflection.cpp

Lines changed: 39 additions & 53 deletions
Original file line numberDiff line numberDiff line change
@@ -174,20 +174,28 @@ class PrintMetadataSource
174174

175175
class ReflectionMetadataBuilder : public ConstantBuilder<> {
176176
protected:
177-
llvm::SetVector<CanType> &BuiltinTypes;
178177

179178
// Collect any builtin types referenced from this type.
180179
void addBuiltinTypeRefs(CanType type) {
181180
type.visit([&](Type t) {
182181
if (t->is<BuiltinType>())
183-
BuiltinTypes.insert(CanType(t));
184-
185-
// We need at least size/alignment information for types imported by
186-
// clang, so we'll treat them as a builtin type, essentially an
187-
// opaque blob.
182+
IGM.BuiltinTypes.insert(CanType(t));
183+
184+
// We need size/alignment information for imported value types,
185+
// so emit builtin descriptors for them.
186+
//
187+
// In effect they're treated like an opaque blob, which is OK
188+
// for now, at least until we want to import C++ types or
189+
// something like that.
190+
//
191+
// Classes go down a different path.
188192
if (auto Nominal = t->getAnyNominal())
189-
if (Nominal->hasClangNode())
190-
BuiltinTypes.insert(CanType(t));
193+
if (Nominal->hasClangNode()) {
194+
if (auto CD = dyn_cast<ClassDecl>(Nominal))
195+
IGM.ImportedClasses.insert(CD);
196+
else
197+
IGM.BuiltinTypes.insert(CanType(t));
198+
}
191199
});
192200
}
193201

@@ -210,9 +218,8 @@ class ReflectionMetadataBuilder : public ConstantBuilder<> {
210218
}
211219

212220
public:
213-
ReflectionMetadataBuilder(IRGenModule &IGM,
214-
llvm::SetVector<CanType> &BuiltinTypes)
215-
: ConstantBuilder(IGM), BuiltinTypes(BuiltinTypes) {}
221+
ReflectionMetadataBuilder(IRGenModule &IGM)
222+
: ConstantBuilder(IGM) {}
216223
};
217224

218225
class AssociatedTypeMetadataBuilder : public ReflectionMetadataBuilder {
@@ -294,15 +301,11 @@ class AssociatedTypeMetadataBuilder : public ReflectionMetadataBuilder {
294301
}
295302

296303
public:
297-
AssociatedTypeMetadataBuilder(IRGenModule &IGM, const NominalTypeDecl *Decl,
298-
llvm::SetVector<CanType> &BuiltinTypes)
299-
: ReflectionMetadataBuilder(IGM, BuiltinTypes),
300-
NominalOrExtensionDecl(Decl) {}
304+
AssociatedTypeMetadataBuilder(IRGenModule &IGM, const NominalTypeDecl *Decl)
305+
: ReflectionMetadataBuilder(IGM), NominalOrExtensionDecl(Decl) {}
301306

302-
AssociatedTypeMetadataBuilder(IRGenModule &IGM, const ExtensionDecl *Decl,
303-
llvm::SetVector<CanType> &BuiltinTypes)
304-
: ReflectionMetadataBuilder(IGM, BuiltinTypes),
305-
NominalOrExtensionDecl(Decl) {}
307+
AssociatedTypeMetadataBuilder(IRGenModule &IGM, const ExtensionDecl *Decl)
308+
: ReflectionMetadataBuilder(IGM), NominalOrExtensionDecl(Decl) {}
306309

307310

308311
llvm::GlobalVariable *emit() {
@@ -362,12 +365,6 @@ class FieldTypeMetadataBuilder : public ReflectionMetadataBuilder {
362365
addConstantInt32(0);
363366
}
364367

365-
void addImportedRecord() {
366-
addConstantInt16(uint16_t(FieldDescriptorKind::Imported));
367-
addConstantInt16(0);
368-
addConstantInt32(0);
369-
}
370-
371368
void layout() {
372369
using swift::reflection::FieldDescriptorKind;
373370

@@ -383,10 +380,8 @@ class FieldTypeMetadataBuilder : public ReflectionMetadataBuilder {
383380
}
384381
}
385382

386-
if (NTD->hasClangNode()) {
387-
addImportedRecord();
383+
if (NTD->hasClangNode())
388384
return;
389-
}
390385

391386
switch (NTD->getKind()) {
392387
case DeclKind::Class:
@@ -442,10 +437,8 @@ class FieldTypeMetadataBuilder : public ReflectionMetadataBuilder {
442437

443438
public:
444439
FieldTypeMetadataBuilder(IRGenModule &IGM,
445-
const NominalTypeDecl * NTD,
446-
llvm::SetVector<CanType> &BuiltinTypes)
447-
: ReflectionMetadataBuilder(IGM, BuiltinTypes),
448-
NTD(NTD) {}
440+
const NominalTypeDecl * NTD)
441+
: ReflectionMetadataBuilder(IGM), NTD(NTD) {}
449442

450443
llvm::GlobalVariable *emit() {
451444

@@ -487,15 +480,14 @@ class BuiltinTypeMetadataBuilder : public ReflectionMetadataBuilder {
487480
}
488481

489482
void layout() {
490-
for (auto builtinType : BuiltinTypes) {
483+
for (auto builtinType : IGM.BuiltinTypes) {
491484
addBuiltinType(builtinType);
492485
}
493486
}
494487

495488
public:
496-
BuiltinTypeMetadataBuilder(IRGenModule &IGM,
497-
llvm::SetVector<CanType> &BuiltinTypes)
498-
: ReflectionMetadataBuilder(IGM, BuiltinTypes) {}
489+
BuiltinTypeMetadataBuilder(IRGenModule &IGM)
490+
: ReflectionMetadataBuilder(IGM) {}
499491

500492
llvm::GlobalVariable *emit() {
501493

@@ -531,11 +523,8 @@ class BuiltinTypeMetadataBuilder : public ReflectionMetadataBuilder {
531523
class BoxDescriptorBuilder : public ReflectionMetadataBuilder {
532524
CanType BoxedType;
533525
public:
534-
BoxDescriptorBuilder(IRGenModule &IGM,
535-
llvm::SetVector<CanType> &BuiltinTypes,
536-
CanType BoxedType)
537-
: ReflectionMetadataBuilder(IGM, BuiltinTypes),
538-
BoxedType(BoxedType) {}
526+
BoxDescriptorBuilder(IRGenModule &IGM, CanType BoxedType)
527+
: ReflectionMetadataBuilder(IGM), BoxedType(BoxedType) {}
539528

540529
void layout() {
541530
addConstantInt32(1);
@@ -585,13 +574,12 @@ class CaptureDescriptorBuilder : public ReflectionMetadataBuilder {
585574
const HeapLayout &Layout;
586575
public:
587576
CaptureDescriptorBuilder(IRGenModule &IGM,
588-
llvm::SetVector<CanType> &BuiltinTypes,
589577
SILFunction &Caller,
590578
CanSILFunctionType OrigCalleeType,
591579
CanSILFunctionType SubstCalleeType,
592580
ArrayRef<Substitution> Subs,
593581
const HeapLayout &Layout)
594-
: ReflectionMetadataBuilder(IGM, BuiltinTypes),
582+
: ReflectionMetadataBuilder(IGM),
595583
Caller(Caller), OrigCalleeType(OrigCalleeType),
596584
SubstCalleeType(SubstCalleeType), Subs(Subs),
597585
Layout(Layout) {}
@@ -819,7 +807,7 @@ IRGenModule::getAddrOfBoxDescriptor(CanType BoxedType) {
819807
if (!IRGen.Opts.EnableReflectionMetadata)
820808
return llvm::Constant::getNullValue(CaptureDescriptorPtrTy);
821809

822-
BoxDescriptorBuilder builder(*this, BuiltinTypes, BoxedType);
810+
BoxDescriptorBuilder builder(*this, BoxedType);
823811

824812
auto var = builder.emit();
825813
if (var)
@@ -837,7 +825,7 @@ IRGenModule::getAddrOfCaptureDescriptor(SILFunction &Caller,
837825
if (!IRGen.Opts.EnableReflectionMetadata)
838826
return llvm::Constant::getNullValue(CaptureDescriptorPtrTy);
839827

840-
CaptureDescriptorBuilder builder(*this, BuiltinTypes, Caller,
828+
CaptureDescriptorBuilder builder(*this, Caller,
841829
OrigCalleeType, SubstCalleeType, Subs,
842830
Layout);
843831

@@ -860,7 +848,7 @@ void IRGenModule::emitAssociatedTypeMetadataRecord(const NominalTypeDecl *Decl){
860848
if (!IRGen.Opts.EnableReflectionMetadata)
861849
return;
862850

863-
AssociatedTypeMetadataBuilder builder(*this, Decl, BuiltinTypes);
851+
AssociatedTypeMetadataBuilder builder(*this, Decl);
864852
auto var = builder.emit();
865853
if (var)
866854
addUsedGlobal(var);
@@ -870,7 +858,7 @@ void IRGenModule::emitAssociatedTypeMetadataRecord(const ExtensionDecl *Ext) {
870858
if (!IRGen.Opts.EnableReflectionMetadata)
871859
return;
872860

873-
AssociatedTypeMetadataBuilder builder(*this, Ext, BuiltinTypes);
861+
AssociatedTypeMetadataBuilder builder(*this, Ext);
874862
auto var = builder.emit();
875863
if (var)
876864
addUsedGlobal(var);
@@ -885,12 +873,10 @@ void IRGenModule::emitBuiltinReflectionMetadata() {
885873
BuiltinTypes.insert(Context.TheUnsafeValueBufferType);
886874
}
887875

888-
for (auto Ty : BuiltinTypes)
889-
if (auto Nominal = Ty.getAnyNominal())
890-
if (Nominal->hasClangNode())
891-
emitFieldMetadataRecord(Nominal);
876+
for (auto CD : ImportedClasses)
877+
emitFieldMetadataRecord(CD);
892878

893-
BuiltinTypeMetadataBuilder builder(*this, BuiltinTypes);
879+
BuiltinTypeMetadataBuilder builder(*this);
894880
auto var = builder.emit();
895881
if (var)
896882
addUsedGlobal(var);
@@ -900,7 +886,7 @@ void IRGenModule::emitFieldMetadataRecord(const NominalTypeDecl *Decl) {
900886
if (!IRGen.Opts.EnableReflectionMetadata)
901887
return;
902888

903-
FieldTypeMetadataBuilder builder(*this, Decl, BuiltinTypes);
889+
FieldTypeMetadataBuilder builder(*this, Decl);
904890
auto var = builder.emit();
905891
if (var)
906892
addUsedGlobal(var);

lib/IRGen/IRGenModule.h

Lines changed: 30 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -619,27 +619,6 @@ class IRGenModule {
619619
llvm::Constant *emitProtocolConformances();
620620
llvm::Constant *emitTypeMetadataRecords();
621621

622-
// Remote reflection metadata
623-
void emitReflectionMetadata(const NominalTypeDecl *Decl);
624-
void emitAssociatedTypeMetadataRecord(const NominalTypeDecl *Decl);
625-
void emitAssociatedTypeMetadataRecord(const ExtensionDecl *Ext);
626-
void emitFieldMetadataRecord(const NominalTypeDecl *Decl);
627-
void emitBuiltinReflectionMetadata();
628-
llvm::Constant *getAddrOfStringForTypeRef(StringRef Str);
629-
llvm::Constant *getAddrOfFieldName(StringRef Name);
630-
llvm::Constant *getAddrOfCaptureDescriptor(SILFunction &caller,
631-
CanSILFunctionType origCalleeType,
632-
CanSILFunctionType substCalleeType,
633-
ArrayRef<Substitution> subs,
634-
const HeapLayout &layout);
635-
llvm::Constant *getAddrOfBoxDescriptor(CanType boxedType);
636-
std::string getBuiltinTypeMetadataSectionName();
637-
std::string getFieldTypeMetadataSectionName();
638-
std::string getAssociatedTypeMetadataSectionName();
639-
std::string getCaptureDescriptorMetadataSectionName();
640-
std::string getReflectionStringsSectionName();
641-
std::string getReflectionTypeRefSectionName();
642-
643622
llvm::Constant *getOrCreateHelperFunction(StringRef name,
644623
llvm::Type *resultType,
645624
ArrayRef<llvm::Type*> paramTypes,
@@ -694,9 +673,6 @@ class IRGenModule {
694673
SmallVector<NormalProtocolConformance *, 4> ProtocolConformances;
695674
/// List of nominal types to generate type metadata records for.
696675
SmallVector<CanType, 4> RuntimeResolvableTypes;
697-
/// Builtin types referenced by types in this module when emitting
698-
/// reflection metadata.
699-
llvm::SetVector<CanType> BuiltinTypes;
700676
/// List of ExtensionDecls corresponding to the generated
701677
/// categories.
702678
SmallVector<ExtensionDecl*, 4> ObjCCategoryDecls;
@@ -737,6 +713,36 @@ class IRGenModule {
737713
void emitAutolinkInfo();
738714
void cleanupClangCodeGenMetadata();
739715

716+
//--- Remote reflection metadata --------------------------------------------
717+
public:
718+
/// Builtin types referenced by types in this module when emitting
719+
/// reflection metadata.
720+
llvm::SetVector<CanType> BuiltinTypes;
721+
/// Imported classes referenced by types in this module when emitting
722+
/// reflection metadata.
723+
llvm::SetVector<ClassDecl *> ImportedClasses;
724+
725+
llvm::Constant *getAddrOfStringForTypeRef(StringRef Str);
726+
llvm::Constant *getAddrOfFieldName(StringRef Name);
727+
llvm::Constant *getAddrOfCaptureDescriptor(SILFunction &caller,
728+
CanSILFunctionType origCalleeType,
729+
CanSILFunctionType substCalleeType,
730+
ArrayRef<Substitution> subs,
731+
const HeapLayout &layout);
732+
llvm::Constant *getAddrOfBoxDescriptor(CanType boxedType);
733+
734+
void emitReflectionMetadata(const NominalTypeDecl *Decl);
735+
void emitAssociatedTypeMetadataRecord(const NominalTypeDecl *Decl);
736+
void emitAssociatedTypeMetadataRecord(const ExtensionDecl *Ext);
737+
void emitFieldMetadataRecord(const NominalTypeDecl *Decl);
738+
void emitBuiltinReflectionMetadata();
739+
std::string getBuiltinTypeMetadataSectionName();
740+
std::string getFieldTypeMetadataSectionName();
741+
std::string getAssociatedTypeMetadataSectionName();
742+
std::string getCaptureDescriptorMetadataSectionName();
743+
std::string getReflectionStringsSectionName();
744+
std::string getReflectionTypeRefSectionName();
745+
740746
//--- Runtime ---------------------------------------------------------------
741747
public:
742748
llvm::Constant *getEmptyTupleMetadata();

stdlib/public/Reflection/TypeLowering.cpp

Lines changed: 9 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -239,7 +239,6 @@ class ExistentialTypeInfoBuilder {
239239
case FieldDescriptorKind::Protocol:
240240
WitnessTableCount++;
241241
continue;
242-
case FieldDescriptorKind::Imported:
243242
case FieldDescriptorKind::ObjCClass:
244243
case FieldDescriptorKind::Struct:
245244
case FieldDescriptorKind::Enum:
@@ -745,8 +744,16 @@ class LowerType
745744

746745
const TypeInfo *visitAnyNominalTypeRef(const TypeRef *TR) {
747746
const FieldDescriptor *FD = TC.getBuilder().getFieldTypeInfo(TR);
748-
if (FD == nullptr)
747+
if (FD == nullptr) {
748+
// Maybe this type is opaque -- look for a builtin
749+
// descriptor to see if we at least know its size
750+
// and alignment.
751+
if (auto ImportedTypeDescriptor = TC.getBuilder().getBuiltinTypeInfo(TR))
752+
return TC.makeTypeInfo<BuiltinTypeInfo>(ImportedTypeDescriptor);
753+
754+
// Otherwise, we're out of luck.
749755
return nullptr;
756+
}
750757

751758
switch (FD->Kind) {
752759
case FieldDescriptorKind::Class:
@@ -801,15 +808,6 @@ class LowerType
801808

802809
return nullptr;
803810
}
804-
case FieldDescriptorKind::Imported:
805-
// Imported types are represented as a builtin type, an opaque blob with
806-
// some size, alignment, etc. If we find it in the builtins, we'll use
807-
// that information.
808-
//
809-
// FIXME: Emit field information for imported record types?
810-
if (auto ImportedTypeDescriptor = TC.getBuilder().getBuiltinTypeInfo(TR))
811-
return TC.makeTypeInfo<BuiltinTypeInfo>(ImportedTypeDescriptor);
812-
return nullptr;
813811
case FieldDescriptorKind::ObjCClass:
814812
return TC.getReferenceTypeInfo(ReferenceKind::Strong,
815813
ReferenceCounting::Unknown);
@@ -1032,7 +1030,6 @@ const TypeInfo *TypeConverter::getClassInstanceTypeInfo(const TypeRef *TR,
10321030
case FieldDescriptorKind::ObjCProtocol:
10331031
case FieldDescriptorKind::ClassProtocol:
10341032
case FieldDescriptorKind::Protocol:
1035-
case FieldDescriptorKind::Imported:
10361033
case FieldDescriptorKind::ObjCClass:
10371034
// Invalid field descriptor.
10381035
return nullptr;

test/Reflection/Inputs/ObjectiveCTypes.swift

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ public class GenericOC<T> : NSObject {
2626
public class HasObjCClasses {
2727
let url = NSURL()
2828
let integer = NSInteger()
29+
let rect = NSRect(x: 0, y: 1, width: 2, height: 3)
2930
}
3031

3132
public func closureHasObjCClasses(b: NSBundle) -> () -> () {

0 commit comments

Comments
 (0)