Re-merge #63668: Improve @objcImplementation member checking #64525
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
beccadax
requested review from
hborla,
slavapestov,
xedin,
zoecarver,
hyp and
egorzhdan
as code owners
tshortli
reviewed
Mar 22, 2023
|
@swift-ci test |
tshortli
approved these changes
Mar 23, 2023
|
Tightened up some assertions in a test to avoid false positives. @swift-ci please test |
• `@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.
|
@swift-ci please smoke test and merge |
|
Rebased to fix a merge conflict. |
|
@swift-ci smoke test and merge |
1 similar comment
|
@swift-ci smoke test and merge |
|
Can we also merge this to 5.9 branch? |
beccadax
added a commit
to beccadax/swift
that referenced
this pull request
Aug 21, 2024
This became dead when we started adding synthesized destructors to the implementation extension in swiftlang#64525.
beccadax
added a commit
to beccadax/swift
that referenced
this pull request
Aug 21, 2024
This became dead when we started adding synthesized destructors to the implementation extension in swiftlang#64525.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR undoes the revert of #63668 and adds additional changes to fix a bug blocking integration and improve tests. The original:
This PR additionally:
@objcImplementationmember implementations from being included in module interfaces, fixing the issue that originally forced the revert.deinitsupport.@objcImplementation-specific bug in the logic which decides whether to print theoverridekeyword in a module interface.