Skip to content

Conversation

beccadax
Copy link
Contributor

Cherry-picks #64525 to release/5.9:

This PR undoes the revert of #63668 and adds additional changes to fix a bug blocking integration and improve tests. The original:

This PR introduces a new, more robust checker for the members of @_objcImplementation extensions and makes related Sema-level improvements. With it, swiftc can now:

  • Infer @objc on member implementations.
  • Emit diagnostics which describe currently-proposed semantics (e.g. using fileprivate and private to create @objc helper methods).
  • Globally handle name matching (for instance, if there are two ObjC methods with the same Swift name, adding @objc(<explicit selector>) to one of the Swift implementations will make both of them match the right method).
  • Diagnose and offer fix-its for mismatched names in some situations.
  • Allow explicit deinits in @_objcImplementation extensions.

This PR is also capable of detecting ambiguous matches, but these situations are not being diagnosed yet because I haven't figured out how to avoid burying users in a blizzard of diagnostics.

Fixes rdar://103150189.

This PR additionally:

  • Excludes @objcImplementation member implementations from being included in module interfaces, fixing the issue that originally forced the revert.
  • Re-enables the IRGen/objc_implementation.swift test on macOS, where it's known to work correctly.
  • Tweaks the test modifications in IRGen/objc_implementation.swift relating to deinit support.
  • NEW: Fixes an @objcImplementation-specific bug in the logic which decides whether to print the override keyword in a module interface.

• `@objc` is now inferred on non-`final` members of @objcImplementation extensions
• Diagnostics now suggest adding `private` to ObjC helper members, not `@objc`, in line with currently proposed behavior
• Better diagnostic for members implemented in the wrong extension

Part of rdar://103150189.
Create a checker for @_objcImplementation member implementations that considers all of a class’s interface and implementation decls at once. This allows us to handle several things better:

• Unimplemented requirements are now diagnosed

• Header members that can match several implementations, or implementations that could match several header members, are now diagnosed

• Tailored diagnostic when the implementation's Swift name matches the header's selector instead of its Swift name

• Recommends inserting `@objc(<selector>)` when a Swift name matches but the implicit ObjC name doesn't

• An `@objc(<selector>)` on one implementation can eliminate its requirement from being considered for other implementations, resolving ambiguities

This does unfortunately regress the diagnostics when a requirement is implemented in the wrong extension. Some sort of whole-module checking would be needed to address this problem.
Module interfaces should not include the @objcImplementation attribute, member implementations that are redundant with the ObjC header, or anything that would be invalid in an ordinary extension (e.g. overridden initializers, stored Swift-only properties).
Partly fixes rdar://101420862, and catches up the test with subsequent changes.
When writing an @objc subclass of an @objcImplementation class, implicit initializers in the subclass were treated as overriding the *implementation decl*, not the *interface decl*, of the initializer in the superclass. This caused Swift to incorrectly compute the visibility of the superclass initializer and omit an `override` keyword from the module interface when one would definitely be necessary.

Correct this oversight by looking up the interface decl matching the superclass implementation in `collectNonOveriddenSuperclassInits()`.
Switch the ModuleInterface/objc_implementation.swift test to using standard substitutions to properly emit its module interface, and make sure it also verifies.
@beccadax beccadax requested a review from a team as a code owner March 30, 2023 22:42
@beccadax
Copy link
Contributor Author

@swift-ci please test

@beccadax beccadax merged commit e05b958 into swiftlang:release/5.9 Mar 31, 2023
@AnthonyLatsis AnthonyLatsis added the 🍒 release cherry pick Flag: Release branch cherry picks label May 3, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🍒 release cherry pick Flag: Release branch cherry picks
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants