Skip to content

Conversation

@mikeash
Copy link
Contributor

@mikeash mikeash commented Oct 23, 2019

rdar://problem/53400364

Remove the destructor from ConcurrentList so it can be used as a global. This is the only use of the type aside from a test in Metadata.cpp. rdar://problem/53400364
The two ErrorObject implementations (ObjC and non-ObjC) can share the implementation of the will throw callbacks. rdar://problem/53400364
@mikeash mikeash requested a review from stmontgomery October 23, 2019 15:57

/// SPI for registering a callback to be called whenever an error is thrown.
@_silgen_name("_swift_addErrorWillThrowCallback")
public func _addErrorWillThrowCallback(_ callback: @escaping (Error) -> Void)
Copy link
Contributor

Choose a reason for hiding this comment

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

Don't have a strong opinion, but maybe _addWillThrowErrorCallback would be a clearer name? (i.e. "will throw error" instead of "error will throw".) Just since Error types don't perform the throw, they are thrown.

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.

What are you trying to do? swift_willThrow is not reliably emitted by the compiler, and I don't think we want to lock down the places we happen to emit it today. It gets optimized out of release builds, and gets emitted too often in debug builds (you'll get a callback along every implicit propagation edge, not just at the throw statement). I don't think it's suitable for adding any logic a program might depend on in its current state. Even if it were more reliably emitted, I don't think we'd want to expose this even as SPI, because it's easy to misuse, and we wouldn't be able to prevent people from doing so. It's semantically problematic because the optimizer expects throwing or error propagation to be a pure operation like return; inlining or control flow transformations could arbitrary introduce or remove throws.

@mikeash
Copy link
Contributor Author

mikeash commented Oct 24, 2019

@jckarter and I discussed this some more offline and we decided that a function pointer like _swift_retain would be a decent way to do this. That makes it more obviously dangerous and less prone to weird abuse, and it won't show up in the API (even with a leading underscore).

Since that's fairly different from what I have here, I've created a new PR for that over at #27863.

@mikeash mikeash closed this Oct 24, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

3 participants