Skip to content
Open
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
7 changes: 7 additions & 0 deletions include/swift/AST/ExistentialLayout.h
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,13 @@ struct ExistentialLayout {
/// Determine whether this refers to any non-marker protocols.
bool containsNonMarkerProtocols() const;

/// Is the memory representation of this layout ABI-compatible with another?
///
/// ABI compatability primarily has to do with which protocol's witness tables
/// will exist in the layout. Having any differences in terms of which witness
/// tables are present is an ABI difference.
bool isABICompatibleWith(ExistentialLayout const &other) const;

ArrayRef<ParameterizedProtocolType *> getParameterizedProtocols() const & {
return parameterized;
}
Expand Down
16 changes: 16 additions & 0 deletions lib/AST/Type.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1230,6 +1230,22 @@ bool ExistentialLayout::containsNonMarkerProtocols() const {
return false;
}

bool ExistentialLayout::isABICompatibleWith(
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not sure I follow. There are other situations where two existential types are ABI compatible but not the same type in the compiler, for example P & AnyObject vs P & C for an arbitrary class C. Does the compiler also enter an infinite loop in that case?

ExistentialLayout const &other) const {
/// All non-marker protocols are expected to have witness tables.
auto hasWitnessTable = [](ProtocolDecl const *proto) {
return !proto->isMarkerProtocol();
};
auto selfProtos = make_filter_range(getProtocols(), hasWitnessTable);
auto otherProtos = make_filter_range(other.getProtocols(), hasWitnessTable);

// Ensure the number of non-marker protocols are the same and that they're
// the same ProtocolDecl* according to operator==.
//
// We rely on the relative order of protocols being canonical.
return llvm::equal(selfProtos, otherProtos);
}

LayoutConstraint ExistentialLayout::getLayoutConstraint() const {
if (hasExplicitAnyObject) {
return LayoutConstraint::getLayoutConstraint(
Expand Down
14 changes: 13 additions & 1 deletion lib/SIL/IR/TypeLowering.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
#include "swift/AST/Decl.h"
#include "swift/AST/DiagnosticEngine.h"
#include "swift/AST/DiagnosticsSIL.h"
#include "swift/AST/ExistentialLayout.h"
#include "swift/AST/Expr.h"
#include "swift/AST/FileUnit.h"
#include "swift/AST/GenericEnvironment.h"
Expand All @@ -43,8 +44,8 @@
#include "swift/SIL/SILInstruction.h"
#include "swift/SIL/SILModule.h"
#include "swift/SIL/Test.h"
#include "clang/AST/Type.h"
#include "clang/AST/Decl.h"
#include "clang/AST/Type.h"
#include "llvm/Support/Compiler.h"
#include "llvm/Support/Debug.h"

Expand Down Expand Up @@ -4914,6 +4915,17 @@ TypeConverter::checkForABIDifferences(SILModule &M,
}
}

// Existentials are ABI-compatible if their layouts are compatible.
if (!optionalityChange) {
auto ct1 = type1.getASTType();
auto ct2 = type2.getASTType();
if (ct1->isExistentialType() && ct2->isExistentialType() &&
ct1->getExistentialLayout().isABICompatibleWith(
Copy link
Contributor

Choose a reason for hiding this comment

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

If dropping marker protocols is the correct fix here, I would prefer you used the existing withoutMarkerProtocols() (which then allows you to compare for canonical type equality after applying it to both sides) instead of adding a new method to ExistentialLayout that is only used in one place. withoutMarkerProtocols() is already used in a number of places in the compiler.

Copy link
Contributor

Choose a reason for hiding this comment

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

I second this, we have a tendency to add a lot of things that essentially do the same thing but with slight differences and it's sometimes really hard to use these APIs. withoutMarkerProtocols seems appropriate here, we use that in the Sema for the same purpose.

Copy link
Contributor

Choose a reason for hiding this comment

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

Although perhaps we should sink it down to TypeBase from ProtocolCompositionType, so that it can properly handle a single ProtocolType that's a marker protocol

ct2->getExistentialLayout())) {
return ABIDifference::CompatibleRepresentation;
}
}

// Tuple types are ABI-compatible if their elements are.
if (!optionalityChange) {
if (auto tuple1 = type1.getAs<TupleType>()) {
Expand Down
12 changes: 4 additions & 8 deletions lib/SILGen/SILGenDynamicCast.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -417,19 +417,15 @@ adjustForConditionalCheckedCastOperand(SILLocation loc, ManagedValue src,
if (!hasAbstraction && (!requiresAddress || src.getType().isAddress()))
return src;

TemporaryInitializationPtr init;
if (requiresAddress) {
init = SGF.emitTemporary(loc, srcAbstractTL);

if (hasAbstraction)
src = SGF.emitSubstToOrigValue(loc, src, abstraction, sourceType);

// Okay, if all we need to do is drop the value in an address,
// this is easy.
SGF.B.emitStoreValueOperation(loc, src.forward(SGF), init->getAddress(),
StoreOwnershipQualifier::Init);
init->finishInitialization(SGF);
return init->getManagedAddress();
if (src.getType().isAddress())
return src;
Copy link
Member Author

Choose a reason for hiding this comment

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

Looks like these changes fix compiler_crashers/SILBuilder-emitStoreValueOperation-a20514.swift as well.


return src.materialize(SGF, loc);
}

assert(hasAbstraction);
Expand Down
5 changes: 4 additions & 1 deletion test/SILGen/Inputs/objc_bridging_sendable.h
Original file line number Diff line number Diff line change
Expand Up @@ -4,4 +4,7 @@
- (void) takeSendable: (id __attribute__((swift_attr("@Sendable")))) x;
@property(readonly) id __attribute__((swift_attr("@Sendable"))) x;
- (nullable __attribute__((swift_attr("@Sendable"))) id)test:(NSError *_Nullable __autoreleasing * _Nullable)error;
@end
- (nullable __attribute__((swift_attr("@Sendable"))) id)sendableNullableNSObject;
- (nullable id)regularNullableNSObject;
- (nonnull id)regularNSObject;
@end
65 changes: 65 additions & 0 deletions test/SILGen/objc_bridging_sendable.swift
Original file line number Diff line number Diff line change
Expand Up @@ -34,3 +34,68 @@ func test_use_of_buffer_init() throws {
try $0.test()
}
}

// CHECK-LABEL: sil {{.*}}[ossa] @$s22objc_bridging_sendable5cast1yyXlSgSo6NSBlahCF : $@convention(thin) (@guaranteed NSBlah) -> @owned Optional<AnyObject> {
// CHECK: bb0(%0 : @guaranteed $NSBlah):
// CHECK: [[TEST_REF:%.*]] = objc_method %0 : $NSBlah, #NSBlah.sendableNullableNSObject!foreign : (NSBlah) -> () -> (any Sendable)?, $@convention(objc_method) (NSBlah) -> @autoreleased Optional<AnyObject>
// CHECK: [[RESULT:%.*]] = apply [[TEST_REF]](%0) : $@convention(objc_method) (NSBlah) -> @autoreleased Optional<AnyObject>
// CHECK: [[OPTIONAL_SENDABLE:%.*]] = alloc_stack $Optional<any Sendable>
// CHECK: switch_enum [[RESULT]] : $Optional<AnyObject>, case #Optional.some!enumelt: bb1, case #Optional.none!enumelt: bb2
//
// AnyObject -> Optional<AnyObject> -> Any -> any Sendable -> Optional<any Sendable>
//
// CHECK: bb1([[SUCCESS:%.*]] : @owned $AnyObject):
// CHECK: [[OPT_RESULT_VALUE:%.*]] = unchecked_ref_cast [[SUCCESS]] : $AnyObject to $Optional<AnyObject>
// CHECK: [[BRIDGE_INTRINSIC_REF:%.*]] = function_ref @$ss018_bridgeAnyObjectToB0yypyXlSgF : $@convention(thin) (@guaranteed Optional<AnyObject>) -> @out Any
// CHECK: [[INTRINSIC_RET:%.*]] = alloc_stack $Any
// CHECK: apply [[BRIDGE_INTRINSIC_REF]]([[INTRINSIC_RET]], [[OPT_RESULT_VALUE]]) : $@convention(thin) (@guaranteed Optional<AnyObject>) -> @out Any
// CHECK: [[ANY_EXT_ADDR:%.*]] = open_existential_addr immutable_access [[INTRINSIC_RET]] : $*Any to $*@opened({{.*}}, Any) Self
// CHECK: [[ANY_SENDABLE:%.*]] = alloc_stack $any Sendable
// CHECK: [[ANY_SENDABLE_EXT_ADDR:%.*]] = init_existential_addr [[ANY_SENDABLE]] : $*any Sendable, $@opened({{.*}}, Any) Self
// CHECK: copy_addr [[ANY_EXT_ADDR]] to [init] [[ANY_SENDABLE_EXT_ADDR]]
// CHECK: [[OPTIONAL_SENDABLE_DATA_ADDR:%.*]] = init_enum_data_addr [[OPTIONAL_SENDABLE]] : $*Optional<any Sendable>, #Optional.some!enumelt
// CHECK: copy_addr [take] [[ANY_SENDABLE]] to [init] [[OPTIONAL_SENDABLE_DATA_ADDR]]
// CHECK: inject_enum_addr [[OPTIONAL_SENDABLE]] : $*Optional<any Sendable>, #Optional.some!enumelt
// CHECK: br bb3
//
// The main thing we previously had trouble with is:
// Optional<any Sendable> -> Optional<Any> -> AnyObject
//
// CHECK: bb3:
// CHECK-NEXT: [[ABI_COMPATABLE_IMPLICIT_CAST_RESULT:%.*]] = unchecked_addr_cast [[OPTIONAL_SENDABLE]] : $*Optional<any Sendable> to $*Optional<Any>
// CHECK-NEXT: [[EXPLICIT_CAST_RESULT:%.*]] = alloc_stack $AnyObject
// CHECK: checked_cast_addr_br take_always Optional<Any> in [[ABI_COMPATABLE_IMPLICIT_CAST_RESULT]] : $*Optional<Any> to AnyObject in [[EXPLICIT_CAST_RESULT]] : $*AnyObject
func cast1(_ b: NSBlah) -> AnyObject? {
return b.sendableNullableNSObject() as? AnyObject
}

// CHECK-LABEL: sil {{.*}}[ossa] @$s22objc_bridging_sendable5cast2yyXlSgSo6NSBlahCF
// CHECK: [[OPTIONAL_ANY:%.*]] = alloc_stack $Optional<Any>
// CHECK-NEXT: switch_enum {{.*}} : $Optional<AnyObject>, case #Optional.some!enumelt: bb1, case #Optional.none!enumelt: bb2
//
// I've abbreviated the CHECK lines for bb1, since `cast1` already covers most of this:
// AnyObject -> Optional<AnyObject> -> Any -> Optional<Any>
//
// CHECK: bb1({{.*}} : @owned $AnyObject):
// CHECK: function_ref @$ss018_bridgeAnyObjectToB0yypyXlSgF : $@convention(thin) (@guaranteed Optional<AnyObject>) -> @out Any
// CHECK: [[OPTIONAL_ANY_DATA_ADDR:%.*]] = init_enum_data_addr [[OPTIONAL_ANY]] : $*Optional<Any>, #Optional.some!enumelt
// CHECK: copy_addr [take] {{.*}} to [init] [[OPTIONAL_ANY_DATA_ADDR]] : $*Any
// CHECK: inject_enum_addr [[OPTIONAL_ANY]] : $*Optional<Any>
//
// Optional<Any> -> AnyObject
//
// CHECK: bb3: // Preds: bb2 bb1
// CHECK-NEXT: [[EXPLICIT_CAST_RESULT:%.*]] = alloc_stack $AnyObject
// CHECK-NEXT: checked_cast_addr_br take_always Optional<Any> in [[OPTIONAL_ANY]] : $*Optional<Any> to AnyObject in [[EXPLICIT_CAST_RESULT]] : $*AnyObject
func cast2(_ b: NSBlah) -> AnyObject? {
return b.regularNullableNSObject() as? AnyObject
}

// CHECK-LABEL: sil {{.*}}[ossa] @$s22objc_bridging_sendable5cast3yyXlSgSo6NSBlahCF
// CHECK: [[BRIDGE_FN:%.*]] = function_ref @$ss018_bridgeAnyObjectToB0yypyXlSgF : $@convention(thin) (@guaranteed Optional<AnyObject>) -> @out Any
// CHECK: apply [[BRIDGE_FN]]([[ANY:%.*]], {{.*}})
// CHECK: [[EXPLICIT_CAST_RESULT:%.*]] = alloc_stack $AnyObject
// CHECK: checked_cast_addr_br take_always Any in [[ANY]] : $*Any to AnyObject in [[EXPLICIT_CAST_RESULT]] : $*AnyObject
func cast3(_ b: NSBlah) -> AnyObject? {
return b.regularNSObject() as? AnyObject
}