Skip to content

Give more extra inhabitants to BridgeObject. #20416

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 1 commit into from
Nov 8, 2018
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
33 changes: 0 additions & 33 deletions lib/IRGen/GenObjC.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -262,39 +262,6 @@ namespace {
ReferenceCounting getReferenceCounting() const {
return ReferenceCounting::Bridge;
}

// BridgeObject exposes only null as an extra inhabitant for enum layout.
// Other representations are reserved for future use by the stdlib.

bool mayHaveExtraInhabitants(IRGenModule &IGM) const override {
return true;
}
unsigned getFixedExtraInhabitantCount(IRGenModule &IGM) const override {
return 1;
}
APInt getFixedExtraInhabitantValue(IRGenModule &IGM,
unsigned bits,
unsigned index) const override {
return APInt(bits, 0);
}
llvm::Value *getExtraInhabitantIndex(IRGenFunction &IGF, Address src,
SILType T,
bool isOutlined) const override {
src = IGF.Builder.CreateBitCast(src, IGF.IGM.SizeTy->getPointerTo());
auto val = IGF.Builder.CreateLoad(src);
auto isNonzero = IGF.Builder.CreateICmpNE(val,
llvm::ConstantInt::get(IGF.IGM.SizeTy, 0));
// We either have extra inhabitant 0 or no extra inhabitant (-1).
// Conveniently, this is just a sext i1 -> i32 away.
return IGF.Builder.CreateSExt(isNonzero, IGF.IGM.Int32Ty);
}
void storeExtraInhabitant(IRGenFunction &IGF, llvm::Value *index,
Address dest, SILType T, bool isOutlined)
const override {
// There's only one extra inhabitant, 0.
dest = IGF.Builder.CreateBitCast(dest, IGF.IGM.SizeTy->getPointerTo());
IGF.Builder.CreateStore(llvm::ConstantInt::get(IGF.IGM.SizeTy, 0), dest);
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay, I understand why this works. As long as we're staying out of the reserved bits for tagged pointers (including BridgeObject's own notion of tagged pointers) — which the superclass implementation does — it doesn't matter if we're intruding on the BridgeObject representation because the rest of the pointer still needs to be in the valid range for objects.

};
} // end anonymous namespace

Expand Down
11 changes: 0 additions & 11 deletions stdlib/public/runtime/MetadataImpl.h
Original file line number Diff line number Diff line change
Expand Up @@ -407,24 +407,13 @@ struct UnknownObjectRetainableBox
/// A box implementation class for BridgeObject.
struct BridgeObjectBox :
RetainableBoxBase<BridgeObjectBox, void*> {
// TODO: Enable the nil extra inhabitant.
static constexpr unsigned numExtraInhabitants = 1;

static void *retain(void *obj) {
return swift_bridgeObjectRetain(obj);
}

static void release(void *obj) {
swift_bridgeObjectRelease(obj);
}

static void storeExtraInhabitant(void **dest, int index) {
*dest = nullptr;
}

static int getExtraInhabitantIndex(void* const *src) {
return *src == nullptr ? 0 : -1;
}
};

/// A box implementation class for unmanaged, pointer-aligned pointers.
Expand Down
32 changes: 30 additions & 2 deletions test/Interpreter/struct_extra_inhabitants.swift
Original file line number Diff line number Diff line change
Expand Up @@ -113,13 +113,36 @@ typealias GenericPairWithPointerSecond_AnyObject
var tests = TestSuite("extra inhabitants of structs")

