Skip to content

Commit cb2d756

Browse files
committed
[rbi] Fix demangling of sending results.
The issue here is that the demangler (since we have a postfix mangling) parses parameters/results/etc and then uses earlier postfix type arguments to attach the relevant types to the parameters/results/etc. Since the flag for a sending result was placed in between the parameters and results, we get an off by one error. Rather than fix that specific issue by introducing an offset for the off by one error, I used the fact that the impl-function part of the mangling is not ABI and can be modified to move the bit used to signify a sending result to before the parameters so the whole problem is avoided. I also while I was doing this looked through the sending result mangling for any further issues and fixed them as I found them. rdar://141962865
1 parent bd37998 commit cb2d756

File tree

9 files changed

+63
-16
lines changed

9 files changed

+63
-16
lines changed

docs/ABI/Mangling.rst

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -837,7 +837,7 @@ mangled in to disambiguate.
837837
impl-function-type ::= type* 'I' FUNC-ATTRIBUTES '_'
838838
impl-function-type ::= type* generic-signature 'I' FUNC-ATTRIBUTES '_'
839839

840-
FUNC-ATTRIBUTES ::= PATTERN-SUBS? INVOCATION-SUBS? PSEUDO-GENERIC? CALLEE-ESCAPE? ISOLATION? DIFFERENTIABILITY-KIND? CALLEE-CONVENTION FUNC-REPRESENTATION? COROUTINE-KIND? SENDABLE? ASYNC? (PARAM-CONVENTION PARAM-DIFFERENTIABILITY?)* RESULT-CONVENTION* ('Y' PARAM-CONVENTION)* ('z' RESULT-CONVENTION RESULT-DIFFERENTIABILITY?)?
840+
FUNC-ATTRIBUTES ::= PATTERN-SUBS? INVOCATION-SUBS? PSEUDO-GENERIC? CALLEE-ESCAPE? ISOLATION? DIFFERENTIABILITY-KIND? CALLEE-CONVENTION FUNC-REPRESENTATION? COROUTINE-KIND? SENDABLE? ASYNC? SENDING-RESULT? (PARAM-CONVENTION PARAM-DIFFERENTIABILITY?)* RESULT-CONVENTION* ('Y' PARAM-CONVENTION)* ('z' RESULT-CONVENTION RESULT-DIFFERENTIABILITY?)?
841841

842842
PATTERN-SUBS ::= 's' // has pattern substitutions
843843
INVOCATION-SUB ::= 'I' // has invocation substitutions
@@ -871,8 +871,9 @@ mangled in to disambiguate.
871871
COROUTINE-KIND ::= 'G' // yield-many coroutine
872872

873873
#if SWIFT_RUNTIME_VERSION >= 5.5
874-
SENDABLE ::= 'h' // @Sendable
875-
ASYNC ::= 'H' // @async
874+
SENDABLE ::= 'h' // @Sendable
875+
ASYNC ::= 'H' // @async
876+
SENDING-RESULT ::= 'T' // sending result
876877
#endif
877878

878879
PARAM-CONVENTION ::= 'i' // indirect in

