Skip to content

[rbi] Fix demangling of sending results. #78612

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.

Sign up for GitHub

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
Jan 15, 2025
Merged

[rbi] Fix demangling of sending results. #78612

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
bd37998
Make sure that SILResultInfo::{getWithInterfaceType,getWithConvention…
gottesmm Jan 13, 2025
cb2d756
[rbi] Fix demangling of sending results.
gottesmm Jan 13, 2025
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: 4 additions & 3 deletions docs/ABI/Mangling.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -837,7 +837,7 @@ mangled in to disambiguate.
impl-function-type ::= type* 'I' FUNC-ATTRIBUTES '_'
impl-function-type ::= type* generic-signature 'I' FUNC-ATTRIBUTES '_'

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?)?
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?)?

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

#if SWIFT_RUNTIME_VERSION >= 5.5
SENDABLE ::= 'h' // @Sendable
ASYNC ::= 'H' // @async
SENDABLE ::= 'h' // @Sendable
ASYNC ::= 'H' // @async
SENDING-RESULT ::= 'T' // sending result
#endif

PARAM-CONVENTION ::= 'i' // indirect in
Expand Down
4 changes: 2 additions & 2 deletions include/swift/AST/Types.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4842,12 +4842,12 @@ class SILResultInfo {
SILType getSILStorageInterfaceType() const;
/// Return a version of this result info with the type replaced.
SILResultInfo getWithInterfaceType(CanType type) const {
return SILResultInfo(type, getConvention());
return SILResultInfo(type, getConvention(), getOptions());
}

/// Return a version of this result info with the convention replaced.
SILResultInfo getWithConvention(ResultConvention c) const {
return SILResultInfo(getInterfaceType(), c);
return SILResultInfo(getInterfaceType(), c, getOptions());
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I noticed this when I was preparing the test cases for this PR. For some reason we were generating the thunks I was not expecting (since we were losing the sending result bit) and then due to the adjustFunctionType code generation, we were recovering. I tracked down that this was occurring due to SILGenPoly using this API and found this thinko. It didn't change any actual test output though, so I think we have been getting lucky.

}

// Does this result convention require indirect storage? This reflects a
Expand Down
7 changes: 7 additions & 0 deletions include/swift/Demangling/TypeDecoder.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1093,7 +1093,14 @@ class TypeDecoder {
flags = flags.withSendable();
} else if (child->getText() == "@async") {
flags = flags.withAsync();
} else if (child->getText() == "sending-result") {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

this is part of the code that I added that handles the legacy demangler code.

flags = flags.withSendingResult();
}
} else if (child->getKind() == NodeKind::ImplSendingResult) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This block is needed of course since we were not handling ImplSendingResult at all. The reason why it being not there was not an issue is b/c we also place a bit on the individual result info... so we got the appropriate bits from there.

