Skip to content

[region-isolation] Reword merge-region diagnostics so we never fail on missing names#89270

Open
gottesmm wants to merge 1 commit into
swiftlang:mainfrom
gottesmm:pr-a4bb80740cb2c23d59d4ca24e8ff2565468c2737
Open

[region-isolation] Reword merge-region diagnostics so we never fail on missing names#89270
gottesmm wants to merge 1 commit into
swiftlang:mainfrom
gottesmm:pr-a4bb80740cb2c23d59d4ca24e8ff2565468c2737

Conversation

@gottesmm
Copy link
Copy Markdown
Contributor

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

…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
@gottesmm
Copy link
Copy Markdown
Contributor Author

@swift-ci smoke test

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