@inline(never)
func expectHasExtraInhabitant<T>(_: T.Type, nil: T?,
func expectHasAtLeastThreeExtraInhabitants<T>(_: T.Type,
nil theNil: T???,
someNil: T???,
someSomeNil: T???,
file: String = #file, line: UInt = #line) {
expectEqual(MemoryLayout<T>.size, MemoryLayout<T???>.size,
"\(T.self) has at least three extra inhabitants",
file: file, line: line)

expectNil(theNil,
"\(T.self) extra inhabitant should agree in generic and concrete " +
"context")

expectNil(someNil!,
"\(T.self) extra inhabitant should agree in generic and concrete " +
"context")

expectNil(someSomeNil!!,
"\(T.self) extra inhabitant should agree in generic and concrete " +
"context")
}

@inline(never)
func expectHasExtraInhabitant<T>(_: T.Type, nil theNil: T?,
file: String = #file, line: UInt = #line) {
expectEqual(MemoryLayout<T>.size, MemoryLayout<T?>.size,
"\(T.self) has extra inhabitant",
file: file, line: line)

expectNil(Optional<T>.none,
expectNil(theNil,
"\(T.self) extra inhabitant should agree in generic and concrete " +
"context")
}
Expand Down Expand Up @@ -167,5 +190,10 @@ tests.test("types that have extra inhabitants") {
expectHasExtraInhabitant(GenericFullHouse<UnsafeRawPointer, UnsafeRawPointer>.self, nil: nil)
}

tests.test("types that have more than one extra inhabitant") {
expectHasAtLeastThreeExtraInhabitants(StringAlike64.self, nil: nil, someNil: .some(nil), someSomeNil: .some(.some(nil)))
expectHasAtLeastThreeExtraInhabitants(StringAlike32.self, nil: nil, someNil: .some(nil), someSomeNil: .some(.some(nil)))
Copy link
Contributor

@jrose-apple jrose-apple Nov 8, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's probably also worth checking that a non-nil object doesn't have the same representation as any of the nils. In particular, _bridgeObject(taggingPayload: 0) and _bridgeObject(taggingPayload: 1) are probably worth checking.

}

runAllTests()

4 changes: 2 additions & 2 deletions validation-test/Reflection/reflect_Array.swift
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ reflect(object: obj)
// CHECK-64: Type info:
// CHECK-64: (class_instance size=24 alignment=8 stride=24 num_extra_inhabitants=0 bitwise_takable=1
// CHECK-64: (field name=t offset=16
// CHECK-64: (struct size=8 alignment=8 stride=8 num_extra_inhabitants=1 bitwise_takable=1
// CHECK-64: (struct size=8 alignment=8 stride=8 num_extra_inhabitants=2147483647 bitwise_takable=1
// (unstable implementation details omitted)

// CHECK-32: Reflecting an object.
Expand All @@ -39,7 +39,7 @@ reflect(object: obj)
// CHECK-32: Type info:
// CHECK-32: (class_instance size=12 alignment=4 stride=12 num_extra_inhabitants=0 bitwise_takable=1
// CHECK-32: (field name=t offset=8
// CHECK-32: (struct size=4 alignment=4 stride=4 num_extra_inhabitants=1 bitwise_takable=1
// CHECK-32: (struct size=4 alignment=4 stride=4 num_extra_inhabitants=4096 bitwise_takable=1
// (unstable implementation details omitted)

doneReflecting()
Expand Down
10 changes: 5 additions & 5 deletions validation-test/Reflection/reflect_Character.swift
Original file line number Diff line number Diff line change
Expand Up @@ -27,21 +27,21 @@ reflect(object: obj)
// CHECK-64-LABEL: Type info:
// CHECK-64: (class_instance size=32 alignment=8 stride=32 num_extra_inhabitants=0 bitwise_takable=1
// CHECK-64-NEXT: (field name=t offset=16
// CHECK-64-NEXT: (struct size=16 alignment=8 stride=16 num_extra_inhabitants=1 bitwise_takable=1
// CHECK-64-NEXT: (struct size=16 alignment=8 stride=16 num_extra_inhabitants=2147483647 bitwise_takable=1
// CHECK-64-NEXT: (field name=_str offset=0
// CHECK-64-NEXT: (struct size=16 alignment=8 stride=16 num_extra_inhabitants=1 bitwise_takable=1
// CHECK-64-NEXT: (struct size=16 alignment=8 stride=16 num_extra_inhabitants=2147483647 bitwise_takable=1
// CHECK-64-NEXT: (field name=_guts offset=0
// CHECK-64-NEXT: (struct size=16 alignment=8 stride=16 num_extra_inhabitants=1 bitwise_takable=1
// CHECK-64-NEXT: (struct size=16 alignment=8 stride=16 num_extra_inhabitants=2147483647 bitwise_takable=1
// CHECK-64-NEXT: (field name=_object offset=0
// CHECK-64-NEXT: (struct size=16 alignment=8 stride=16 num_extra_inhabitants=1 bitwise_takable=1
// CHECK-64-NEXT: (struct size=16 alignment=8 stride=16 num_extra_inhabitants=2147483647 bitwise_takable=1
// CHECK-64-NEXT: (field name=_countAndFlags offset=0
// CHECK-64-NEXT: (struct size=8 alignment=8 stride=8 num_extra_inhabitants=0 bitwise_takable=1
// CHECK-64-NEXT: (field name=_storage offset=0
// CHECK-64-NEXT: (struct size=8 alignment=8 stride=8 num_extra_inhabitants=0 bitwise_takable=1
// CHECK-64-NEXT: (field name=_value offset=0
// CHECK-64-NEXT: (builtin size=8 alignment=8 stride=8 num_extra_inhabitants=0 bitwise_takable=1))))))
// CHECK-64-NEXT: (field name=_object offset=8
// CHECK-64-NEXT: (builtin size=8 alignment=8 stride=8 num_extra_inhabitants=1 bitwise_takable=1)))))))))))
// CHECK-64-NEXT: (builtin size=8 alignment=8 stride=8 num_extra_inhabitants=2147483647 bitwise_takable=1)))))))))))

// CHECK-32: Reflecting an object.
// CHECK-32: Type reference:
Expand Down
4 changes: 2 additions & 2 deletions validation-test/Reflection/reflect_Dictionary.swift
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ reflect(object: obj)
// CHECK-64: Type info:
// CHECK-64: (class_instance size=24 alignment=8 stride=24 num_extra_inhabitants=0 bitwise_takable=1
// CHECK-64: (field name=t offset=16
// CHECK-64: (struct size=8 alignment=8 stride=8 num_extra_inhabitants=1 bitwise_takable=1
// CHECK-64: (struct size=8 alignment=8 stride=8 num_extra_inhabitants=2147483647 bitwise_takable=1
// (unstable implementation details omitted)

// CHECK-32: Reflecting an object.
Expand All @@ -39,7 +39,7 @@ reflect(object: obj)
// CHECK-32: Type info:
// CHECK-32: (class_instance size=12 alignment=4 stride=12 num_extra_inhabitants=0 bitwise_takable=1
// CHECK-32: (field name=t offset=8
// CHECK-32: (struct size=4 alignment=4 stride=4 num_extra_inhabitants=1 bitwise_takable=1
// CHECK-32: (struct size=4 alignment=4 stride=4 num_extra_inhabitants=4096 bitwise_takable=1
// (unstable implementation details omitted)

doneReflecting()
Expand Down
4 changes: 2 additions & 2 deletions validation-test/Reflection/reflect_Set.swift
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ reflect(object: obj)
// CHECK-64: Type info:
// CHECK-64: (class_instance size=24 alignment=8 stride=24 num_extra_inhabitants=0 bitwise_takable=1
// CHECK-64: (field name=t offset=16
// CHECK-64: (struct size=8 alignment=8 stride=8 num_extra_inhabitants=1 bitwise_takable=1
// CHECK-64: (struct size=8 alignment=8 stride=8 num_extra_inhabitants=2147483647 bitwise_takable=1
// (unstable implementation details omitted)

// CHECK-32: Reflecting an object.
Expand All @@ -39,7 +39,7 @@ reflect(object: obj)
// CHECK-32: Type info:
// CHECK-32: (class_instance size=12 alignment=4 stride=12 num_extra_inhabitants=0 bitwise_takable=1
// CHECK-32: (field name=t offset=8
// CHECK-32: (struct size=4 alignment=4 stride=4 num_extra_inhabitants=1 bitwise_takable=1
// CHECK-32: (struct size=4 alignment=4 stride=4 num_extra_inhabitants=4096 bitwise_takable=1
// (unstable implementation details omitted)

doneReflecting()
Expand Down
2 changes: 1 addition & 1 deletion validation-test/Reflection/reflect_String.swift
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ reflect(object: obj)
// CHECK-64: Type info:
// CHECK-64-NEXT: (class_instance size=32 alignment=8 stride=32 num_extra_inhabitants=0 bitwise_takable=1
// CHECK-64-NEXT: (field name=t offset=16
// CHECK-64-NEXT: (struct size=16 alignment=8 stride=16 num_extra_inhabitants=1 bitwise_takable=1
// CHECK-64-NEXT: (struct size=16 alignment=8 stride=16 num_extra_inhabitants=2147483647 bitwise_takable=1
// (unstable implementation details omitted)

// CHECK-32: Reflecting an object.
Expand Down
24 changes: 12 additions & 12 deletions validation-test/Reflection/reflect_multiple_types.swift
Original file line number Diff line number Diff line change
Expand Up @@ -117,32 +117,32 @@ reflect(object: obj)
// CHECK-64-LABEL: Type info:
// CHECK-64-NEXT: (class_instance size=185 alignment=8 stride=192 num_extra_inhabitants=0 bitwise_takable=1
// CHECK-64-NEXT: (field name=t00 offset=16
// CHECK-64-NEXT: (struct size=8 alignment=8 stride=8 num_extra_inhabitants=1 bitwise_takable=1
// CHECK-64-NEXT: (struct size=8 alignment=8 stride=8 num_extra_inhabitants=2147483647 bitwise_takable=1
// (unstable implementation details omitted)
// CHECK-64: (field name=t01 offset=24
// CHECK-64-NEXT: (struct size=1 alignment=1 stride=1 num_extra_inhabitants=254 bitwise_takable=1
// CHECK-64-NEXT: (field name=_value offset=0
// CHECK-64-NEXT: (builtin size=1 alignment=1 stride=1 num_extra_inhabitants=254 bitwise_takable=1))))

// CHECK-64-NEXT: (field name=t02 offset=32
// CHECK-64-NEXT: (struct size=16 alignment=8 stride=16 num_extra_inhabitants=1 bitwise_takable=1
// CHECK-64-NEXT: (struct size=16 alignment=8 stride=16 num_extra_inhabitants=2147483647 bitwise_takable=1
// CHECK-64-NEXT: (field name=_str offset=0
// CHECK-64-NEXT: (struct size=16 alignment=8 stride=16 num_extra_inhabitants=1 bitwise_takable=1
// CHECK-64-NEXT: (struct size=16 alignment=8 stride=16 num_extra_inhabitants=2147483647 bitwise_takable=1
// CHECK-64-NEXT: (field name=_guts offset=0
// CHECK-64-NEXT: (struct size=16 alignment=8 stride=16 num_extra_inhabitants=1 bitwise_takable=1
// CHECK-64-NEXT: (struct size=16 alignment=8 stride=16 num_extra_inhabitants=2147483647 bitwise_takable=1
// CHECK-64-NEXT: (field name=_object offset=0
// CHECK-64-NEXT: (struct size=16 alignment=8 stride=16 num_extra_inhabitants=1 bitwise_takable=1
// CHECK-64-NEXT: (struct size=16 alignment=8 stride=16 num_extra_inhabitants=2147483647 bitwise_takable=1
// CHECK-64-NEXT: (field name=_countAndFlags offset=0
// CHECK-64-NEXT: (struct size=8 alignment=8 stride=8 num_extra_inhabitants=0 bitwise_takable=1
// CHECK-64-NEXT: (field name=_storage offset=0
// CHECK-64-NEXT: (struct size=8 alignment=8 stride=8 num_extra_inhabitants=0 bitwise_takable=1
// CHECK-64-NEXT: (field name=_value offset=0
// CHECK-64-NEXT: (builtin size=8 alignment=8 stride=8 num_extra_inhabitants=0 bitwise_takable=1))))))
// CHECK-64-NEXT: (field name=_object offset=8
// CHECK-64-NEXT: (builtin size=8 alignment=8 stride=8 num_extra_inhabitants=1 bitwise_takable=1))))))))))
// CHECK-64-NEXT: (builtin size=8 alignment=8 stride=8 num_extra_inhabitants=2147483647 bitwise_takable=1))))))))))

// CHECK-64-NEXT: (field name=t03 offset=48
// CHECK-64-NEXT: (struct size=8 alignment=8 stride=8 num_extra_inhabitants=1 bitwise_takable=1
// CHECK-64-NEXT: (struct size=8 alignment=8 stride=8 num_extra_inhabitants=2147483647 bitwise_takable=1
// (unstable implementation details omitted)
// CHECK-64: (field name=t04 offset=56
// CHECK-64-NEXT: (struct size=8 alignment=8 stride=8 num_extra_inhabitants=0 bitwise_takable=1
Expand Down Expand Up @@ -181,12 +181,12 @@ reflect(object: obj)
// CHECK-64-NEXT: (field name=t14 offset=128
// CHECK-64-NEXT: (reference kind=strong refcounting=unknown))
// CHECK-64-NEXT: (field name=t15 offset=136
// CHECK-64-NEXT: (struct size=8 alignment=8 stride=8 num_extra_inhabitants=1 bitwise_takable=1
// CHECK-64-NEXT: (struct size=8 alignment=8 stride=8 num_extra_inhabitants=2147483647 bitwise_takable=1

// (unstable implementation details omitted)

// CHECK-64: (field name=t16 offset=144
// CHECK-64-NEXT: (struct size=16 alignment=8 stride=16 num_extra_inhabitants=1 bitwise_takable=1
// CHECK-64-NEXT: (struct size=16 alignment=8 stride=16 num_extra_inhabitants=2147483647 bitwise_takable=1
// (unstable implementation details omitted)

// CHECK-64: (field name=t17 offset=160
Expand Down Expand Up @@ -218,7 +218,7 @@ reflect(object: obj)
// CHECK-32: Type info:
// CHECK-32-NEXT: (class_instance size=121 alignment=8 stride=128 num_extra_inhabitants=0 bitwise_takable=1
// CHECK-32-NEXT: (field name=t00 offset=8
// CHECK-32-NEXT: (struct size=4 alignment=4 stride=4 num_extra_inhabitants=1 bitwise_takable=1
// CHECK-32-NEXT: (struct size=4 alignment=4 stride=4 num_extra_inhabitants=4096 bitwise_takable=1
// (unstable implementation details omitted)
// CHECK-32: (field name=t01 offset=12
// CHECK-32-NEXT: (struct size=1 alignment=1 stride=1 num_extra_inhabitants=254 bitwise_takable=1
Expand All @@ -229,7 +229,7 @@ reflect(object: obj)
// CHECK-32-NEXT: (field name=_str offset=0
// (unstable implementation details omitted)
// CHECK-32: (field name=t03 offset=28
// CHECK-32-NEXT: (struct size=4 alignment=4 stride=4 num_extra_inhabitants=1 bitwise_takable=1
// CHECK-32-NEXT: (struct size=4 alignment=4 stride=4 num_extra_inhabitants=4096 bitwise_takable=1
// (unstable implementation details omitted)
// CHECK-32: (field name=t04 offset=32
// CHECK-32-NEXT: (struct size=8 alignment=8 stride=8 num_extra_inhabitants=0 bitwise_takable=1
Expand Down Expand Up @@ -268,7 +268,7 @@ reflect(object: obj)
// CHECK-32-NEXT: (field name=t14 offset=80
// CHECK-32-NEXT: (reference kind=strong refcounting=unknown))
// CHECK-32-NEXT: (field name=t15 offset=84
// CHECK-32-NEXT: (struct size=4 alignment=4 stride=4 num_extra_inhabitants=1 bitwise_takable=1
// CHECK-32-NEXT: (struct size=4 alignment=4 stride=4 num_extra_inhabitants=4096 bitwise_takable=1
// (unstable implementation details omitted)
// CHECK-32: (field name=t16 offset=88
// CHECK-32-NEXT: (struct size=12 alignment=4 stride=12 num_extra_inhabitants=128 bitwise_takable=1
Expand Down