Skip to content

[CodeComplete] Mark results as not recommended if global actor context doesn't match #37711

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 1 commit into from
Jun 23, 2021
Merged

[CodeComplete] Mark results as not recommended if global actor context doesn't match #37711

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
9 changes: 7 additions & 2 deletions lib/IDE/CodeCompletion.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2649,9 +2649,14 @@ class CompletionLookup final : public swift::VisibleDeclConsumer {
break;
}
case ActorIsolation::GlobalActor:
case ActorIsolation::GlobalActorUnsafe:
// TODO: Implement.
case ActorIsolation::GlobalActorUnsafe: {
auto contextIsolation = getActorIsolationOfContext(
const_cast<DeclContext *>(CurrDeclContext));
if (contextIsolation != isolation) {
implicitlyAsync = true;
}
break;
}
case ActorIsolation::Unspecified:
case ActorIsolation::Independent:
return;
Expand Down
7 changes: 7 additions & 0 deletions lib/Sema/TypeCheckConcurrency.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2492,6 +2492,7 @@ namespace {
return getExplicitGlobalActor(closure);
}

public:
/// Determine the isolation of a particular closure.
///
/// This function assumes that enclosing closures have already had their
Expand Down Expand Up @@ -2715,6 +2716,12 @@ void swift::checkPropertyWrapperActorIsolation(
expr->walk(checker);
}

ClosureActorIsolation
swift::determineClosureActorIsolation(AbstractClosureExpr *closure) {
ActorIsolationChecker checker(closure->getParent());
return checker.determineClosureIsolation(closure);
}

/// Determine actor isolation solely from attributes.
///
/// \returns the actor isolation determined from attributes alone (with no
Expand Down
11 changes: 11 additions & 0 deletions lib/Sema/TypeCheckConcurrency.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ class ActorIsolation;
class AnyFunctionType;
class ASTContext;
class ClassDecl;
class ClosureActorIsolation;
class ClosureExpr;
class ConcreteDeclRef;
class CustomAttr;
Expand Down Expand Up @@ -59,6 +60,16 @@ void checkEnumElementActorIsolation(EnumElementDecl *element, Expr *expr);
void checkPropertyWrapperActorIsolation(
PatternBindingDecl *binding, Expr *expr);

/// Determine the isolation of a particular closure.
///
/// This forwards to \c ActorIsolationChecker::determineClosureActorIsolation
/// and thus assumes that enclosing closures have already had their isolation
/// checked.
///
/// This does not set the closure's actor isolation
ClosureActorIsolation
determineClosureActorIsolation(AbstractClosureExpr *closure);