include/swift/Demangling/TypeDecoder.h

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1093,7 +1093,14 @@ class TypeDecoder {
10931093
flags = flags.withSendable();
10941094
} else if (child->getText() == "@async") {
10951095
flags = flags.withAsync();
1096+
} else if (child->getText() == "sending-result") {
1097+
flags = flags.withSendingResult();
10961098
}
1099+
} else if (child->getKind() == NodeKind::ImplSendingResult) {
1100+
// NOTE: This flag needs to be set both at the function level and on
1101+
// each of the parameters. The flag on the function just means that
1102+
// all parameters are sending (which is always true today).
1103+
flags = flags.withSendingResult();
10971104
} else if (child->getKind() == NodeKind::ImplCoroutineKind) {
10981105
if (!child->hasText())
10991106
return MAKE_NODE_TYPE_ERROR0(child, "expected text");

lib/AST/ASTDemangler.cpp

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -677,6 +677,10 @@ Type ASTBuilder::createImplFunctionType(
677677
auto type = result.getType()->getCanonicalType();
678678
auto conv = getResultConvention(result.getConvention());
679679
auto options = *getResultOptions(result.getOptions());
680+
// We currently set sending result at the function level, but we set sending
681+
// result on each result.
682+
if (flags.hasSendingResult())
683+
options |= SILResultInfo::IsSending;
680684
funcResults.emplace_back(type, conv, options);
681685
}
682686

lib/AST/ASTMangler.cpp

Lines changed: 13 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2301,6 +2301,19 @@ void ASTMangler::appendImplFunctionType(SILFunctionType *fn,
23012301
OpArgs.push_back('H');
23022302
}
23032303

2304+
// Mangle if we have a sending result and we are in a recursive position.
2305+
//
2306+
// DISCUSSION: We only want sending results to change ABI if it is using in a
2307+
// function value passed to a parameter or generic position... but not if it
2308+
// is just added to a return type.
2309+
//
2310+
// E.x.:
2311+
//
2312+
// func foo() -> sending X // No mangling
2313+
// func bar(_ x: () -> sending X) {} // Add to mangling for x
2314+
if (isInRecursion && fn->hasSendingResult())
2315+
OpArgs.push_back('T');
2316+
23042317
GenericSignature sig = fn->getSubstGenericSignature();
23052318

23062319
// Mangle the parameters.
@@ -2313,10 +2326,6 @@ void ASTMangler::appendImplFunctionType(SILFunctionType *fn,
23132326
appendType(param.getInterfaceType(), sig, forDecl);
23142327
}
23152328

2316-
// Mangle if we have a sending result.
2317-
if (isInRecursion && fn->hasSendingResult())
2318-
OpArgs.push_back('T');
2319-
23202329
// Mangle the results.
23212330
for (auto result : fn->getResults()) {
23222331
OpArgs.push_back(getResultConvention(result.getConvention()));

lib/Demangling/Demangler.cpp

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2418,6 +2418,10 @@ NodePointer Demangler::demangleImplFunctionType() {
24182418
*this);
24192419
}
24202420

2421+
if (nextIf('T')) {
2422+
type->addChild(createNode(Node::Kind::ImplSendingResult), *this);
2423+
}
2424+
24212425
addChild(type, GenSig);
24222426

24232427
int NumTypesToAdd = 0;
@@ -2431,10 +2435,6 @@ NodePointer Demangler::demangleImplFunctionType() {
24312435
++NumTypesToAdd;
24322436
}
24332437

2434-
if (nextIf('T')) {
2435-
type->addChild(createNode(Node::Kind::ImplSendingResult), *this);
2436-
}
2437-
24382438
while (NodePointer Result = demangleImplResultConvention(
24392439
Node::Kind::ImplResult)) {
24402440
type = addChild(type, Result);
@@ -2467,7 +2467,7 @@ NodePointer Demangler::demangleImplFunctionType() {
24672467
return nullptr;
24682468
type->getChild(type->getNumChildren() - Idx - 1)->addChild(ConvTy, *this);
24692469
}
2470-
2470+
24712471
return createType(type);
24722472
}
24732473

lib/Demangling/OldDemangler.cpp

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2252,6 +2252,9 @@ class OldDemangler {
22522252
if (Mangled.nextIf('H'))
22532253
addImplFunctionAttribute(type, "@async");
22542254

2255+
if (Mangled.nextIf('T'))
2256+
addImplFunctionAttribute(type, "sending-result");
2257+
22552258
// Enter a new generic context if this type is generic.
22562259
// FIXME: replace with std::optional, when we have it.
22572260
bool isPseudogeneric = false;

lib/Demangling/Remangler.cpp

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2120,9 +2120,10 @@ ManglingError Remangler::mangleImplFunctionType(Node *node, unsigned depth) {
21202120
}
21212121
case Node::Kind::ImplFunctionAttribute: {
21222122
char FuncAttr = llvm::StringSwitch<char>(Child->getText())
2123-
.Case("@Sendable", 'h')
2124-
.Case("@async", 'H')
2125-
.Default(0);
2123+
.Case("@Sendable", 'h')
2124+
.Case("@async", 'H')
2125+
.Case("sending-result", 'T')
2126+
.Default(0);
21262127
if (!FuncAttr) {
21272128
return MANGLING_ERROR(ManglingError::InvalidImplFunctionAttribute,
21282129
Child);

test/Concurrency/sending_mangling.swift

Lines changed: 21 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,6 @@
1-
// RUN: %target-swift-frontend %s -emit-silgen -swift-version 6 | swift-demangle | %FileCheck -check-prefix=CHECK %s
1+
// RUN: %empty-directory(%t)
2+
// RUN: %target-swift-frontend %s -emit-silgen -swift-version 6 | swift-demangle | %FileCheck %s
3+
// RUN: %target-swift-frontend %s -emit-silgen -swift-version 6 | %FileCheck -check-prefix=SIL %s
24

35
// REQUIRES: concurrency
46
// REQUIRES: asserts
@@ -110,3 +112,21 @@ struct ConstructorSharedTest {
110112
// CHECK: sil hidden [ossa] @sending_mangling.ConstructorSharedTest.functionSuppressed(sending_mangling.NonSendableKlass) -> () : $@convention(method) (@sil_sending @guaranteed NonSendableKlass, ConstructorSharedTest) -> () {
111113
func functionSuppressed(_ x: __shared sending NonSendableKlass) {}
112114
}
115+
116+
// Make sure that we produce the appropriate reabstraction thunk.
117+
func reabstractionThunkTest_takeSendingReturnSending<T>(
118+
_ x: sending T) -> sending T { fatalError() }
119+
func reabstractionThunkTest_reabstractionThunkGenerator<T>(
120+
_ x: sending T,
121+
_ f: (sending T) -> T) {}
122+
123+
// CHECK: sil shared [transparent] [serialized] [reabstraction_thunk] [ossa] @reabstraction thunk helper from @escaping @callee_guaranteed (@in sending sending_mangling.NonSendableKlass) -> sending (@out sending_mangling.NonSendableKlass) to @escaping @callee_guaranteed (@owned sending sending_mangling.NonSendableKlass) -> sending (@owned sending_mangling.NonSendableKlass) : $@convention(thin) (@sil_sending @owned NonSendableKlass, @guaranteed @callee_guaranteed (@sil_sending @in NonSendableKlass) -> @sil_sending @out NonSendableKlass) -> @sil_sending @owned NonSendableKlass {
124+
// SIL: sil shared [transparent] [serialized] [reabstraction_thunk] [ossa] @$s16sending_mangling16NonSendableKlassCACIegTiTr_A2CIegTxTo_TR : $@convention(thin) (@sil_sending @owned NonSendableKlass, @guaranteed @callee_guaranteed (@sil_sending @in NonSendableKlass) -> @sil_sending @out NonSendableKlass) -> @sil_sending @owned NonSendableKlass {
125+
126+
// CHECK: sil shared [transparent] [serialized] [reabstraction_thunk] [ossa] @reabstraction thunk helper from @callee_guaranteed (@owned sending sending_mangling.NonSendableKlass) -> (@owned sending_mangling.NonSendableKlass) to @escaping @callee_guaranteed (@in sending sending_mangling.NonSendableKlass) -> (@out sending_mangling.NonSendableKlass) : $@convention(thin) (@sil_sending @in NonSendableKlass, @guaranteed @noescape @callee_guaranteed (@sil_sending @owned NonSendableKlass) -> @owned NonSendableKlass) -> @out NonSendableKlass {
127+
// SIL: sil shared [transparent] [serialized] [reabstraction_thunk] [ossa] @$s16sending_mangling16NonSendableKlassCACIgxTo_A2CIegiTr_TR : $@convention(thin) (@sil_sending @in NonSendableKlass, @guaranteed @noescape @callee_guaranteed (@sil_sending @owned NonSendableKlass) -> @owned NonSendableKlass) -> @out NonSendableKlass {
128+
func reabstractionThunkTest() {
129+
reabstractionThunkTest_reabstractionThunkGenerator(
130+
NonSendableKlass(),
131+
reabstractionThunkTest_takeSendingReturnSending)
132+
}

test/Demangle/Inputs/manglings.txt

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -485,3 +485,5 @@ $s4main4SlabVy$1_SiG ---> main.Slab<2, Swift.Int>
485485
$s$n3_SSBV ---> Builtin.FixedArray<-4, Swift.String>
486486
$s3red7MyActorC3runyxxyYaKACYcYTXEYaKlFZ ---> static red.MyActor.run<A>(@red.MyActor () async throws -> sending A) async throws -> A
487487
$s3red7MyActorC3runyxxyYaKYAYTXEYaKlFZ ---> static red.MyActor.run<A>(@isolated(any) () async throws -> sending A) async throws -> A
488+
$s7ToolKit10TypedValueOACs5Error_pIgHTnTrzo_A2CsAD_pIegHiTrzr_TR ---> {T:} reabstraction thunk helper from @callee_guaranteed @async (@in_guaranteed sending ToolKit.TypedValue) -> sending (@out ToolKit.TypedValue, @error @owned Swift.Error) to @escaping @callee_guaranteed @async (@in sending ToolKit.TypedValue) -> (@out ToolKit.TypedValue, @error @out Swift.Error)
489+
$s16sending_mangling16NonSendableKlassCACIegTiTr_A2CIegTxTo_TR ---> {T:} reabstraction thunk helper from @escaping @callee_guaranteed (@in sending sending_mangling.NonSendableKlass) -> sending (@out sending_mangling.NonSendableKlass) to @escaping @callee_guaranteed (@owned sending sending_mangling.NonSendableKlass) -> sending (@owned sending_mangling.NonSendableKlass)

0 commit comments

Comments
 (0)