-
Notifications
You must be signed in to change notification settings - Fork 10.6k
[Sema] @objc functions shall not have typed throw #81054
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
Changes from 7 commits
b664a40
cf15d46
813aae6
3afddfa
841b583
ea05881
05644c1
5af6e5e
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -809,6 +809,26 @@ bool swift::isRepresentableInLanguage( | |||||
| return false; | ||||||
| } | ||||||
| } | ||||||
| auto isTypedThrow = [&]() { | ||||||
| // With no AST token this should be `throws` | ||||||
| if (!AFD->getThrownTypeRepr()) | ||||||
| return false; | ||||||
|
||||||
| // Or it is a `throws(<ErrorType>)` | ||||||
| CanType thrownType = AFD->getThrownInterfaceType()->getCanonicalType(); | ||||||
| // TODO: only `throws(Error)` is allowed. | ||||||
| // Throwing `any MyError` that confronts `Error` is not implemented yet. | ||||||
| // Shall we allow `any MyError` in the future, we should check against | ||||||
| // `isExistentialType` instead. | ||||||
| if (thrownType->isErrorExistentialType()) | ||||||
|
||||||
| if (thrownType->isErrorExistentialType()) | |
| if (thrownType->isNever() || thrownType->isErrorExistentialType()) |
You would not need to explicitly check hasThrows() before calling your isTypedThrow() function.
(If you don't do the isNever() check, you might want to remove the call to getCanonicalType() on line 817—TypeBase::isErrorExistentialType() implicitly canonicalizes the type, so you don't have to.)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thrownType could be a null pointer if not getCanonicalType, even if AFD hasThrow. I'm not sure why.
I'd prefer keep the hasThrow check to make the intention more clear, it also made more sense to erase the closure that this point.
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, checking this before the sync and async code paths diverge is clever!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would probably word this more similarly to the other diagnostics emitted by
isRepresentableInLanguage(), such asnot_objc_function_async:Note that this will insert a string like "instance method" where
%kindonly0is written—you'll need to pass the decl into thediagnose()call, though.(There are other ways you might word this—if you'd like, take a look around at a few other diagnostics in this method and see if there's something you like better.)