Skip to content
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

Jump to bottom

Fixes for stored property initializers #40652

Merged
merged 2 commits into from Jan 4, 2022
Merged

Fixes for stored property initializers #40652

merged 2 commits into from Jan 4, 2022

Conversation

kavon
Copy link
Contributor

@kavon kavon commented Dec 21, 2021 •
edited by AnthonyLatsis

There are two related fixes in this PR:

  • Stored properties that are members of a type previously had the isolation applied to the var carry-over to the initializing expression. This can create an impossible-to-satisfy isolation requirement on the non-delegating initializers of the type. In line with the solution for this in SE-327, this PR removes that carry-over of isolation, making them effectively a nonisolated context. While this will cause a source break, the isolation was not being respected anyway. The workaround for this is to just not use the short-hand and initialize those properties in the type's initializer. If your type has an impossible constraint, then that problem should become apparent while trying to implement this workaround. (rdar://84225474)
  • In some instances, static properties of a type marked with MainActor were not having their initializing expression run on the main thread. That has been resolved in this PR. (Resolves [SR-15227] MainActor static variable initialization not enforced on main thread. #57549, rdar://83411416)

@kavon kavon added the concurrency Feature: umbrella label for concurrency language features label Dec 21, 2021
@kavon
some vars need unspecified isolation for initializing exprs
386940c 
It's possible to create an impossible set of constraints for
instance-member stored properties of a type. For example:

@mainactor func getStatus() -> Int { /* ... */ }
@PIDActor func genPID() -> ProcessID { /* ... */ }

class Process {
  @mainactor var status: Int = getStatus()
  @PIDActor var pid: ProcessID = genPID()

  init() {} // Problem: what is the isolation of this init?
}

We cannot satisfy the isolation of the initilizing expressions,
which demand that genStatus and genPID are run with isolation
from a non-async designated initializer, which is not possible.

This patch changes the isolation for those initializer expressions
for instance members, saying that the isolation is unspecified.

fixes rdar://84225474
@kavon kavon changed the title Defaultvalue isolation Fixes for stored property initializers Dec 22, 2021
@kavon
Copy link
Contributor Author

kavon commented Dec 22, 2021

@swift-ci please test

@kavon kavon marked this pull request as ready for review December 22, 2021 02:06
@swift-ci
Copy link
Collaborator

Build failed
Swift Test Linux Platform
Git Sha - fcd3a88f9c9d29e903dec51c1e893ee3d8e093b0

@swift-ci
Copy link
Collaborator

Build failed
Swift Test OS X Platform
Git Sha - fcd3a88f9c9d29e903dec51c1e893ee3d8e093b0

DougGregor
DougGregor approved these changes Dec 22, 2021
View reviewed changes
Copy link
Member

@DougGregor DougGregor left a comment

Choose a reason for hiding this comment

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

Looks great, thanks!

@kavon
gain isolation for static stored property initializer expressions
51f8cd1 
Some globals, like static stored properties, are lazily initialized.
For situations where we do a direct access to that property,
we will first call a function that tries to initialize the var if needed,
before returning the address of the global to perform the direct access.
In this specific case, we were not on the right actor when invoking
that function.

fixes rdar://83411416
@kavon
Copy link
Contributor Author

kavon commented Dec 22, 2021

@swift-ci please test and merge

This was referenced Dec 22, 2021
Merged
Merged
@kavon
Copy link
Contributor Author

kavon commented Jan 4, 2022

@swift-ci please test and merge

@swift-ci swift-ci merged commit 2928459 into apple:main Jan 4, 2022
kavon added a commit to kavon/swift that referenced this pull request Jan 19, 2022
@kavon
reimplement nonisolated initializing exprs for instance properties
6823744 
It's possible to create an impossible set of constraints for
instance-member stored properties of a type. For example:

@mainactor func getStatus() -> Int { /* ... */ }
@PIDActor func genPID() -> ProcessID { /* ... */ }

class Process {
  @mainactor var status: Int = getStatus()
  @PIDActor var pid: ProcessID = genPID()

  init() {} // Problem: what is the isolation of this init?
}

We cannot satisfy the isolation of the initilizing expressions,
which demand that genStatus and genPID are run with isolation
from a non-async designated initializer, which is not possible.

This patch changes the isolation for those initializer expressions
for instance members, saying that the isolation is unspecified.

fixes rdar://84225474

The first attempt to do this was in
apple#40652
But, I implemented that as a hard source break, since the isolation
was changed in a way that an error diagnostic would be emitted.
This commit reimplements the change more gently, as a warning for
Swift 5 users.
@kavon kavon mentioned this pull request Jan 19, 2022
Merged
kavon added a commit to kavon/swift that referenced this pull request Jan 20, 2022
@kavon
reimplement nonisolated initializing exprs for instance properties
c370269 
It's possible to create an impossible set of constraints for
instance-member stored properties of a type. For example:

@mainactor func getStatus() -> Int { /* ... */ }
@PIDActor func genPID() -> ProcessID { /* ... */ }

class Process {
  @mainactor var status: Int = getStatus()
  @PIDActor var pid: ProcessID = genPID()

  init() {} // Problem: what is the isolation of this init?
}

We cannot satisfy the isolation of the initilizing expressions,
which demand that genStatus and genPID are run with isolation
from a non-async designated initializer, which is not possible.

This patch changes the isolation for those initializer expressions
for instance members, saying that the isolation is unspecified.

fixes rdar://84225474

The first attempt to do this was in
apple#40652
But, I implemented that as a hard source break, since the isolation
was changed in a way that an error diagnostic would be emitted.
This commit reimplements the change more gently, as a warning for
Swift 5 users.
@kavon kavon mentioned this pull request Feb 1, 2022
Merged
@kavon kavon mentioned this pull request Jan 20, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@DougGregor DougGregor DougGregor approved these changes

Assignees
No one assigned
Labels
concurrency Feature: umbrella label for concurrency language features
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[SR-15227] MainActor static variable initialization not enforced on main thread.
3 participants
@kavon @swift-ci @DougGregor