Add @_objcImplementation #60630

beccadax · 2022-08-18T18:01:47Z

This PR adds a mechanism to implement "pure" Objective-C classes in Swift. Unlike the existing @objc interop mechanism, these classes use traditional handwritten ObjC header files and can be subclassed, swizzled, and otherwise manipulated from ObjC code like any other ObjC class.

To do this, you arrange to import the header file via your bridging or umbrella header, as though it were implemented in Objective-C, and then you apply the @_objcImplementation attribute to an extension and implement the methods there. Each @_objcImplementation extension corresponds to one @interface in the header; the attribute can specify a category name in parentheses to implement a category, or it can leave the category name out to implement the main body of the class. The extension that implements the main body of the class can declare stored properties.

This initial PR includes fairly minimal support sufficient to allow you to implement initializers, methods, stored properties, and accessors/observers for ObjC classes without leaking memory. Sema-level diagnostics are very incomplete and stored resilient properties have not yet been tested. The vast majority of Swift-specific metadata has been eliminated, but there may still be a couple of things that shouldn't be there.

Parser reviewers: There is a matching pull request, swiftlang/swift-syntax#798, which includes a test for the new attribute.

Fixes rdar://70730077.

test/IRGen/objc_implementation.m

docs/ReferenceGuides/UnderscoredAttributes.md

include/swift/IRGen/Linking.h

include/swift/AST/Decl.h

lib/IRGen/GenClass.cpp

lib/IRGen/GenDecl.cpp

Part of swiftlang/swift#60630, but safe to merge before the compiler-side implementation.

beccadax · 2022-10-05T03:24:51Z

@swift-ci please test

Part of swiftlang/swift#60630, but safe to merge before the compiler-side implementation.

beccadax · 2022-10-05T17:41:23Z

With swiftlang/swift-syntax#798

@swift-ci please test

beccadax · 2022-10-05T18:59:08Z

With swiftlang/swift-syntax#798

@swift-ci please test

beccadax · 2022-10-08T01:28:09Z

With swiftlang/swift-syntax#798

@swift-ci please test

beccadax · 2022-10-11T19:15:00Z

@swift-ci build toolchain

beccadax · 2022-10-11T22:51:31Z

With swiftlang/swift-syntax#798

@swift-ci please build toolchain macOS platform

Does not validate members yet; nor does it emit different metadata.

@objc

They either must be @objc dynamic (the dynamic is implicit) or final.

Stored properties are only allowed in the extension implementing the class's main interface, not its categories. This also means banning `@objc final`, which is unenforceable anyway when ObjC subclasses are allowed, and therefore allowing `@objc let` and `@objc static` properties to be overridden if they're declared in objcImplementations.

@objc

This commit begins to generate correct metadata for @_objcImplementation extensions: • Swift-specific metadata and symbols are not generated. • For main-class @_objcImpls, we visit the class to emit metadata, but visit the extension’s members. • Includes both IR tests and executable tests, including coverage of same-module @objc subclasses, different-module @objc subclasses, and clang subclasses. The test cases do not yet cover stored properties.

Only works for trivial types right now because features related to initialization and deinitialization are seriously busted.

Accessing a stored property from a normal method should send a selector, but accessing it from an initializer or accessor should use the field offset.

This is failing in the iOS simulator, but *only* in Swift CI, not locally (even with the same tools). Disable the test ouside of macOS while we investigate the failure.

Part of swiftlang/swift#60630, but safe to merge before the compiler-side implementation.

beccadax · 2022-10-19T17:51:29Z

With swiftlang/swift-syntax#798

@swift-ci please test

beccadax · 2022-10-19T20:25:28Z

With swiftlang/swift-syntax#798

@swift-ci please clean test

beccadax · 2022-10-19T22:58:35Z

With swiftlang/swift-syntax#798

@swift-ci please clean test

DougGregor

This is looking great. I appreciate the testing discipline for this tricky feature.

tbkka · 2022-10-22T00:34:38Z

I'm seeing a failure on a local build of main on Ubuntu 22.10:

.../swift/include/swift/AST/Attr.h:2289:21: error: use of undeclared identifier 'DAK_ObjCImplementation'
    : DeclAttribute(DAK_ObjCImplementation, AtLoc, Range, Implicit),
.../swift/include/swift/AST/Attr.h:2303:29: error: use of undeclared identifier 'DAK_ObjCImplementation'
    return DA->getKind() == DAK_ObjCImplementation;

Any ideas? Is DAK_ObjCImplementation supposed to be declared on Linux?

tbkka · 2022-10-22T01:26:58Z

Ignore that. I just updated all my sources and now it seems to build fine, so whatever this issue was you must have already fixed it. Thanks!!

ahoppen · 2022-10-22T09:03:55Z

This PR was paired with swiftlang/swift-syntax#798 and you needed to pull that PR’s commits to fix your build.

rjmccall · 2022-10-25T03:52:48Z

docs/ReferenceGuides/UnderscoredAttributes.md

+
+Declares an extension that defines an implementation for the Objective-C
+category `CategoryName` on the class in question, or for the main `@interface`
+if the argument list is omitted.


