Skip to content

Commit b4d0826

Browse files
committed
[NFC] Fixup __shared propagation
Two minor fixes that affect propagation of the __shared bit * Type canonicalization now preserves __shared. This could manifest as invalid references to SILDeclRefs with the wrong signature. * Function type lowering dispatches __shared tuples that are not valid substitution targets through the @owned path in all cases.
1 parent 8a8f7c8 commit b4d0826

File tree

3 files changed

+17
-7
lines changed

3 files changed

+17
-7
lines changed

lib/AST/Type.cpp

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1123,8 +1123,12 @@ CanType TypeBase::getCanonicalType() {
11231123
auto &ctx = function->getInput()->getASTContext();
11241124
auto &mod = *ctx.TheBuiltinModule;
11251125
Type inputTy = function->getInput()->getCanonicalType(sig, mod);
1126-
if (!AnyFunctionType::isCanonicalFunctionInputType(inputTy))
1127-
inputTy = ParenType::get(ctx, inputTy->getInOutObjectType(), ParameterTypeFlags().withInOut(inputTy->is<InOutType>()));
1126+
if (!AnyFunctionType::isCanonicalFunctionInputType(inputTy)) {
1127+
auto flags = ParameterTypeFlags().withInOut(inputTy->is<InOutType>());
1128+
if (auto parenTy = dyn_cast<ParenType>(function->getInput().getPointer()))
1129+
flags = flags.withShared(parenTy->getParameterFlags().isShared());
1130+
inputTy = ParenType::get(ctx, inputTy->getInOutObjectType(), flags);
1131+
}
11281132
auto resultTy = function->getResult()->getCanonicalType(sig, mod);
11291133

11301134
Result = GenericFunctionType::get(sig, inputTy, resultTy,
@@ -1142,7 +1146,10 @@ CanType TypeBase::getCanonicalType() {
11421146
FunctionType *FT = cast<FunctionType>(this);
11431147
Type In = FT->getInput()->getCanonicalType();
11441148
if (!AnyFunctionType::isCanonicalFunctionInputType(In)) {
1145-
In = ParenType::get(In->getASTContext(), In->getInOutObjectType(), ParameterTypeFlags().withInOut(In->is<InOutType>()));
1149+
auto flags = ParameterTypeFlags().withInOut(In->is<InOutType>());
1150+
if (auto parenTy = dyn_cast<ParenType>(FT->getInput().getPointer()))
1151+
flags = flags.withShared(parenTy->getParameterFlags().isShared());
1152+
In = ParenType::get(In->getASTContext(), In->getInOutObjectType(), flags);
11461153
assert(AnyFunctionType::isCanonicalFunctionInputType(In));
11471154
}
11481155
Type Out = FT->getResult()->getCanonicalType();

lib/SIL/SILFunctionType.cpp

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -486,6 +486,11 @@ enum class ConventionsKind : uint8_t {
486486
ParameterConvention convention;
487487
if (origType.getAs<InOutType>()) {
488488
convention = ParameterConvention::Indirect_Inout;
489+
} else if (isa<TupleType>(substType) && !origType.isTypeParameter()) {
490+
// Do not lower tuples @guaranteed. This can create conflicts with
491+
// substitutions for witness thunks e.g. we take $*(T, T)
492+
// @in_guaranteed and try to substitute it for $*T.
493+
return visit(origType, substType);
489494
} else if (isFormallyPassedIndirectly(origType, substType, substTL)) {
490495
if (rep == SILFunctionTypeRepresentation::WitnessMethod)
491496
convention = ParameterConvention::Indirect_In_Guaranteed;
@@ -586,7 +591,7 @@ enum class ConventionsKind : uint8_t {
586591
// materializable -- if it doesn't contain an l-value type -- then it's
587592
// a valid target for substitution and we should not expand it.
588593
if (!tty || (eltPattern.isTypeParameter() && !tty->hasInOutElement())) {
589-
if (params[i].getParameterFlags().isShared()) {
594+
if (params[i].isShared()) {
590595
visitSharedType(eltPattern, ty, extInfo.getSILRepresentation());
591596
} else {
592597
visit(eltPattern, ty);

test/SILGen/shared.swift

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -134,7 +134,5 @@ func owned_to_shared_conversion(_ f : (Int, ValueAggregate, RefAggregate) -> Voi
134134
return owned_to_shared_conversion { (trivial : __shared Int, val : __shared ValueAggregate, ref : __shared RefAggregate) in }
135135
}
136136

137-
// TODO: Investigate a different lowering scheme than @owned. Perhaps '@guaranteed $@callee_guaranteed'?
138-
139-
// CHECK-LABEL: sil hidden @_T06shared0A17_closure_loweringyySi_AA14ValueAggregateVAA03RefE0CtcF : $@convention(thin) (@owned @callee_owned (Int, @owned ValueAggregate, @owned RefAggregate) -> ()) -> ()
137+
// CHECK-LABEL: sil hidden @_T06shared0A17_closure_loweringyySi_AA14ValueAggregateVAA03RefE0CtcF : $@convention(thin) (@guaranteed @callee_owned (Int, @owned ValueAggregate, @owned RefAggregate) -> ()) -> ()
140138
func shared_closure_lowering(_ f : __shared (Int, ValueAggregate, RefAggregate) -> Void) {}

0 commit comments

Comments
 (0)