Skip to content

Conversation

jckarter
Copy link
Contributor

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.

@jckarter
Copy link
Contributor Author

@swift-ci Please test

@jckarter
Copy link
Contributor Author

@adrian-prantl @jasonmolenda Do you recall whether it was intentional that we emitted willThrow hooks before implicit propagation like this? IIRC we only intended to call willThrow before an explicit throw in source code.

@swift-ci
Copy link
Contributor

Build failed
Swift Test OS X Platform
Git Sha - 9207930fc1d74130d7e7a1938e200cdf77d5eeb1

@adrian-prantl
Copy link
Contributor

I'm afraid I don't know anything about this. Was the commit history helpful at all?

@swift-ci
Copy link
Contributor

Build failed
Swift Test Linux Platform
Git Sha - 9207930fc1d74130d7e7a1938e200cdf77d5eeb1

@jckarter
Copy link
Contributor Author

It looks like the willThrow call was put there in @rjmccall's original commit 58b5e1d and @atrick's 482b264 without any specific commentary. I spoke with @rjmccall, and his recollection matched mine.

…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.
@jckarter jckarter force-pushed the willthrow-only-on-throw branch from 9207930 to c3a2d98 Compare December 22, 2017 00:43
@jckarter
Copy link
Contributor Author

@swift-ci Please test

@swift-ci
Copy link
Contributor

Build failed
Swift Test Linux Platform
Git Sha - 9207930fc1d74130d7e7a1938e200cdf77d5eeb1

@swift-ci
Copy link
Contributor

Build failed
Swift Test OS X Platform
Git Sha - 9207930fc1d74130d7e7a1938e200cdf77d5eeb1

@rjmccall
Copy link
Contributor

rjmccall commented Dec 22, 2017

Yes, I think this is reasonable.

You could make an argument that we should call willThrow before starting the propagation of an error received from ObjC, just so that such throws do always get trapped. However, that would still sweep up a fair amount of propagation-rethrows and expose the details of exactly how we performed a call. I think the "it's only triggered by Swift throw statements" line is something that users will understand.

@jckarter
Copy link
Contributor Author

jckarter commented Jan 2, 2018

ObjC-to-Swift propagation has its own code paths, so we could conceivably still emit the hook when propagating in that situation.

@jckarter jckarter changed the title SILGen: Don't inject "willThrow" hooks before rethrow propagation branches. [WIP] SILGen: Don't inject "willThrow" hooks before rethrow propagation branches. Jan 3, 2018
@jckarter
Copy link
Contributor Author

jckarter commented Jan 3, 2018

I discussed this a bit with @jimingham on swift-lldb-dev. We should keep the willThrow call after ObjC calls that bridge to throw.

@CodaFi
Copy link
Contributor

CodaFi commented Nov 17, 2019

@jckarter Do you think this would make for a good starter bug in IRGen?

@jckarter
Copy link
Contributor Author

Yeah, that's a good idea!

@CodaFi
Copy link
Contributor

CodaFi commented Nov 18, 2019

Filed https://bugs.swift.org/browse/SR-11803. Please let me know if I'm missing any details in there.

Going to close this out.

@CodaFi CodaFi closed this Nov 18, 2019
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