/// Describes the kind of operation that introduced the concurrent refernece.
enum class ConcurrentReferenceKind {
/// A synchronous operation that was "promoted" to an asynchronous call
Expand Down
13 changes: 13 additions & 0 deletions lib/Sema/TypeCheckStmt.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1949,6 +1949,19 @@ bool TypeCheckASTNodeAtLocRequest::evaluate(Evaluator &evaluator,
if (auto CE = dyn_cast<ClosureExpr>(DC)) {
if (CE->getBodyState() == ClosureExpr::BodyState::Parsed) {
swift::typeCheckASTNodeAtLoc(CE->getParent(), CE->getLoc());
// We need the actor isolation of the closure to be set so that we can
// annotate results that are on the same global actor.
// Since we are evaluating TypeCheckASTNodeAtLocRequest for every closure
// from outermost to innermost, we don't want to call checkActorIsolation,
// because that would cause actor isolation to be checked multiple times
// for nested closures. Instead, call determineClosureActorIsolation
// directly and set the closure's actor isolation manually. We can
// guarantee of that the actor isolation of enclosing closures have their
// isolation checked before nested ones are being checked by the way
// TypeCheckASTNodeAtLocRequest is called multiple times, as described
// above.
auto ActorIsolation = determineClosureActorIsolation(CE);
CE->setActorIsolation(ActorIsolation);
Copy link
Member

Choose a reason for hiding this comment

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

Hmm, it feels a bit dicey setting the actor isolation for a closure in more than one place. Should we request'ify determineClosureActorIsolation?

Copy link
Member Author

Choose a reason for hiding this comment

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

I agree that it’s not really pretty. But the problem is that the ActorIsolationChecker currently implicitly assumes that it’s walking the tree top to bottom.

IMHO the proper solution would be to completely requesting ActorIsolationChecker so that it works bottom to top, requesting the actor isolation of an outer type as needed. But I felt that was too big of a change for Swift 5.5 and that’s why I stuck with the current, more local, approach.

if (CE->getBodyState() != ClosureExpr::BodyState::ReadyForTypeChecking)
return false;
}
Expand Down
176 changes: 176 additions & 0 deletions test/IDE/complete_global_actorisolation.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,176 @@
// REQUIRES: concurrency
// RUN: %target-swift-ide-test -batch-code-completion -source-filename %s -filecheck %raw-FileCheck -completion-output-dir %t -enable-experimental-concurrency

class MyNonSendable {}
struct MySendable {}

@globalActor
actor MyGlobalActor {
static var shared = MyGlobalActor()
}

@globalActor
actor MyOtherGlobalActor {
static var shared = MyOtherGlobalActor()
}

@MyGlobalActor func globalFuncOnGlobalActor() {}

func takeClosure<T>(_: () async -> T) {}

var otherInstanceOfMyClass = MyClass()

class MyClass {
@MyGlobalActor func funcOnGlobalActor() -> Int { return 0 }
@MyOtherGlobalActor func funcOnOtherGlobalActor() -> Int { return 0 }
func funcSync() -> Int { return 0 }

@MyGlobalActor func nonSenableFuncOnGlobalActor(arg: MyNonSendable) -> Int { return 0 }
@MyOtherGlobalActor func nonSenableFuncOnOtherGlobalActor(arg: MyNonSendable) -> Int { return 0 }

@MyGlobalActor var varOnGlobalActor: Int = 0
@MyOtherGlobalActor var varOnOtherGlobalActor: Int = 0
var varSync: Int = 0

@MyGlobalActor subscript(onGlobalActor onGlobalActor: Int) -> Int { get { 1 } set { } }
@MyOtherGlobalActor subscript(onOtherGlobalActor onOtherGlobalActor: Int) -> Int { get { 1 } set { } }
subscript(sync sync: Int) -> Int { get { 1 } set { } }
}

extension MyClass {
@MyGlobalActor func testOnGlobalActor() {
let _ = #^IN_FUNC_ON_GLOBAL_ACTOR^#
// IN_FUNC_ON_GLOBAL_ACTOR: Begin completions
// IN_FUNC_ON_GLOBAL_ACTOR-DAG: Decl[InstanceMethod]/CurrNominal: funcOnGlobalActor()[#Int#]; name=funcOnGlobalActor()
// IN_FUNC_ON_GLOBAL_ACTOR-DAG: Decl[InstanceMethod]/CurrNominal/NotRecommended: funcOnOtherGlobalActor()[' async'][#Int#]; name=funcOnOtherGlobalActor() async
// IN_FUNC_ON_GLOBAL_ACTOR-DAG: Decl[InstanceMethod]/CurrNominal: funcSync()[#Int#]; name=funcSync()
// IN_FUNC_ON_GLOBAL_ACTOR-DAG: Decl[InstanceMethod]/CurrNominal: nonSenableFuncOnGlobalActor({#arg: MyNonSendable#})[#Int#]; name=nonSenableFuncOnGlobalActor(arg: MyNonSendable)
// IN_FUNC_ON_GLOBAL_ACTOR-DAG: Decl[InstanceMethod]/CurrNominal/NotRecommended: nonSenableFuncOnOtherGlobalActor({#arg: MyNonSendable#})[' async'][#Int#]; name=nonSenableFuncOnOtherGlobalActor(arg: MyNonSendable) async
// IN_FUNC_ON_GLOBAL_ACTOR-DAG: Decl[InstanceVar]/CurrNominal: varOnGlobalActor[#Int#]; name=varOnGlobalActor
// IN_FUNC_ON_GLOBAL_ACTOR-DAG: Decl[InstanceVar]/CurrNominal/NotRecommended: varOnOtherGlobalActor[#Int#][' async']; name=varOnOtherGlobalActor async
// IN_FUNC_ON_GLOBAL_ACTOR-DAG: Decl[InstanceVar]/CurrNominal: varSync[#Int#]; name=varSync
// IN_FUNC_ON_GLOBAL_ACTOR: End completions

let _ = self.#^IN_FUNC_ON_GLOBAL_ACTOR_SELF_DOT^#
// IN_FUNC_ON_GLOBAL_ACTOR_SELF_DOT: Begin completions
// IN_FUNC_ON_GLOBAL_ACTOR_SELF_DOT-DAG: Decl[InstanceMethod]/CurrNominal: funcOnGlobalActor()[#Int#]; name=funcOnGlobalActor()
// IN_FUNC_ON_GLOBAL_ACTOR_SELF_DOT-DAG: Decl[InstanceMethod]/CurrNominal/NotRecommended: funcOnOtherGlobalActor()[' async'][#Int#]; name=funcOnOtherGlobalActor() async
// IN_FUNC_ON_GLOBAL_ACTOR_SELF_DOT-DAG: Decl[InstanceMethod]/CurrNominal: funcSync()[#Int#]; name=funcSync()
// IN_FUNC_ON_GLOBAL_ACTOR_SELF_DOT-DAG: Decl[InstanceMethod]/CurrNominal: nonSenableFuncOnGlobalActor({#arg: MyNonSendable#})[#Int#]; name=nonSenableFuncOnGlobalActor(arg: MyNonSendable)
// IN_FUNC_ON_GLOBAL_ACTOR_SELF_DOT-DAG: Decl[InstanceMethod]/CurrNominal/NotRecommended: nonSenableFuncOnOtherGlobalActor({#arg: MyNonSendable#})[' async'][#Int#]; name=nonSenableFuncOnOtherGlobalActor(arg: MyNonSendable) async
// IN_FUNC_ON_GLOBAL_ACTOR_SELF_DOT-DAG: Decl[InstanceVar]/CurrNominal: varOnGlobalActor[#Int#]; name=varOnGlobalActor
// IN_FUNC_ON_GLOBAL_ACTOR_SELF_DOT-DAG: Decl[InstanceVar]/CurrNominal/NotRecommended: varOnOtherGlobalActor[#Int#][' async']; name=varOnOtherGlobalActor async
// IN_FUNC_ON_GLOBAL_ACTOR_SELF_DOT-DAG: Decl[InstanceVar]/CurrNominal: varSync[#Int#]; name=varSync
// IN_FUNC_ON_GLOBAL_ACTOR_SELF_DOT: End completions

let _ = self#^IN_FUNC_ON_GLOBAL_ACTOR_NODOT^#
// IN_FUNC_ON_GLOBAL_ACTOR_NODOT: Begin completions
// IN_FUNC_ON_GLOBAL_ACTOR_NODOT-DAG: Decl[InstanceMethod]/CurrNominal: .funcOnGlobalActor()[#Int#]; name=funcOnGlobalActor()
// IN_FUNC_ON_GLOBAL_ACTOR_NODOT-DAG: Decl[InstanceMethod]/CurrNominal/NotRecommended: .funcOnOtherGlobalActor()[' async'][#Int#]; name=funcOnOtherGlobalActor() async
// IN_FUNC_ON_GLOBAL_ACTOR_NODOT-DAG: Decl[InstanceMethod]/CurrNominal: .funcSync()[#Int#]; name=funcSync()
// IN_FUNC_ON_GLOBAL_ACTOR_NODOT-DAG: Decl[InstanceMethod]/CurrNominal: .nonSenableFuncOnGlobalActor({#arg: MyNonSendable#})[#Int#]; name=nonSenableFuncOnGlobalActor(arg: MyNonSendable)
// IN_FUNC_ON_GLOBAL_ACTOR_NODOT-DAG: Decl[InstanceMethod]/CurrNominal/NotRecommended: .nonSenableFuncOnOtherGlobalActor({#arg: MyNonSendable#})[' async'][#Int#]; name=nonSenableFuncOnOtherGlobalActor(arg: MyNonSendable) async
// IN_FUNC_ON_GLOBAL_ACTOR_NODOT-DAG: Decl[InstanceVar]/CurrNominal: .varOnGlobalActor[#Int#]; name=varOnGlobalActor
// IN_FUNC_ON_GLOBAL_ACTOR_NODOT-DAG: Decl[InstanceVar]/CurrNominal/NotRecommended: .varOnOtherGlobalActor[#Int#][' async']; name=varOnOtherGlobalActor async
// IN_FUNC_ON_GLOBAL_ACTOR_NODOT-DAG: Decl[InstanceVar]/CurrNominal: .varSync[#Int#]; name=varSync
// IN_FUNC_ON_GLOBAL_ACTOR_NODOT-DAG: Decl[Subscript]/CurrNominal: [{#onGlobalActor: Int#}][#Int#]; name=[onGlobalActor: Int]
// IN_FUNC_ON_GLOBAL_ACTOR_NODOT-DAG: Decl[Subscript]/CurrNominal/NotRecommended: [{#onOtherGlobalActor: Int#}][' async'][#Int#]; name=[onOtherGlobalActor: Int] async
// IN_FUNC_ON_GLOBAL_ACTOR_NODOT-DAG: Decl[Subscript]/CurrNominal: [{#sync: Int#}][#Int#]; name=[sync: Int]
// IN_FUNC_ON_GLOBAL_ACTOR_NODOT: End completions

let _ = otherInstanceOfMyClass.#^IN_FUNC_ON_GLOBAL_ACTOR_OTHER_DOT?check=IN_FUNC_ON_GLOBAL_ACTOR_SELF_DOT^#
let _ = otherInstanceOfMyClass#^IN_FUNC_ON_GLOBAL_ACTOR_OTHER_NODOT?check=IN_FUNC_ON_GLOBAL_ACTOR_NODOT^#
}

func testInSyncFunc() {
let _ = #^IN_SYNC_FUNC^#
// IN_SYNC_FUNC: Begin completions
// IN_SYNC_FUNC_DOT-DAG: Decl[InstanceMethod]/CurrNominal/NotRecommended: funcOnGlobalActor()[' async'][#Int#]; name=funcOnGlobalActor()
// IN_SYNC_FUNC_DOT-DAG: Decl[InstanceMethod]/CurrNominal/NotRecommended: funcOnOtherGlobalActor()[' async'][#Int#]; name=funcOnOtherGlobalActor() async
// IN_SYNC_FUNC_DOT-DAG: Decl[InstanceMethod]/CurrNominal: funcSync()[#Int#]; name=funcSync()
// IN_SYNC_FUNC_DOT-DAG: Decl[InstanceMethod]/CurrNominal/NotRecommended: nonSenableFuncOnGlobalActor({#arg: MyNonSendable#})[' async'][#Int#]; name=nonSenableFuncOnGlobalActor(arg: MyNonSendable)
// IN_SYNC_FUNC_DOT-DAG: Decl[InstanceMethod]/CurrNominal/NotRecommended: nonSenableFuncOnOtherGlobalActor({#arg: MyNonSendable#})[' async'][#Int#]; name=nonSenableFuncOnOtherGlobalActor(arg: MyNonSendable) async
// IN_SYNC_FUNC_DOT-DAG: Decl[InstanceVar]/CurrNominal/NotRecommended: varOnGlobalActor[#Int#][' async']; name=varOnGlobalActor
// IN_SYNC_FUNC_DOT-DAG: Decl[InstanceVar]/CurrNominal/NotRecommended: varOnOtherGlobalActor[#Int#][' async']; name=varOnOtherGlobalActor async
// IN_SYNC_FUNC_DOT-DAG: Decl[InstanceVar]/CurrNominal: varSync[#Int#]; name=varSync
// IN_SYNC_FUNC: End completions

let _ = self.#^IN_SYNC_FUNC_SELF_DOT^#
// IN_SYNC_FUNC_SELF_DOT: Begin completions
// IN_SYNC_FUNC_SELF_DOT-DAG: Decl[InstanceMethod]/CurrNominal/NotRecommended: funcOnGlobalActor()[' async'][#Int#]; name=funcOnGlobalActor()
// IN_SYNC_FUNC_SELF_DOT-DAG: Decl[InstanceMethod]/CurrNominal/NotRecommended: funcOnOtherGlobalActor()[' async'][#Int#]; name=funcOnOtherGlobalActor() async
// IN_SYNC_FUNC_SELF_DOT-DAG: Decl[InstanceMethod]/CurrNominal: funcSync()[#Int#]; name=funcSync()
// IN_SYNC_FUNC_SELF_DOT-DAG: Decl[InstanceMethod]/CurrNominal/NotRecommended: nonSenableFuncOnGlobalActor({#arg: MyNonSendable#})[' async'][#Int#]; name=nonSenableFuncOnGlobalActor(arg: MyNonSendable)
// IN_SYNC_FUNC_SELF_DOT-DAG: Decl[InstanceMethod]/CurrNominal/NotRecommended: nonSenableFuncOnOtherGlobalActor({#arg: MyNonSendable#})[' async'][#Int#]; name=nonSenableFuncOnOtherGlobalActor(arg: MyNonSendable) async
// IN_SYNC_FUNC_SELF_DOT-DAG: Decl[InstanceVar]/CurrNominal/NotRecommended: varOnGlobalActor[#Int#][' async']; name=varOnGlobalActor
// IN_SYNC_FUNC_SELF_DOT-DAG: Decl[InstanceVar]/CurrNominal/NotRecommended: varOnOtherGlobalActor[#Int#][' async']; name=varOnOtherGlobalActor async
// IN_SYNC_FUNC_SELF_DOT-DAG: Decl[InstanceVar]/CurrNominal: varSync[#Int#]; name=varSync
// IN_SYNC_FUNC_SELF_DOT: End completions

let _ = self#^IN_SYNC_FUNC_NODOT^#
// IN_SYNC_FUNC_NODOT: Begin completions
// IN_SYNC_FUNC_NODOT-DAG: Decl[InstanceMethod]/CurrNominal/NotRecommended: .funcOnGlobalActor()[' async'][#Int#]; name=funcOnGlobalActor()
// IN_SYNC_FUNC_NODOT-DAG: Decl[InstanceMethod]/CurrNominal/NotRecommended: .funcOnOtherGlobalActor()[' async'][#Int#]; name=funcOnOtherGlobalActor() async
// IN_SYNC_FUNC_NODOT-DAG: Decl[InstanceMethod]/CurrNominal: .funcSync()[#Int#]; name=funcSync()
// IN_SYNC_FUNC_NODOT-DAG: Decl[InstanceMethod]/CurrNominal/NotRecommended: .nonSenableFuncOnGlobalActor({#arg: MyNonSendable#})[' async'][#Int#]; name=nonSenableFuncOnGlobalActor(arg: MyNonSendable)
// IN_SYNC_FUNC_NODOT-DAG: Decl[InstanceMethod]/CurrNominal/NotRecommended: .nonSenableFuncOnOtherGlobalActor({#arg: MyNonSendable#})[' async'][#Int#]; name=nonSenableFuncOnOtherGlobalActor(arg: MyNonSendable) async
// IN_SYNC_FUNC_NODOT-DAG: Decl[InstanceVar]/CurrNominal/NotRecommended: .varOnGlobalActor[#Int#][' async']; name=varOnGlobalActor
// IN_SYNC_FUNC_NODOT-DAG: Decl[InstanceVar]/CurrNominal/NotRecommended: .varOnOtherGlobalActor[#Int#][' async']; name=varOnOtherGlobalActor async
// IN_SYNC_FUNC_NODOT-DAG: Decl[InstanceVar]/CurrNominal: .varSync[#Int#]; name=varSync
// IN_SYNC_FUNC_NODOT-DAG: Decl[Subscript]/CurrNominal/NotRecommended: [{#onGlobalActor: Int#}][' async'][#Int#]; name=[onGlobalActor: Int]
// IN_SYNC_FUNC_NODOT-DAG: Decl[Subscript]/CurrNominal/NotRecommended: [{#onOtherGlobalActor: Int#}][' async'][#Int#]; name=[onOtherGlobalActor: Int] async
// IN_SYNC_FUNC_NODOT-DAG: Decl[Subscript]/CurrNominal: [{#sync: Int#}][#Int#]; name=[sync: Int]
// IN_SYNC_FUNC_NODOT: End completions

let _ = otherInstanceOfMyClass.#^IN_SYNC_FUNC_OTHER_DOT?check=IN_SYNC_FUNC_SELF_DOT^#
let _ = otherInstanceOfMyClass#^IN_SYNC_FUNC_OTHER_NODOT?check=IN_SYNC_FUNC_NODOT^#
}

func testInGlobalActorClosure() {
_ = { @MyGlobalActor () -> Void in
let _ = otherInstanceOfMyClass.#^IN_CLOSURE_ON_GLOBAL_ACTOR_OTHER_DOT?check=IN_FUNC_ON_GLOBAL_ACTOR_SELF_DOT^#
}
}

func testInGlobalActorClosureWithoutExplicitAttribute() {
let callback: @MyGlobalActor () -> Void
callback = {
let _ = otherInstanceOfMyClass.#^IN_CLOSURE_ON_GLOBAL_ACTOR_WITHOUT_EXPLICIT_LABEL_OTHER_DOT?check=IN_FUNC_ON_GLOBAL_ACTOR_SELF_DOT^#
}
}

@MyGlobalActor func testInClosureInGlobalActorFunc() {
_ = { () -> Void in
let _ = otherInstanceOfMyClass.#^IN_CLOSURE_IN_FUNC_ON_GLOBAL_ACTOR_OTHER_DOT?check=IN_FUNC_ON_GLOBAL_ACTOR_SELF_DOT^#
}
}

func testInClosureNestedInClosureOnGlobalActorFunc() {
_ = { @MyGlobalActor () -> Void in
_ = { () -> Void in
let _ = otherInstanceOfMyClass.#^IN_CLOSURE_NESTED_IN_CLOSURE_ON_GLOBAL_ACTOR_OTHER_DOT?check=IN_FUNC_ON_GLOBAL_ACTOR_SELF_DOT^#
}
}
}

func testInLocalFunc() {
@MyGlobalActor func localFunc() {
let _ = otherInstanceOfMyClass.#^IN_LOCAL_FUNC_ON_GLOBAL_ACTOR_OTHER_DOT?check=IN_FUNC_ON_GLOBAL_ACTOR_SELF_DOT^#
}
}

@MyGlobalActor func testInNestedSingleExpressionClosure() {
takeClosure {
takeClosure {
otherInstanceOfMyClass.#^IN_NESTED_SINGLE_EXPRESSION_CLOSURE_ON_GLBOAL_ACTOR_OTHER_DOT?check=IN_FUNC_ON_GLOBAL_ACTOR_SELF_DOT^#
}
}
}
}

Copy link
Member

Choose a reason for hiding this comment

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

Could you add a nested single expression closure test? e.g.

@MyGlobalActor func globalFuncOnMyGlobalActor() {}
@MyOtherGlobalActor func globalFuncOnMyOtherGlobalActor() {}

func foo<T>(_: () async -> T) {}

@MyGlobalActor func test() {
    foo {
        foo {
            <HERE>
        }
    }
}

actor ActorTests {
func testInActor() {
let _ = otherInstanceOfMyClass.#^IN_ACTOR_OTHER_DOT?check=IN_SYNC_FUNC_SELF_DOT^#
let _ = otherInstanceOfMyClass#^IN_ACTOR_OTHER_NODOT?check=IN_SYNC_FUNC_NODOT^#
}
}