Skip to content

[region-isolation] Convert more diagnostics from type isolation form to named variable form #72435

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.

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

Conversation

gottesmm
Copy link
Contributor

@gottesmm gottesmm commented Mar 19, 2024

Just chopping off parts of a larger PR that can exist independently.

In this PR, I eliminate a ton more of the cases where we failed to identify a value name resulting in us emitting a type isolation error. Example:

// After
func letSendableTrivialClassFieldTest() async {
  let test = ClassFieldTests()
  await transferToMain(test) // expected-tns-warning {{transferring 'test' may cause a race}}
  // expected-tns-note @-1 {{'test' is transferred from nonisolated caller to main actor-isolated callee. Later uses in caller could race with potential uses in callee}}
  // expected-complete-warning @-2 {{passing argument of non-sendable type 'ClassFieldTests' into main actor-isolated context may introduce data races}}
  _ = test.letSendableTrivial
  useValue(test) // expected-tns-note {{access here could race}}
}

from,

// Before
func letSendableTrivialClassFieldTest() async {
  let test = ClassFieldTests()
  await transferToMain(test) // expected-tns-warning {{transferring value of non-Sendable type 'ClassFieldTests' from nonisolated context to main actor-isolated context; later accesses could race}}
  // expected-complete-warning @-1 {{passing argument of non-sendable type 'ClassFieldTests' into main actor-isolated context may introduce data races}}
  _ = test.letSendableTrivial
  useValue(test) // expected-tns-note {{access here could race}}
}

I still need to change that specific diagnostic to use IsolationRegionInformation rather than pure callee information, but this an improvement (and more importantly just what I needed to add to do some other work). Once I am able to loop back around that would cause the note diagnostic to be transformed as follows:

  // expected-tns-note @-1 {{'test' is transferred from nonisolated caller to main actor-isolated callee. Later uses in caller could race with potential uses in callee}}

->

  // expected-tns-note @-1 {{disconnected 'test' is transferred to main actor-isolated callee. Later uses in caller could race with actor-isolated uses in callee}}

…transfer error.

This is just a pseudo-why are these two things part of the same region error. I
am going to remove this for now and the proper form of this diagnostic will come
back when I land the region history functionality.
… store_borrow initialized variables.

This eliminates a bunch of cases where we couldn't infer the name of a variable
and used the type based diagnostic.

It also eliminates an 'unknown' case for move checking.
… patterns.

Just slicing off from a larger patch.
@gottesmm gottesmm requested review from kavon and ktoso as code owners March 19, 2024 20:22
@gottesmm
Copy link
Contributor Author

@swift-ci smoke test

@gottesmm gottesmm enabled auto-merge March 19, 2024 20:22
@gottesmm gottesmm merged commit f025657 into swiftlang:main Mar 19, 2024
@gottesmm gottesmm deleted the pr-89ce065b7f50a41a58d8fb358d0145913d8b53a3 branch March 20, 2024 01:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant