Mimic old objcImpl behavior for early adopters #76462
Merged
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
Before the update to support the new syntax, the decl checker treated private and fileprivate members of objcImpl extensions as non-@objc by default. But SE-0436 specified that private and fileprivate members should be implicitly @objc unless opted out, so when support for the final syntax was added, this behavior was changed retroactively. Unfortunately, we’ve found that some early adopters depended on the old behavior. Restore some logic deleted by swiftlang#73309 for early adopter syntax *only*, with a warning telling the developer to put `final` or `@nonobjc` on the declaration if they want to preserve the behavior after updating it. Also tweaks the ObjCImplementationChecker to ensure this logic will actually be run in time for it to suppress the “upgrade to @objc @implementation” warning, and corrects a couple of regressions arising from that change. Fixes rdar://135747897.
beccadax
requested review from
hborla,
slavapestov,
xymus,
eeckstein and
xedin
as code owners
|
@swift-ci please test |
A previous change made it so that an objcImpl extension could declare a `final` or `static` member which overrode an `@objc` superclass method, or witnessed an `@objc` protocol requirement, without itself being `@objc`. Prevent that from happening by allowing processing of `final` members to continue to later codepaths, rather than returning early. Fixes rdar://136113393.
Automatically detect when shouldMarkAsObjC() will behave differently based on the objcImpl early adopter flag and diagnose it. This works in either direction, although there isn’t anything yet that will emit diag:: objc_implementation_will_become_nonobjc. NFC except for some minor changes to the wording of notes.
The combination `@objc @_objcImplementation extension` used to be treated like a normal `@objc extension`. Restore that behavior for early adopters.
|
@swift-ci please test |
tshortli
approved these changes
Sep 20, 2024
|
@swift-ci please test |
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.
Before the update to support the new syntax, the decl checker treated private and fileprivate members of objcImpl extensions as non-@objc by default. But SE-0436 specified that private and fileprivate members should be implicitly @objc unless opted out, so when support for the final syntax was added, this behavior was changed retroactively.
Unfortunately, we’ve found that some early adopters depended on the old behavior. Restore some logic deleted by #73309 for early adopter syntax only, with a warning telling the developer to put
finalor@nonobjcon the declaration if they want to preserve the behavior after updating it.Also tweaks the ObjCImplementationChecker to ensure this logic will actually be run in time for it to suppress the “upgrade to @objc @implementation” warning, and corrects a couple of regressions arising from that change.
Edit: I've added additional commits to correct other issues with this part of the code in #73309:
@objc @_objcImplementation extensionwith a warning telling the developer to correct their codestaticorfinalmember which overrode a superclass method or witnessed a protocol requirement would not be inferred as@objc.Fixes rdar://135747897 and rdar://136113393.
Cherry-picking note: This relies on changes in the final version of #76270.