I know this is just internal documentation, but I think I would phrase this as:

## `@_objcImplementation` and `@_objcImplementation(CategoryName)` Must be written on an `extension` of a non-generic imported Objective-C class. When written without an argument list, emits an Objective-C class implementation for the class. When written with a category name argument, emits an Objective-C category implementation for that category of the class, which must be declared in the imported headers. In both cases, the emitted implementation will look exactly like what an Objective-C compiler would have emitted for a corresponding `@implementation`.

Some part of me really hates that the first use case is done with an extension.
I don't know that I have a better option, though. Having two ClassDecls, or
gutting one for the other, both seem worse.

rjmccall · 2022-10-25T04:04:29Z

include/swift/AST/Attr.h

+
+  bool isCategoryNameInvalid() const {
+    return Bits.ObjCImplementationAttr.isCategoryNameInvalid;
+  }


This means that a category name was given but doesn't match anything from the class?

rjmccall · 2022-10-25T04:05:50Z

include/swift/AST/Decl.h

+  ///
+  /// If \c this (an otherwise nonsensical value), the value has not yet been
+  /// computed.
+  Decl *CachedObjCImplementationDecl;


I don't think we want to pay for this on literally every declaration we make in the compiler. I doubt this is even hot. Can you use out-of-line storage for it?

rjmccall · 2022-10-25T04:11:47Z

include/swift/IRGen/Linking.h

