Skip to content

Always, no, never... forget to check your references #4060

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Aug 16, 2016
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions include/swift/ABI/MetadataValues.h
Original file line number Diff line number Diff line change
Expand Up @@ -520,6 +520,7 @@ class FieldType {
// some high bits as well.
enum : int_type {
Indirect = 1,
Weak = 2,

TypeMask = ((uintptr_t)-1) & ~(alignof(void*) - 1),
};
Expand All @@ -537,10 +538,19 @@ class FieldType {
| (indirect ? Indirect : 0));
}

constexpr FieldType withWeak(bool weak) const {
return FieldType((Data & ~Weak)
| (weak ? Weak : 0));
}

bool isIndirect() const {
return bool(Data & Indirect);
}

bool isWeak() const {
return bool(Data & Weak);
}

const Metadata *getType() const {
return (const Metadata *)(Data & TypeMask);
}
Expand Down
8 changes: 4 additions & 4 deletions include/swift/Runtime/HeapObject.h
Original file line number Diff line number Diff line change
Expand Up @@ -899,8 +899,8 @@ extern "C" void *swift_unknownWeakLoadStrong(WeakReference *ref);

#else

static inline void swift_unknownWeakLoadStrong(WeakReference *ref) {
swift_weakLoadStrong(ref);
static inline void *swift_unknownWeakLoadStrong(WeakReference *ref) {
return static_cast<void *>(swift_weakLoadStrong(ref));
}

#endif /* SWIFT_OBJC_INTEROP */
Expand All @@ -918,8 +918,8 @@ extern "C" void *swift_unknownWeakTakeStrong(WeakReference *ref);

#else

static inline void swift_unknownWeakTakeStrong(WeakReference *ref) {
swift_weakTakeStrong(ref);
static inline void *swift_unknownWeakTakeStrong(WeakReference *ref) {
return static_cast<void *>(swift_weakTakeStrong(ref));
}

#endif /* SWIFT_OBJC_INTEROP */
Expand Down
16 changes: 11 additions & 5 deletions lib/IRGen/GenMeta.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2414,8 +2414,10 @@ namespace {
NominalTypeDecl::StoredPropertyRange storedProperties){
SmallVector<FieldTypeInfo, 4> types;
for (VarDecl *prop : storedProperties) {
types.push_back(FieldTypeInfo(prop->getType()->getCanonicalType(),
/*indirect*/ false));
auto propertyType = prop->getType()->getCanonicalType();
types.push_back(FieldTypeInfo(propertyType,
/*indirect*/ false,
propertyType->is<WeakStorageType>()));
}
return getFieldTypeAccessorFn(IGM, type, types);
}
Expand All @@ -2431,7 +2433,7 @@ namespace {
auto caseType = elt.decl->getArgumentType()->getCanonicalType();
bool isIndirect = elt.decl->isIndirect()
|| elt.decl->getParentEnum()->isIndirect();
types.push_back(FieldTypeInfo(caseType, isIndirect));
types.push_back(FieldTypeInfo(caseType, isIndirect, /*weak*/ false));
}
return getFieldTypeAccessorFn(IGM, type, types);
}
Expand Down Expand Up @@ -2784,9 +2786,13 @@ irgen::emitFieldTypeAccessor(IRGenModule &IGM,

auto metadata = IGF.emitTypeMetadataRef(fieldTy);

auto fieldTypeInfo = fieldTypes[i];

// Mix in flag bits.
if (fieldTypes[i].isIndirect()) {
auto flags = FieldType().withIndirect(true);
if (fieldTypeInfo.hasFlags()) {
auto flags = FieldType()
.withIndirect(fieldTypeInfo.isIndirect())
.withWeak(fieldTypeInfo.isWeak());
auto metadataBits = IGF.Builder.CreatePtrToInt(metadata, IGF.IGM.SizeTy);
metadataBits = IGF.Builder.CreateOr(metadataBits,
llvm::ConstantInt::get(IGF.IGM.SizeTy, flags.getIntValue()));
Expand Down
15 changes: 10 additions & 5 deletions lib/IRGen/IRGenModule.h
Original file line number Diff line number Diff line change
Expand Up @@ -133,25 +133,30 @@ class IRGenModule;

/// A type descriptor for a field type accessor.
class FieldTypeInfo {
llvm::PointerIntPair<CanType, 1, unsigned> Info;
llvm::PointerIntPair<CanType, 2, unsigned> Info;
/// Bits in the "int" part of the Info pair.
enum : unsigned {
/// Flag indicates that the case is indirectly stored in a box.
Indirect = 1,
/// Indicates a weak optional reference
Weak = 2,
};

static unsigned getFlags(bool indirect) {
return (indirect ? Indirect : 0);
static unsigned getFlags(bool indirect, bool weak) {
return (indirect ? Indirect : 0)
| (weak ? Weak : 0);
// | (blah ? Blah : 0) ...
}

public:
FieldTypeInfo(CanType type, bool indirect)
: Info(type, getFlags(indirect))
FieldTypeInfo(CanType type, bool indirect, bool weak)
: Info(type, getFlags(indirect, weak))
{}

CanType getType() const { return Info.getPointer(); }
bool isIndirect() const { return Info.getInt() & Indirect; }
bool isWeak() const { return Info.getInt() & Weak; }
bool hasFlags() const { return Info.getInt() != 0; }
};

/// The principal singleton which manages all of IR generation.
Expand Down
59 changes: 51 additions & 8 deletions stdlib/public/runtime/Reflection.mm
Original file line number Diff line number Diff line change
Expand Up @@ -398,6 +398,40 @@ void swift_TupleMirror_subscript(String *outString,
return fieldName;
}


static bool loadSpecialReferenceStorage(HeapObject *owner,
OpaqueValue *fieldData,
const FieldType fieldType,
Mirror *outMirror) {
// TODO: Switch over the other kinds of reference storage here:
// - unowned(safe)
// - unowned(unsafe)
// - class-bound existentials
// - Optional existential types
// This will require a change in the two low flag bits in the field type
// returned from the field type accessor generated by IRGen.

// isWeak() implies a reference type via Sema.
if (!fieldType.isWeak())
return false;

auto weakField = reinterpret_cast<WeakReference *>(fieldData);
auto strongValue = swift_unknownWeakLoadStrong(weakField);
fieldData = reinterpret_cast<OpaqueValue *>(&strongValue);

// This MagicMirror constructor creates a box to hold the loaded refernce
// value, which becomes the new owner for the value.
new (outMirror) MagicMirror(fieldData, fieldType.getType(), /*take*/ true);

// However, swift_StructMirror_subscript and swift_ClassMirror_subscript
// requires that the owner be consumed. Since we have the new heap box as the
// owner now, we need to release the old owner to maintain the contract.
if (owner->metadata->isAnyClass())
swift_unknownRelease(owner);

return true;
}

// -- Struct destructuring.

SWIFT_CC(swift) SWIFT_RUNTIME_STDLIB_INTERFACE
Expand All @@ -416,7 +450,7 @@ void swift_StructMirror_subscript(String *outString,
Mirror *outMirror,
intptr_t i,
HeapObject *owner,
const OpaqueValue *value,
OpaqueValue *value,
const Metadata *type) {
auto Struct = static_cast<const StructMetadata *>(type);

Expand All @@ -427,13 +461,17 @@ void swift_StructMirror_subscript(String *outString,
auto fieldType = Struct->getFieldTypes()[i];
auto fieldOffset = Struct->getFieldOffsets()[i];

auto bytes = reinterpret_cast<const char*>(value);
auto fieldData = reinterpret_cast<const OpaqueValue *>(bytes + fieldOffset);
auto bytes = reinterpret_cast<char*>(value);
auto fieldData = reinterpret_cast<OpaqueValue *>(bytes + fieldOffset);

new (outString) String(getFieldName(Struct->Description->Struct.FieldNames, i));

// 'owner' is consumed by this call.
assert(!fieldType.isIndirect() && "indirect struct fields not implemented");

if (loadSpecialReferenceStorage(owner, fieldData, fieldType, outMirror))
return;

new (outMirror) Mirror(reflect(owner, fieldData, fieldType.getType()));
}

Expand Down Expand Up @@ -614,7 +652,7 @@ void swift_ClassMirror_subscript(String *outString,
Mirror *outMirror,
intptr_t i,
HeapObject *owner,
const OpaqueValue *value,
OpaqueValue *value,
const Metadata *type) {
auto Clas = static_cast<const ClassMetadata*>(type);

Expand Down Expand Up @@ -655,10 +693,15 @@ void swift_ClassMirror_subscript(String *outString,
#endif
}

auto bytes = *reinterpret_cast<const char * const*>(value);
auto fieldData = reinterpret_cast<const OpaqueValue *>(bytes + fieldOffset);

new (outString) String(getFieldName(Clas->getDescription()->Class.FieldNames, i));
auto bytes = *reinterpret_cast<char * const *>(value);
auto fieldData = reinterpret_cast<OpaqueValue *>(bytes + fieldOffset);

new (outString) String(getFieldName(Clas->getDescription()->Class.FieldNames,
i));

if (loadSpecialReferenceStorage(owner, fieldData, fieldType, outMirror))
return;

// 'owner' is consumed by this call.
new (outMirror) Mirror(reflect(owner, fieldData, fieldType.getType()));
}
Expand Down
Loading