[region-isolation] Reword merge-region diagnostics so we never fail on missing names#89270
Open
gottesmm wants to merge 1 commit into
Open
[region-isolation] Reword merge-region diagnostics so we never fail on missing names#89270gottesmm wants to merge 1 commit into
gottesmm wants to merge 1 commit into
Conversation
…n missing names
Restructure the five "incompatible region merge" diagnostics emitted by
IncompatibleRegionMergeDiagnosticEmitter so that the primary error never
depends on recovering a value name. Previously, when VariableNameInferrer
could not infer a name for either side of the merge, every emitter fell
back to emitUnknownPatternError() and the user got the unhelpful "we do
not understand this pattern" diagnostic for a violation the analysis had
in fact understood.
Now:
* Each primary states the *operation* that performs the merge and the
two isolation domains involved -- e.g. "passing arguments to %kind2
could allow for references between values exposed to %0 and %1 code,
risking data races". The primary requires only the isolation strings
and (where applicable) the callee decl or cast target type.
* Per-value details move to follow-up notes anchored at each value's
*defining* source location (parameter decl for SILFunctionArgument,
defining-instruction loc otherwise) so the user can see where each
side came from. Two note variants: named ("'self.x' is exposed to ...
code") when name inference succeeds, bare ("value is exposed to ...
code") otherwise. We deliberately do not emit a "value of type T"
variant because SIL values often do not carry a reliable AST type.
* The shared emitMergeRegionValueNote() helper centralises note
emission and the new preciseSourceLocForValue() helper picks the
most precise SourceLoc available for a value.
emitNonisolatedFunction now falls through to emitUnknown rather than
emitUnknownPatternError when ApplySite/getCalleeDeclRef recovery fails,
so the primary still fires with isolation information in those cases.
emitUnknownPatternError remains reachable only when isolation info
itself is invalid -- a genuinely unknown pattern.
rdar://175592408
Contributor
Author
|
@swift-ci smoke test |
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
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.
Restructure the five "incompatible region merge" diagnostics emitted by IncompatibleRegionMergeDiagnosticEmitter so that the primary error never depends on recovering a value name. Previously, when VariableNameInferrer could not infer a name for either side of the merge, every emitter fell back to emitUnknownPatternError() and the user got the unhelpful "we do not understand this pattern" diagnostic for a violation the analysis had in fact understood.
Now:
Each primary states the operation that performs the merge and the two isolation domains involved -- e.g. "passing arguments to %kind2 could allow for references between values exposed to %0 and %1 code, risking data races". The primary requires only the isolation strings and (where applicable) the callee decl or cast target type.
Per-value details move to follow-up notes anchored at each value's defining source location (parameter decl for SILFunctionArgument, defining-instruction loc otherwise) so the user can see where each side came from. Two note variants: named ("'self.x' is exposed to ... code") when name inference succeeds, bare ("value is exposed to ... code") otherwise. We deliberately do not emit a "value of type T" variant because SIL values often do not carry a reliable AST type.
The shared emitMergeRegionValueNote() helper centralises note emission and the new preciseSourceLocForValue() helper picks the most precise SourceLoc available for a value.
emitNonisolatedFunction now falls through to emitUnknown rather than emitUnknownPatternError when ApplySite/getCalleeDeclRef recovery fails, so the primary still fires with isolation information in those cases. emitUnknownPatternError remains reachable only when isolation info itself is invalid -- a genuinely unknown pattern.
rdar://175592408