-
Notifications
You must be signed in to change notification settings - Fork 10.5k
[SR-12723][Sema] Validate function type param representations thick-to-thin conversions #31814
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 all commits
8f2f14b
bacbc57
baccbde
d6bf34e
adba283
da53129
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 |
---|---|---|
@@ -0,0 +1,81 @@ | ||
// RUN: %target-swift-emit-silgen %s -verify | ||
|
||
func SR12723_thin(_: (@convention(thin) () -> Void) -> Void) {} | ||
func SR12723_block(_: (@convention(block) () -> Void) -> Void) {} | ||
func SR12723_c(_: (@convention(c) () -> Void) -> Void) {} | ||
|
||
func SR12723_function(_: () -> Void) {} | ||
|
||
func context() { | ||
SR12723_c(SR12723_function) | ||
|
||
SR12723_block(SR12723_function) | ||
|
||
SR12723_thin(SR12723_function) | ||
} | ||
|
||
struct SR12723_C { | ||
let function: (@convention(c) () -> Void) -> Void | ||
} | ||
|
||
struct SR12723_Thin { | ||
let function: (@convention(thin) () -> Void) -> Void | ||
} | ||
|
||
struct SR12723_Block { | ||
let function: (@convention(block) () -> Void) -> Void | ||
} | ||
|
||
func proxy(_ f: (() -> Void) -> Void) { | ||
let a = 1 | ||
f { print(a) } | ||
} | ||
|
||
func cContext() { | ||
let c = SR12723_C { app in app() } | ||
|
||
proxy(c.function) | ||
// expected-error@-1 {{cannot convert value of type '(@convention(c) () -> Void) -> Void' to expected argument type '(() -> Void) -> Void'}} | ||
|
||
let _ : (@convention(block) () -> Void) -> Void = c.function | ||
// expected-error@-1 {{cannot convert value of type '(@convention(c) () -> Void) -> Void' to specified type '(@convention(block) () -> Void) -> Void'}} | ||
|
||
let _ : (@convention(c) () -> Void) -> Void = c.function // OK | ||
|
||
let _ : (@convention(thin) () -> Void) -> Void = c.function // OK | ||
|
||
let _ : (() -> Void) -> Void = c.function | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This and some other conversions here are There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. At least this is what Joe mentioned in his comment. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hey @xedin :) Also, the logic on There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, you are correct, sorry for the confusion. I was just merely trying to point out that we might want to change diagnostic or add a custom note which explains precisely why these types couldn't be converted (related to contravariance) because it appears that it's passing There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I got tripped by this myself because I didn't notice that diagnostic mentioned whole function type... There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Right, thanks for the clarification :)
About the note, I think it makes sense, but I'm not sure how to phrase it because I'm not sure if we should mention variance, or only mention that the representations are not convertible in a parameter subtype context ... There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah, it's a good question how note should be worded exactly... There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. After thinking about this some more - I think we should just open a follow up SR for the note and merge what we have now. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think it makes sense :) |
||
// expected-error@-1 {{cannot convert value of type '(@convention(c) () -> Void) -> Void' to specified type '(() -> Void) -> Void'}} | ||
|
||
} | ||
|
||
func thinContext() { | ||
let thin = SR12723_Thin { app in app() } | ||
|
||
proxy(thin.function) | ||
// expected-error@-1 {{cannot convert value of type '(@convention(thin) () -> Void) -> Void' to expected argument type '(() -> Void) -> Void'}} | ||
|
||
let _ : (@convention(block) () -> Void) -> Void = thin.function | ||
// expected-error@-1 {{cannot convert value of type '(@convention(thin) () -> Void) -> Void' to specified type '(@convention(block) () -> Void) -> Void'}} | ||
|
||
let _ : (@convention(c) () -> Void) -> Void = thin.function // OK | ||
|
||
let _ : (@convention(thin) () -> Void) -> Void = thin.function // OK | ||
|
||
let _ : (() -> Void) -> Void = thin.function | ||
// expected-error@-1 {{cannot convert value of type '(@convention(thin) () -> Void) -> Void' to specified type '(() -> Void) -> Void'}} | ||
} | ||
|
||
func blockContext() { | ||
let block = SR12723_Block { app in app() } | ||
|
||
proxy(block.function) | ||
|
||
let _ : (@convention(block) () -> Void) -> Void = block.function // OK | ||
|
||
let _ : (@convention(c) () -> Void) -> Void = block.function // OK | ||
|
||
let _ : (@convention(thin) () -> Void) -> Void = block.function // OK | ||
|
||
let _ : (() -> Void) -> Void = block.function // OK | ||
} |
Uh oh!
There was an error while loading. Please reload this page.