Skip to content

Add sending to subscript sending result #79560

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
Mar 28, 2025
Merged

Add sending to subscript sending result #79560

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
43e6030
Add sending to subscript getter sending result
stzn Feb 22, 2025
98209cd
Add a test case with a tuple result
stzn Mar 7, 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: 7 additions & 0 deletions lib/AST/ASTPrinter.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4450,6 +4450,13 @@ void PrintAST::visitSubscriptDecl(SubscriptDecl *decl) {
Printer.printDeclResultTypePre(decl, elementTy);
Printer.callPrintStructurePre(PrintStructureKind::FunctionReturnType);

if (!Options.SuppressSendingArgsAndResults) {
if (auto typeRepr = decl->getElementTypeRepr()) {
if (isa<SendingTypeRepr>(decl->getResultTypeRepr()))
Printer << "sending ";
}
}

PrintWithOpaqueResultTypeKeywordRAII x(Options);
printTypeLocForImplicitlyUnwrappedOptional(
elementTy, decl->isImplicitlyUnwrappedOptional());
Expand Down
3 changes: 3 additions & 0 deletions lib/AST/Decl.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10754,6 +10754,9 @@ AccessorDecl *AccessorDecl::createParsed(

newParams.push_back(param);
}

if (isa<SendingTypeRepr>(SD->getElementTypeRepr()))
accessor->setSendingResult();
}
accessor->setParameters(
ParameterList::create(ctx, paramsStart, newParams, paramsEnd));
Expand Down
4 changes: 4 additions & 0 deletions lib/Sema/TypeCheckDecl.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2638,6 +2638,10 @@ InterfaceTypeRequest::evaluate(Evaluator &eval, ValueDecl *D) const {
AnyFunctionType::ExtInfoBuilder infoBuilder;
maybeAddParameterIsolation(infoBuilder, argTy);

if (auto typeRepr = SD->getElementTypeRepr())
if (isa<SendingTypeRepr>(typeRepr))
infoBuilder = infoBuilder.withSendingResult();

Type funcTy;
// FIXME: Verify ExtInfo state is correct, not working by accident.
auto info = infoBuilder.build();
Expand Down
22 changes: 22 additions & 0 deletions test/Concurrency/sending_subscript.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
// RUN: %target-swift-frontend -swift-version 6 %s -emit-silgen | %FileCheck %s

// REQUIRES: concurrency

class NonSendableKlass {}

// CHECK-DAG: subscript(_: sending NonSendableKlass) -> sending NonSendableKlass { get }

// CHECK-DAG: sil hidden [ossa] @$s17sending_subscript1SVyAA16NonSendableKlassCAEncig : $@convention(method) (@sil_sending @owned NonSendableKlass, S) -> @sil_sending @owned NonSendableKlass {
struct S {
subscript(_: sending NonSendableKlass) -> sending NonSendableKlass { NonSendableKlass() }
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you add a test case with multiple results. IIRC there was some sort of code I put in that forced all result types to also need to have sending result as well. The idea is that with time, top level sending result means all results have sending results... but once we support sending on specific results, then we allow for functions not to set the top level sending result flag and vary that specific sending result flag on results. I at least want some more test cases with a tuple result. Let me see if I can find the code in question.

Copy link
Contributor

Choose a reason for hiding this comment

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

Actually, now that I think about it... that property is actually something at the SIL level. Not something at the AST level where we just IIRC represent results with a type instead of using ResultInfo sort of things. Can you just out of an abundance of caution add a subscript with a tuple result test case.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@gottesmm
Thank you for the comment.
I added a test case with a tuple result. 98209cd

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@gottesmm
Is anything else needed?

}

// CHECK-DAG: subscript(_: sending NonSendableKlass) -> sending (NonSendableKlass, NonSendableKlass) { get }

// CHECK-DAG: sil hidden [ossa] @$s17sending_subscript2S2VyAA16NonSendableKlassC_AEtAEncig : $@convention(method) (@sil_sending @owned NonSendableKlass, S2) -> (@sil_sending @owned NonSendableKlass, @sil_sending @owned NonSendableKlass) {
struct S2 {
subscript(_: sending NonSendableKlass) -> sending (NonSendableKlass, NonSendableKlass) {
(NonSendableKlass(), NonSendableKlass())
}
}