Skip to content

Conversation

@jckarter
Copy link
Contributor

@jckarter jckarter commented Mar 26, 2024

It works well enough now that it should be an acceptable replacement for both borrowing and consuming switches that works in more correct situations than the previous implementation. This does however expose a few known issues that I'll try to fix in follow ups:

  • cases with multiple pattern labels aren't yet supported (rdar://125188955)
  • copyable types with the borrowing or consuming modifiers should probably use noncopyable pattern matching.

The BorrowingSwitch flag is still necessary to enable the surface-level syntax changes (switches without consume and the _borrowing modifier, for instance).

rdar://126209276

@jckarter jckarter requested a review from kavon as a code owner March 26, 2024 01:30
@jckarter
Copy link
Contributor Author

@swift-ci Please test

@jckarter
Copy link
Contributor Author

@swift-ci Please test source compatibility

@jckarter
Copy link
Contributor Author

@swift-ci Please test macOS

jckarter added 2 commits April 9, 2024 14:52
…um_data`.

Under OSSA, the instruction may still be structurally responsible for consuming
its operand even if the result is dead, so we can't remove it without breaking
invariants.

More generally, this should probably apply to any instruction which consumes
one or more of its operands, has no side effects, and doesn't produce any
nontrivial results that require further consumption to keep the value alive.
I went with this targeted fix, since it addresses a problem that shows up
in practice (rdar://125381446) and the more general change appears to
disturb the optimizer pipeline while building the standard library.
It works well enough now that it should be an acceptable replacement for both
borrowing and consuming switches that works in more correct situations than the
previous implementation. This does however expose a few known issues that I'll
try to fix in follow ups:

- overconsumes cause verifier errors instead of raising diagnostics (rdar://125381446)
- cases with multiple pattern labels aren't yet supported (rdar://125188955)
- copyable types with the `borrowing` or `consuming` modifiers should probably use
  noncopyable pattern matching.

The `BorrowingSwitch` flag is still necessary to enable the surface-level syntax
changes (switches without `consume` and the `_borrowing` modifier, for instance).
@jckarter jckarter force-pushed the enable-borrowing-switch-backend branch from a9bfca5 to 5ad2603 Compare April 10, 2024 00:30
@jckarter jckarter requested a review from eeckstein as a code owner April 10, 2024 00:30
@jckarter
Copy link
Contributor Author

@swift-ci Please test

@jckarter
Copy link
Contributor Author

@swift-ci Please test source compatibility

Copy link
Contributor

@eeckstein eeckstein left a comment

Choose a reason for hiding this comment

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

nice!
lgtm

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.

2 participants