Make @objcImpl work with @_cdecl

beccadax · beccadax · commit 8651af432587 · 2023-12-13T11:09:49.000-08:00
No real diagnostics yet, but we’re emitting mostly correct code.
diff --git a/include/swift/AST/Attr.def b/include/swift/AST/Attr.def
@@ -246,7 +246,7 @@ DECL_ATTR(_restatedObjCConformance, RestatedObjCConformance,
   OnProtocol | UserInaccessible | LongAttribute | RejectByParser | NotSerialized | ABIStableToAdd | ABIStableToRemove | APIStableToAdd | APIStableToRemove,
   70)
 DECL_ATTR(_objcImplementation, ObjCImplementation,
-  OnExtension | UserInaccessible | ABIBreakingToAdd | ABIBreakingToRemove | APIBreakingToAdd | APIBreakingToRemove,
+  OnExtension | OnAbstractFunction | UserInaccessible | ABIBreakingToAdd | ABIBreakingToRemove | APIBreakingToAdd | APIBreakingToRemove,
   72)
 DECL_ATTR(_optimize, Optimize,
   OnAbstractFunction | OnSubscript | OnVar | UserInaccessible | ABIStableToAdd | ABIStableToRemove | APIStableToAdd | APIStableToRemove,
diff --git a/include/swift/AST/Decl.h b/include/swift/AST/Decl.h
@@ -941,6 +941,11 @@ class alignas(1 << DeclAlignInBits) Decl : public ASTAllocated<Decl> {
   /// attribute macro expansion.
   DeclAttributes getSemanticAttrs() const;
 
+  /// True if this declaration provides an implementation for an imported
+  /// Objective-C declaration. This implies various restrictions and special
+  /// behaviors for it and, if it's an extension, its members.
+  bool isObjCImplementation() const;
+
   using AuxiliaryDeclCallback = llvm::function_ref<void(Decl *)>;
 
   /// Iterate over the auxiliary declarations for this declaration,
@@ -1836,11 +1841,6 @@ class ExtensionDecl final : public GenericContext, public Decl,
   /// resiliently moved into the original protocol itself.
   bool isEquivalentToExtendedContext() const;
 
-  /// True if this extension provides an implementation for an imported
-  /// Objective-C \c \@interface. This implies various restrictions and special
-  /// behaviors for its members.
-  bool isObjCImplementation() const;
-
   /// Returns the name of the category specified by the \c \@_objcImplementation
   /// attribute, or \c None if the name is invalid. Do not call unless
   /// \c isObjCImplementation() returns \c true.
diff --git a/include/swift/AST/DiagnosticsSema.def b/include/swift/AST/DiagnosticsSema.def
@@ -1743,6 +1743,10 @@ ERROR(attr_objc_implementation_category_not_found,none,
 NOTE(attr_objc_implementation_fixit_remove_category_name,none,
      "remove arguments to implement the main '@interface' for this class",
      ())
+ERROR(attr_objc_implementation_no_category_for_func,none,
+      "%kind0 does not belong to an Objective-C category; remove the category "
+      "name from this attribute",
+      (ValueDecl*))
 ERROR(attr_objc_implementation_no_conformance,none,
       "'@_objcImplementation' extension cannot add conformance to %0; "
       "add this conformance %select{with an ordinary extension|"
diff --git a/lib/AST/Decl.cpp b/lib/AST/Decl.cpp
@@ -1847,7 +1847,7 @@ Type ExtensionDecl::getExtendedType() const {
   return ErrorType::get(ctx);
 }
 
-bool ExtensionDecl::isObjCImplementation() const {
+bool Decl::isObjCImplementation() const {
   return getAttrs().hasAttribute<ObjCImplementationAttr>(/*AllowInvalid=*/true);
 }
 
@@ -4404,7 +4404,8 @@ static bool checkAccess(const DeclContext *useDC, const ValueDecl *VD,
   // a context where we would access its storage directly, forbid access. Name
   // lookups will instead find and use the matching interface decl.
   // FIXME: Passing `true` for `isAccessOnSelf` may cause false positives.
-  if (isObjCMemberImplementation(VD, getAccessLevel) &&
+  if ((VD->isObjCImplementation() ||
+         isObjCMemberImplementation(VD, getAccessLevel)) &&
       VD->getAccessSemanticsFromContext(useDC, /*isAccessOnSelf=*/true)
           != AccessSemantics::DirectToStorage)
     return false;
diff --git a/lib/IRGen/GenDecl.cpp b/lib/IRGen/GenDecl.cpp
@@ -3544,7 +3544,10 @@ llvm::Function *IRGenModule::getAddrOfSILFunction(
       if (hasOrderNumber) {
         auto &fnList = Module.getFunctionList();
         fnList.remove(fn);
-        fnList.insert(llvm::Module::iterator(insertBefore), fn);
+        if (insertBefore)
+          fnList.insert(llvm::Module::iterator(insertBefore), fn);
+        else
+          fnList.push_back(fn);
 
         EmittedFunctionsByOrder.insert(orderNumber, fn);
       }
diff --git a/lib/PrintAsClang/DeclAndTypePrinter.cpp b/lib/PrintAsClang/DeclAndTypePrinter.cpp
@@ -2858,12 +2858,16 @@ static bool hasExposeAttr(const ValueDecl *VD, bool isExtension = false) {
   return false;
 }
 
-/// Skip \c \@objcImplementation \c extension member implementations and
-/// overrides. They are already declared in handwritten headers, and they may
-/// have attributes that aren't allowed in a category.
+/// Skip \c \@objcImplementation functions, \c extension member
+/// implementations, and overrides. They are already declared in handwritten
+/// headers, and they may have attributes that aren't allowed in a category.
 ///
 /// \return true if \p VD should \em not be included in the header.
 static bool excludeForObjCImplementation(const ValueDecl *VD) {
+  // If it's an ObjC implementation (and not an extension, which might have
+  // members that need printing), skip it; it's declared elsewhere.
+  if (VD->isObjCImplementation() && ! isa<ExtensionDecl>(VD))
+    return true;
   // Exclude member implementations; they are declared elsewhere.
   if (VD->isObjCMemberImplementation())
     return true;
diff --git a/lib/Sema/TypeCheckAttr.cpp b/lib/Sema/TypeCheckAttr.cpp
@@ -1517,60 +1517,79 @@ void AttributeChecker::visitNonObjCAttr(NonObjCAttr *attr) {
 
 void AttributeChecker::
 visitObjCImplementationAttr(ObjCImplementationAttr *attr) {
-  auto ED = cast<ExtensionDecl>(D);
-  if (ED->isConstrainedExtension())
-    diagnoseAndRemoveAttr(attr,
-                          diag::attr_objc_implementation_must_be_unconditional);
+  if (auto ED = dyn_cast<ExtensionDecl>(D)) {
+    if (ED->isConstrainedExtension())
+      diagnoseAndRemoveAttr(attr,
+                            diag::attr_objc_implementation_must_be_unconditional);
 
-  auto CD = dyn_cast<ClassDecl>(ED->getExtendedNominal());
-  if (!CD) {
-    diagnoseAndRemoveAttr(attr,
-                          diag::attr_objc_implementation_must_extend_class,
-                          ED->getExtendedNominal());
-    ED->getExtendedNominal()->diagnose(diag::decl_declared_here,
-                                       ED->getExtendedNominal());
-    return;
-  }
+    auto CD = dyn_cast<ClassDecl>(ED->getExtendedNominal());
+    if (!CD) {
+      diagnoseAndRemoveAttr(attr,
+                            diag::attr_objc_implementation_must_extend_class,
+                            ED->getExtendedNominal());
+      ED->getExtendedNominal()->diagnose(diag::decl_declared_here,
+                                         ED->getExtendedNominal());
+      return;
+    }
 
-  if (!CD->hasClangNode()) {
-    diagnoseAndRemoveAttr(attr, diag::attr_objc_implementation_must_be_imported,
-                          CD);
-    CD->diagnose(diag::decl_declared_here, CD);
-    return;
-  }
+    if (!CD->hasClangNode()) {
+      diagnoseAndRemoveAttr(attr, diag::attr_objc_implementation_must_be_imported,
+                            CD);
+      CD->diagnose(diag::decl_declared_here, CD);
+      return;
+    }
 
-  if (!CD->hasSuperclass()) {
-    diagnoseAndRemoveAttr(attr, diag::attr_objc_implementation_must_have_super,
-                          CD);
-    CD->diagnose(diag::decl_declared_here, CD);
-    return;
-  }
+    if (!CD->hasSuperclass()) {
+      diagnoseAndRemoveAttr(attr, diag::attr_objc_implementation_must_have_super,
+                            CD);
+      CD->diagnose(diag::decl_declared_here, CD);
+      return;
+    }
 
-  if (CD->isTypeErasedGenericClass()) {
-    diagnoseAndRemoveAttr(attr, diag::objc_implementation_cannot_have_generics,
-                          CD);
-    CD->diagnose(diag::decl_declared_here, CD);
-  }
+    if (CD->isTypeErasedGenericClass()) {
+      diagnoseAndRemoveAttr(attr, diag::objc_implementation_cannot_have_generics,
+                            CD);
+      CD->diagnose(diag::decl_declared_here, CD);
+    }
 
-  if (!attr->isCategoryNameInvalid() && !ED->getImplementedObjCDecl()) {
-    diagnose(attr->getLocation(),
-             diag::attr_objc_implementation_category_not_found,
-             attr->CategoryName, CD);
-
-    // attr->getRange() covers the attr name and argument list; adjust it to
-    // exclude the first token.
-    auto newStart = Lexer::getLocForEndOfToken(Ctx.SourceMgr,
-                                               attr->getRange().Start);
-    if (attr->getRange().contains(newStart)) {
-      auto argListRange = SourceRange(newStart, attr->getRange().End);
+    if (!attr->isCategoryNameInvalid() && !ED->getImplementedObjCDecl()) {
       diagnose(attr->getLocation(),
-               diag::attr_objc_implementation_fixit_remove_category_name)
-        .fixItRemove(argListRange);
+               diag::attr_objc_implementation_category_not_found,
+               attr->CategoryName, CD);
+
+      // attr->getRange() covers the attr name and argument list; adjust it to
+      // exclude the first token.
+      auto newStart = Lexer::getLocForEndOfToken(Ctx.SourceMgr,
+                                                 attr->getRange().Start);
+      if (attr->getRange().contains(newStart)) {
+        auto argListRange = SourceRange(newStart, attr->getRange().End);
+        diagnose(attr->getLocation(),
+                 diag::attr_objc_implementation_fixit_remove_category_name)
+            .fixItRemove(argListRange);
+      }
+
+      attr->setCategoryNameInvalid();
+
+      return;
     }
+  }
+  else if (auto AFD = dyn_cast<AbstractFunctionDecl>(D)) {
+    if (!attr->CategoryName.empty()) {
+      auto diagnostic =
+          diagnose(attr->getLocation(),
+                   diag::attr_objc_implementation_no_category_for_func, AFD);
 
-    attr->setCategoryNameInvalid();
+      // attr->getRange() covers the attr name and argument list; adjust it to
+      // exclude the first token.
+      auto newStart = Lexer::getLocForEndOfToken(Ctx.SourceMgr,
+                                                 attr->getRange().Start);
+      if (attr->getRange().contains(newStart)) {
+        auto argListRange = SourceRange(newStart, attr->getRange().End);
+        diagnostic.fixItRemove(argListRange);
+      }
 
-    return;
+      attr->setCategoryNameInvalid();
+    }
   }
 }
 
diff --git a/test/IRGen/Inputs/objc_implementation.h b/test/IRGen/Inputs/objc_implementation.h
@@ -24,6 +24,8 @@
 
 @end
 
+extern void implFunc(int param);
+
 
 @interface NoImplClass
 
diff --git a/test/IRGen/objc_implementation.swift b/test/IRGen/objc_implementation.swift
@@ -139,9 +139,13 @@ open class SwiftSubclass: ImplClass {
 // Functions
 //
 
+@_objcImplementation @_cdecl("implFunc")
+public func implFunc(_ param: Int32) {}
+
 public func fn(impl: ImplClass, swiftSub: SwiftSubclass) {
   impl.mainMethod(0)
   swiftSub.mainMethod(1)
+  implFunc(2)
 }
 
 // Swift calling convention -[ImplClass init]
@@ -239,12 +243,18 @@ public func fn(impl: ImplClass, swiftSub: SwiftSubclass) {
 // Swift calling convention SwiftSubclass.deinit (deallocating)
 // CHECK-LABEL: define swiftcc void @"$s19objc_implementation13SwiftSubclassCfD"
 
+// inplFunc(_:)
+// CHECK-LABEL: define void @implFunc
+// FIXME: We'd like this to be internal or hidden, not public.
+// CHECK: define swiftcc void @"$s19objc_implementation8implFuncyys5Int32VF"
+
 // fn(impl:swiftSub:)
 // CHECK-LABEL: define swiftcc void @"$s19objc_implementation2fn4impl8swiftSubySo9ImplClassC_AA13SwiftSubclassCtF"
 // CHECK:   [[SEL_1:%[^ ]+]] = load ptr, ptr @"\01L_selector(mainMethod:)", align 8
 // CHECK:   call void @objc_msgSend(ptr {{.*}}, ptr [[SEL_1]], i32 0)
 // CHECK:   [[SEL_2:%[^ ]+]] = load ptr, ptr @"\01L_selector(mainMethod:)", align 8
 // CHECK:   call void @objc_msgSend(ptr {{.*}}, ptr [[SEL_2]], i32 1)
+// CHECK:   call void @implFunc
 // CHECK:   ret void
 // CHECK: }
 
diff --git a/test/Interpreter/Inputs/objc_implementation.h b/test/Interpreter/Inputs/objc_implementation.h
@@ -14,4 +14,6 @@ NS_ASSUME_NONNULL_BEGIN
 
 @end
 
+extern void implFunc(int param);
+
 NS_ASSUME_NONNULL_END
diff --git a/test/Interpreter/objc_implementation.swift b/test/Interpreter/objc_implementation.swift
@@ -54,6 +54,8 @@ class LastWords {
       print("otherProperty =", swiftSub.otherProperty)
       print("implProperty =", swiftSub.implProperty)
     }
+
+    implFunc(2023 - 34)
   }
 
   @objc func someMethod() -> String { "ImplClass.someMethod()" }
@@ -73,6 +75,10 @@ class SwiftSubclass: ImplClass {
   override func someMethod() -> String { "SwiftSubclass.someMethod()" }
 }
 
+@_objcImplementation @_cdecl("implFunc") public func implFunc(_ param: Int32) {
+  print("implFunc(\(param))")
+}
+
 // `#if swift` to ignore the inactive branch's contents
 #if swift(>=5.0) && TOP_LEVEL_CODE
 ImplClass.runTests()
@@ -90,4 +96,5 @@ ImplClass.runTests()
 // CHECK: otherProperty = 13
 // CHECK: implProperty = 42
 // CHECK: SwiftSubclass It's better to burn out than to fade away.
+// CHECK: implFunc(1989)
 #endif
diff --git a/test/PrintAsObjC/Inputs/custom-modules/objc_implementation/objc_implementation.h b/test/PrintAsObjC/Inputs/custom-modules/objc_implementation/objc_implementation.h
@@ -6,3 +6,4 @@
 
 @end
 
+void CImplFunc(void);
diff --git a/test/PrintAsObjC/objc_implementation.swift b/test/PrintAsObjC/objc_implementation.swift
@@ -35,3 +35,6 @@ extension ObjCClass {
   // NEGATIVE-NOT: )privateMethod{{ }}
   @objc private func privateMethod() -> Any? { nil }
 }
+
+@_cdecl("CImplFunc") @_objcImplementation func CImplFunc() {}
+// NEGATIVE-NOT: CImplFunc(
diff --git a/test/decl/ext/Inputs/objc_implementation.h b/test/decl/ext/Inputs/objc_implementation.h
@@ -176,7 +176,8 @@
 
 @end
 
-
+void CImplFunc1(int param);
+void CImplFunc2(int param);
 
 struct ObjCStruct {
   int foo;
diff --git a/test/decl/ext/objc_implementation.swift b/test/decl/ext/objc_implementation.swift
@@ -464,6 +464,21 @@ protocol EmptySwiftProto {}
   // expected-error@-1 {{'@_objcImplementation' cannot be used to implement generic class 'ObjCImplGenericClass'}}
 }
 
+@_objcImplementation @_cdecl("CImplFunc1")
+func CImplFunc1(_: Int32) {
+  // OK
+}
+
+@_objcImplementation(BadCategory) @_cdecl("CImplFunc2")
+func CImplFunc2(_: Int32) {
+  // expected-error@-2 {{global function 'CImplFunc2' does not belong to an Objective-C category; remove the category name from this attribute}} {{21-34=}}
+}
+
+@_objcImplementation @_cdecl("CImplFuncMissing")
+func CImplFuncMissing(_: Int32) {
+  // FIXME: expected-DISABLED-error@-1 {{fnord}}
+}
+
 func usesAreNotAmbiguous(obj: ObjCClass) {
   obj.method(fromHeader1: 1)
   obj.method(fromHeader2: 2)