// NOTE: This flag needs to be set both at the function level and on
// each of the parameters. The flag on the function just means that
// all parameters are sending (which is always true today).
flags = flags.withSendingResult();
} else if (child->getKind() == NodeKind::ImplCoroutineKind) {
if (!child->hasText())
return MAKE_NODE_TYPE_ERROR0(child, "expected text");
Expand Down
4 changes: 4 additions & 0 deletions lib/AST/ASTDemangler.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -677,6 +677,10 @@ Type ASTBuilder::createImplFunctionType(
auto type = result.getType()->getCanonicalType();
auto conv = getResultConvention(result.getConvention());
auto options = *getResultOptions(result.getOptions());
// We currently set sending result at the function level, but we set sending
// result on each result.
if (flags.hasSendingResult())
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This ensures that we maintain the invariant that I mentioned above that if the flags have a sending result, we also set the options bit on each SILResultInfo to mark them as sending.

options |= SILResultInfo::IsSending;
funcResults.emplace_back(type, conv, options);
}

Expand Down
17 changes: 13 additions & 4 deletions lib/AST/ASTMangler.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2301,6 +2301,19 @@ void ASTMangler::appendImplFunctionType(SILFunctionType *fn,
OpArgs.push_back('H');
}

// Mangle if we have a sending result and we are in a recursive position.
//
// DISCUSSION: We only want sending results to change ABI if it is using in a
// function value passed to a parameter or generic position... but not if it
// is just added to a return type.
//
// E.x.:
//
// func foo() -> sending X // No mangling
// func bar(_ x: () -> sending X) {} // Add to mangling for x
Copy link
Contributor

Choose a reason for hiding this comment

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

This comment implies that impl-function-type mangling is ABI, and it should probably be removed

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes. Sorry! I forgot to remove it.

if (isInRecursion && fn->hasSendingResult())
OpArgs.push_back('T');

GenericSignature sig = fn->getSubstGenericSignature();

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

// Mangle if we have a sending result.
if (isInRecursion && fn->hasSendingResult())
OpArgs.push_back('T');

// Mangle the results.
for (auto result : fn->getResults()) {
OpArgs.push_back(getResultConvention(result.getConvention()));
Expand Down
10 changes: 5 additions & 5 deletions lib/Demangling/Demangler.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2418,6 +2418,10 @@ NodePointer Demangler::demangleImplFunctionType() {
*this);
}

if (nextIf('T')) {
type->addChild(createNode(Node::Kind::ImplSendingResult), *this);
}

addChild(type, GenSig);

int NumTypesToAdd = 0;
Expand All @@ -2431,10 +2435,6 @@ NodePointer Demangler::demangleImplFunctionType() {
++NumTypesToAdd;
}

if (nextIf('T')) {
type->addChild(createNode(Node::Kind::ImplSendingResult), *this);
}

while (NodePointer Result = demangleImplResultConvention(
Node::Kind::ImplResult)) {
type = addChild(type, Result);
Expand Down Expand Up @@ -2467,7 +2467,7 @@ NodePointer Demangler::demangleImplFunctionType() {
return nullptr;
type->getChild(type->getNumChildren() - Idx - 1)->addChild(ConvTy, *this);
}

return createType(type);
}

Expand Down
3 changes: 3 additions & 0 deletions lib/Demangling/OldDemangler.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2252,6 +2252,9 @@ class OldDemangler {
if (Mangled.nextIf('H'))
addImplFunctionAttribute(type, "@async");

if (Mangled.nextIf('T'))
addImplFunctionAttribute(type, "sending-result");

// Enter a new generic context if this type is generic.
// FIXME: replace with std::optional, when we have it.
bool isPseudogeneric = false;
Expand Down
7 changes: 4 additions & 3 deletions lib/Demangling/Remangler.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2120,9 +2120,10 @@ ManglingError Remangler::mangleImplFunctionType(Node *node, unsigned depth) {
}
case Node::Kind::ImplFunctionAttribute: {
char FuncAttr = llvm::StringSwitch<char>(Child->getText())
.Case("@Sendable", 'h')
.Case("@async", 'H')
.Default(0);
.Case("@Sendable", 'h')
.Case("@async", 'H')
.Case("sending-result", 'T')
.Default(0);
if (!FuncAttr) {
return MANGLING_ERROR(ManglingError::InvalidImplFunctionAttribute,
Child);
Expand Down
22 changes: 21 additions & 1 deletion test/Concurrency/sending_mangling.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
// RUN: %target-swift-frontend %s -emit-silgen -swift-version 6 | swift-demangle | %FileCheck -check-prefix=CHECK %s
// RUN: %empty-directory(%t)
// RUN: %target-swift-frontend %s -emit-silgen -swift-version 6 | swift-demangle | %FileCheck %s
// RUN: %target-swift-frontend %s -emit-silgen -swift-version 6 | %FileCheck -check-prefix=SIL %s

// REQUIRES: concurrency
// REQUIRES: asserts
Expand Down Expand Up @@ -110,3 +112,21 @@ struct ConstructorSharedTest {
// CHECK: sil hidden [ossa] @sending_mangling.ConstructorSharedTest.functionSuppressed(sending_mangling.NonSendableKlass) -> () : $@convention(method) (@sil_sending @guaranteed NonSendableKlass, ConstructorSharedTest) -> () {
func functionSuppressed(_ x: __shared sending NonSendableKlass) {}
}

// Make sure that we produce the appropriate reabstraction thunk.
func reabstractionThunkTest_takeSendingReturnSending<T>(
_ x: sending T) -> sending T { fatalError() }
func reabstractionThunkTest_reabstractionThunkGenerator<T>(
_ x: sending T,
_ f: (sending T) -> T) {}

// 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 {
// 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 {

// 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 {
// 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 {
func reabstractionThunkTest() {
reabstractionThunkTest_reabstractionThunkGenerator(
NonSendableKlass(),
reabstractionThunkTest_takeSendingReturnSending)
}
2 changes: 2 additions & 0 deletions test/Demangle/Inputs/manglings.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -485,3 +485,5 @@ $s4main4SlabVy$1_SiG ---> main.Slab<2, Swift.Int>
$s$n3_SSBV ---> Builtin.FixedArray<-4, Swift.String>
$s3red7MyActorC3runyxxyYaKACYcYTXEYaKlFZ ---> static red.MyActor.run<A>(@red.MyActor () async throws -> sending A) async throws -> A
$s3red7MyActorC3runyxxyYaKYAYTXEYaKlFZ ---> static red.MyActor.run<A>(@isolated(any) () async throws -> sending A) async throws -> A
$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)
$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)