Skip to content

[PrintAsObjC] Add __attribute__((warn_unused_result)) for non-@discardableResult methods #6179

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 3 commits into from
Dec 10, 2016

Conversation

timbodeit
Copy link
Contributor

Clang importer currently marks methods imported from Objective-C as @discardableResult based on their warn_unused_result clang attribute.
I would expect PrintAsObjC to do the same the other way around.

This change adds a new SWIFT_WARN_UNUSED_RESULT macro to the prologue of the generated interface header files. This macro evaluates to __attribute__((warn_unused_result)) where supported.

The SWIFT_WARN_UNUSED_RESULT attribute is applied to the Objective-C representation of all non-void methods, that don't have a @discardableResult attribute. The attribute is not emitted for initializers.
It is also not emitted where the error convention leads to a return value for an otherwise void returning method.

Resolves SR-3383.

* Add `SWIFT_WARN_UNUSED_RESULT` macro to prologue.
  Evaluates to `__attribute__((warn_unused_result))` where supported.
* Emit `SWIFT_WARN_UNUSED_RESULT` attribute in generated ObjC headers
  for all non-void methods, that don't have an @discardableResult
  attribute.
  Attribute is not emitted for initializers. Attribute is also not
  emitted where the error convention leads to a return value for an
  otherwise void returning method.
Copy link
Contributor

@jrose-apple jrose-apple left a comment

Choose a reason for hiding this comment

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

This looks great, thanks Tim! One requested test change but other than that everything seems ready to go.

// CHECK-NEXT: - (NSInteger)nonDiscardable:(NSInteger)x SWIFT_WARN_UNUSED_RESULT;
func nonDiscardable(_ x: Int) -> Int { return x }
// CHECK-NEXT: - (NSInteger)discardable:(NSInteger)x;
@discardableResult func discardable(_ x: Int) -> Int { return x }
Copy link
Contributor

Choose a reason for hiding this comment

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

Rather than add a whole new file, I would just stick this in with methods.swift.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@jrose-apple I didn't find a methods.swift file within the PrintAsObjC tests. I put the test case into classes.swift instead.

Copy link
Contributor

Choose a reason for hiding this comment

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

Whoops, yes, that's fine.

Test, that no SWIFT_WARN_UNUSED_RESULT attribute is included in the ObjC
representation of a Swift function annotated with @discardableResult.
@timbodeit timbodeit force-pushed the printasobjc-warnunusedresult branch from b75ec3a to 4eca52e Compare December 10, 2016 01:33
@jrose-apple
Copy link
Contributor

@swift-ci Please test macOS

@jrose-apple
Copy link
Contributor

@swift-ci Please smoke test Linux

@jrose-apple jrose-apple merged commit 21e401f into swiftlang:master Dec 10, 2016
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.

2 participants