[SendNonSendable] Diagnose consumption sites not requirement sites #67318
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
JTurcotti
requested review from
ktoso,
kavon,
hborla,
slavapestov and
xedin
as code owners
…cesses to consumed values
|
@swift-ci please smoke test |
kavon
reviewed
Jul 14, 2023
| @@ -196,7 +211,7 @@ class Partition { | |||
| return; | |||
| canonical = true; | |||
|
|
|||
| std::map<signed, unsigned> relabel; | |||
| std::map<unsigned, unsigned> relabel; | |||
There was a problem hiding this comment.
oh no now I can't tell what this means at all. can your next PR at least include those typealiases for unsigned and signed to give them a better name?
There was a problem hiding this comment.
ok, performing this refactor!
Comment on lines
+669
to
+679
| enum class LocalConsumedReasonKind { | ||
| LocalConsumeInst, | ||
| LocalNonConsumeInst, | ||
| NonLocal | ||
| }; | ||
|
|
||
| // Why was a value consumed, without looking across blocks? | ||
| // kind == LocalConsumeInst: a consume instruction in this block | ||
| // kind == LocalNonConsumeInst: an instruction besides a consume instruction | ||
| // in this block | ||
| // kind == NonLocal: an instruction outside this block |
There was a problem hiding this comment.
good to move these explainer comments directly onto the LocalConsumedReasonKind cases instead of on top of the struct containing it so it shows up in IDE documentation windows for what LocalConsumedReasonKind::X means.
test/Concurrency/DeferredSendableChecking/region_based_sendability.swift
Show resolved
Hide resolved
|
@swift-ci please smoke test |
1 similar comment
|
@swift-ci please smoke test |
…ions, and ensure that all SILValues are converted once to TrackableSILValues in the SendNonSendable analysis pass
|
@swift-ci please smoke test |
|
@swift-ci please smoke test |
kavon
approved these changes
Jul 18, 2023
…in improving diagnostic messages
|
@swift-ci please smoke test |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
In prior versions of the SendNonSendable pass, diagnostics were reported when it was discovered that a value in a consumed region was accessed. This is confusing as the site that performed the consumption is the site that actually invoked a cross-isolation call or other concurrency primitive. This PR changes the behavior to instead search for the site at which an original consumption took place, and diagnose that site along with notes of some potential accesses.