Skip to content

[SILGEN] Inject "willThrow" hooks after foreign error propagation. #28455

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
merged 2 commits into from
Dec 4, 2019

Conversation

kitaisreal
Copy link
Contributor

  1. Don't inject "willThrow" hooks before rethrow propagation branches.
  2. Inject "willThrow" hooks after foreign error propagation branches.
  3. Updated tests

cc @jckarter
More information in ticket: https://bugs.swift.org/browse/SR-11803

jckarter and others added 2 commits November 23, 2019 16:16
…nches.

The hook is intended to be used by debuggers to catch the point a `throw` happened in user source. It's unnecessary and undesirable to hook in places where an already-thrown error is just being implicitly propagated.
@CodaFi CodaFi requested a review from jckarter November 23, 2019 18:21
Copy link
Contributor

@jckarter jckarter left a comment

Choose a reason for hiding this comment

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

LGTM. @dcci or @adrian-prantl might want to look over this too. Does this adequately preserve coverage for "Swift Error" breakpoints?

@adrian-prantl
Copy link
Contributor

@kitaisreal Can you explain why it is safe to omit these specific willThrow calls in this patch?

@jckarter
Copy link
Contributor

jckarter commented Dec 2, 2019

The error should already have been thrown at the point these calls were being inserted. This patch should affect paths where an already-thrown error is being propagated up the callstack.

@adrian-prantl
Copy link
Contributor

That seems reasonable then.

@dcci
Copy link
Member

dcci commented Dec 2, 2019

@swift-ci test

@CodaFi
Copy link
Contributor

CodaFi commented Dec 4, 2019

⛵️

@CodaFi CodaFi merged commit 2b1c1a2 into swiftlang:master Dec 4, 2019
@CodaFi
Copy link
Contributor

CodaFi commented Dec 4, 2019

@kitaisreal Don't forget to update the SR!

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.

5 participants