@@ -1553,6 +1556,16 @@ class LinkEntity {
  bool isAlwaysSharedLinkage() const;
 #undef LINKENTITY_GET_FIELD
 #undef LINKENTITY_SET_FIELD
+
+private:
+  static bool isObjCImplementation(NominalTypeDecl *NTD) {


hasObjCImplementation, maybe? But really I think this assertion can be broader — we should never have a type metadata or descriptor symbol for any imported class. Unless we emit a lazy candidate with the normal symbol name, but I don't think we do? And if we do, it would trigger for your case, too.

rjmccall · 2022-10-25T04:14:51Z

lib/AST/AccessRequests.cpp

@@ -84,7 +84,14 @@ AccessLevelRequest::evaluate(Evaluator &evaluator, ValueDecl *D) const {
    if (D->hasInterfaceType() && D->isInvalid()) {
      return AccessLevel::Private;
    } else {
-      auto container = cast<NominalTypeDecl>(D->getDeclContext());
+      auto container = dyn_cast<NominalTypeDecl>(DC);
+      if (D->getKind() == DeclKind::Destructor && !container) {


isa<DestructorDecl>(D)?

rjmccall · 2022-10-25T04:18:03Z

lib/AST/Decl.cpp

+/// \li It's \c \@objc
+/// \li Its access level is not \c private or \c fileprivate
+static bool
+isObjCMemberImplementation(const ValueDecl *VD,


I don't love this name, or some of the similar ones. "Implementation" is not a word that ambiguously refers to an ObjC @implementation declaration, and there are a lot of places where ObjC means e.g. the @objc attribute. Is there a more distinctive term we can use within the compiler for these things?

rjmccall · 2022-10-25T04:22:37Z

lib/SIL/IR/SIL.cpp

@@ -35,7 +35,8 @@ FormalLinkage swift::getDeclLinkage(const ValueDecl *D) {

  // Clang declarations are public and can't be assured of having a
  // unique defining location.
-  if (isa<ClangModuleUnit>(fileContext))
+  if (isa<ClangModuleUnit>(fileContext) &&
+          !D->getObjCImplementationDecl())


I don't think you want to change this. Do you remember why you found it necessary?

beccadax commented Aug 18, 2022

View reviewed changes

test/IRGen/objc_implementation.m Outdated Show resolved Hide resolved

beccadax force-pushed the at-implementation branch 3 times, most recently from 86a8951 to eaf12b4 Compare August 25, 2022 17:44

DougGregor reviewed Aug 26, 2022

View reviewed changes

beccadax force-pushed the at-implementation branch 2 times, most recently from aec03c1 to 451f2e9 Compare September 15, 2022 00:01

beccadax added a commit to beccadax/swift-syntax that referenced this pull request Sep 15, 2022

Add @_objcImplementation attribute

7f36091

Part of swiftlang/swift#60630, but safe to merge before the compiler-side implementation.

beccadax mentioned this pull request Sep 15, 2022

Add @_objcImplementation attribute swiftlang/swift-syntax#798

Merged

beccadax force-pushed the at-implementation branch from 451f2e9 to 97bf476 Compare September 15, 2022 22:38

beccadax force-pushed the at-implementation branch 3 times, most recently from 41770b0 to 76beb1c Compare September 30, 2022 00:39

beccadax mentioned this pull request Sep 30, 2022

[NFC] Add parsing helpers for @attr(<identifier>) #61378

Merged

beccadax added a commit to beccadax/swift-syntax that referenced this pull request Sep 30, 2022

Add @_objcImplementation attribute

a71cb0d

Part of swiftlang/swift#60630, but safe to merge before the compiler-side implementation.

beccadax force-pushed the at-implementation branch 2 times, most recently from c5953fc to cd72652 Compare October 5, 2022 03:23

beccadax force-pushed the at-implementation branch 2 times, most recently from 5557ebd to 6c28360 Compare October 5, 2022 17:39

beccadax added a commit to beccadax/swift-syntax that referenced this pull request Oct 5, 2022

Add @_objcImplementation attribute

2688154

Part of swiftlang/swift#60630, but safe to merge before the compiler-side implementation.

beccadax force-pushed the at-implementation branch from 6c28360 to bc1b030 Compare October 5, 2022 18:58

beccadax changed the title ~~@_objcImplementation~~ Add @_objcImplementation Oct 5, 2022

beccadax requested review from rjmccall and aschwaighofer October 5, 2022 20:34

beccadax marked this pull request as ready for review October 5, 2022 20:37

beccadax requested review from ahoppen and bnbarham as code owners October 5, 2022 20:37

beccadax added 14 commits October 18, 2022 17:19

[NFC] Ensure Identifier::is() usable in debugger

1ee12c5

[NFC] Print build-script timings to stderr

e2f76c9

[NFC] Make a test more debuggable

4682743

Add basic Sema support for @_objcImplementation

f2a0ab7

Does not validate members yet; nor does it emit different metadata.

Make all internal-or-more @_objcImpl members impls

e544c21

Require objcImplementation members to avoid vtable

0678a38

They either must be @objc dynamic (the dynamic is implicit) or final.

Cache @_objcImpl pointers and diagnose dupes

ecf0ee6

Generate IR for @_objcImpl stored properties

6143b83

Only works for trivial types right now because features related to initialization and deinitialization are seriously busted.

[NFC] Test @_objcImpl with new string constants

f61f807

Directly access @_objcImpl stored props sometimes

a36b521

Accessing a stored property from a normal method should send a selector, but accessing it from an initializer or accessor should use the field offset.

Give @_objcImpl classes implicit deinits

ad359fa

Temporarily disable test in simuilators for CI

7f0791e

This is failing in the iOS simulator, but *only* in Swift CI, not locally (even with the same tools). Disable the test ouside of macOS while we investigate the failure.

beccadax force-pushed the at-implementation branch from 5b89f39 to 7f0791e Compare October 19, 2022 17:50

beccadax added a commit to beccadax/swift-syntax that referenced this pull request Oct 19, 2022

Add @_objcImplementation attribute

ae622f9

Part of swiftlang/swift#60630, but safe to merge before the compiler-side implementation.

DougGregor approved these changes Oct 20, 2022

View reviewed changes

beccadax merged commit 82d78a3 into swiftlang:main Oct 21, 2022

beccadax mentioned this pull request Oct 21, 2022

[NFC] Adjust @_objcImpl test to give IRGen flexibility #61674

Merged

rjmccall reviewed Oct 25, 2022

View reviewed changes

beccadax mentioned this pull request Oct 27, 2022

Allow overrides in @_objcImplementation extensions #61782

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add @_objcImplementation #60630

Add @_objcImplementation #60630

beccadax commented Aug 18, 2022 •

edited

Loading

beccadax commented Oct 5, 2022

beccadax commented Oct 5, 2022

beccadax commented Oct 5, 2022

beccadax commented Oct 8, 2022

beccadax commented Oct 11, 2022

beccadax commented Oct 11, 2022

beccadax commented Oct 19, 2022

beccadax commented Oct 19, 2022

beccadax commented Oct 19, 2022

DougGregor left a comment

tbkka commented Oct 22, 2022

tbkka commented Oct 22, 2022

ahoppen commented Oct 22, 2022

rjmccall Oct 25, 2022

rjmccall Oct 25, 2022

rjmccall Oct 25, 2022

rjmccall Oct 25, 2022

rjmccall Oct 25, 2022

rjmccall Oct 25, 2022

rjmccall Oct 25, 2022

Add @_objcImplementation #60630

Add @_objcImplementation #60630

Conversation

beccadax commented Aug 18, 2022 • edited Loading

beccadax commented Oct 5, 2022

beccadax commented Oct 5, 2022

beccadax commented Oct 5, 2022

beccadax commented Oct 8, 2022

beccadax commented Oct 11, 2022

beccadax commented Oct 11, 2022

beccadax commented Oct 19, 2022

beccadax commented Oct 19, 2022

beccadax commented Oct 19, 2022

DougGregor left a comment

Choose a reason for hiding this comment

tbkka commented Oct 22, 2022

tbkka commented Oct 22, 2022

ahoppen commented Oct 22, 2022

rjmccall Oct 25, 2022

Choose a reason for hiding this comment

rjmccall Oct 25, 2022

Choose a reason for hiding this comment

rjmccall Oct 25, 2022

Choose a reason for hiding this comment

rjmccall Oct 25, 2022

Choose a reason for hiding this comment

rjmccall Oct 25, 2022

Choose a reason for hiding this comment

rjmccall Oct 25, 2022

Choose a reason for hiding this comment

rjmccall Oct 25, 2022

Choose a reason for hiding this comment

beccadax commented Aug 18, 2022 •

edited

Loading