[interop][SwiftToC++] print C++ interface for top-level Swift functions #40995

hyp · 2022-01-25T02:32:19Z

Emit C++ function thunks that correspond to Swift top-level functions, alongside their extern "C" function signatures, which are places in the _impl namespace in the module's namespace.

hyp · 2022-01-25T02:32:26Z

@swift-ci please test

swift-ci · 2022-01-25T07:03:07Z

Build failed
Swift Test OS X Platform
Git Sha - db66120a7a7968a2e9f79b0f62c2411c43f606d1

zoecarver · 2022-01-25T17:11:03Z

lib/PrintAsClang/DeclAndTypePrinter.cpp

@@ -2012,7 +2147,8 @@ class DeclAndTypePrinter::Implementation
 public:
  void print(Type ty, Optional<OptionalTypeKind> optionalKind,
             Identifier name = Identifier(),
-             IsFunctionParam_t isFuncParam = IsNotFunctionParam) {
+             IsFunctionParam_t isFuncParam = IsNotFunctionParam,
+             size_t *nameIndex = nullptr) {


This doesn't make sense as an API. Please use optional, or maybe a signed type. But optional is probably best.

I was also thinking, maybe we could compute this in the caller and just pass it as name.

Another thought: this requires a bit of refactoring but name doesn't need to be an Identifier which might help. We could refactor it to be a std::string and in the caller just use std::to_string.

zoecarver · 2022-01-25T17:11:45Z

lib/PrintAsClang/DeclAndTypePrinter.cpp

+            std::tie(objTy, kind) =
+                getObjectTypeAndOptionality(param, param->getInterfaceType());
+            print(objTy, kind, param->getName(), IsFunctionParam,
+                  printEmptyParamNames ? &index : nullptr);


See below comment.

zoecarver · 2022-01-25T17:12:48Z

lib/PrintAsClang/DeclAndTypePrinter.cpp

+    os << "inline ";
+    printFunctionDeclAsCFunctionDecl(FD,
+                                     FD->getName().getBaseIdentifier().get(),
+                                     resultTy, /*printEmptyParamNames=*/true);


Why only print empty param names some places? Seems like that should maybe be a global config. Then you also wouldn't have to launder an int down to print.

we need to print out empty parameters for the inline thunks specifically as we need to use them in the body.

zoecarver · 2022-01-25T17:14:32Z

lib/PrintAsClang/CxxSynthesis.h

+
+namespace swift {
+
+namespace cxx_synthesis {


I don't think there's anything wrong with this, but usually, there'd be some object which we put all these functions in. Maybe Implementation?

Sure, I can pull it into an object class.

zoecarver · 2022-01-25T17:20:00Z

lib/PrintAsClang/DeclAndTypePrinter.cpp

+      }
+    }
+
+    StringRef getSymbolName() const {


This is returning a reference to a string owned by the object, which is a bug waiting to happen. I wonder if it wouldn't be better to allocate mangledName in the AST context and just hold one StringRef.

zoecarver · 2022-01-25T17:22:51Z

lib/PrintAsClang/DeclAndTypePrinter.cpp

+  }
+
+  void printAbstractFunctionAsCxxFunctionThunk(
+      FuncDecl *FD, const FuncionSwiftABIInformation &funcABI) {


Suggested change

FuncDecl *FD, const FuncionSwiftABIInformation &funcABI) {

FuncDecl *FD, const FuncionSwiftABIInformation &funcABI) {

assert(outputLang == OutputLanguageMode::Cxx);

zoecarver · 2022-01-25T17:24:14Z

lib/PrintAsClang/DeclAndTypePrinter.cpp

+  };
+
+  FuncionSwiftABIInformation
+  printSwiftABIFunctionSignatureAsCxxFunction(FuncDecl *FD) {


Maybe add a comment above this and printAbstractFunctionAsCxxFunctionThunk to explain roughly what they do and why you would use one vs the other.

zoecarver · 2022-01-25T17:25:11Z

lib/PrintAsClang/DeclAndTypePrinter.cpp

+    auto params = FD->getParameters();
+    if (params->size()) {
+      size_t index = 1;
+      interleave(


Nit: interleaveComma

zoecarver · 2022-01-25T17:30:23Z

test/Interop/SwiftToCxx/functions/cdecl.swift

+// CHECK: extern "C" int cfuncPassTwo(int x, int y) noexcept;
+// CHECK: }
+
+@_cdecl("cfuncPassTwo")


It's kind of interesting that this cdecl name is different from the declared fn, so maybe the name should be differentCDeclName or something like that. What do you think?

(Or maybe the test name?)

Yeah, that makes sense to me.

zoecarver · 2022-01-25T17:30:40Z

test/Interop/SwiftToCxx/functions/cdecl.swift

@@ -0,0 +1,19 @@
+// RUN: %empty-directory(%t)
+// RUN: %target-swift-frontend(mock-sdk: %clang-importer-sdk) %s -typecheck -module-name CdeclFunctions -emit-cxx-header-path %t/empty.h


Do we need the mock sdk here?

Nope, removed.

zoecarver · 2022-01-25T17:31:03Z

test/Interop/SwiftToCxx/functions/cdecl.swift

+// RUN: %check-cxx-header-in-clang -std=c++14 %t/empty.h
+// RUN: %check-cxx-header-in-clang -std=c++17 %t/empty.h


We can't automate this to run 5-10 configs?

Good idea. I can do that as a follow-up.

Added support here: #41794

zoecarver

Overall, looks really good. Excited to see the progress! I may have a couple more comments, but generally it looks good.

zoecarver · 2022-01-25T17:54:56Z

@LebJe, @hooray I noticed both of you have been putting emojis on the interop patches. If you're just watching from the side lines, that's totally fine (and thanks for the encouragement). But if you want to get involved in the project, please reach out to me (either on the forums or my email) and I can help you get started :)

plotfi · 2022-02-02T08:19:19Z

@zoecarver @hyp is this a better alternative to @_cdecl?

zoecarver · 2022-02-02T17:24:53Z

@zoecarver @hyp is this a better alternative to @_cdecl?

I mean, "better" is really subjective. I think we may actually not take this path of "export everything," and instead require users to mark the functions they want exported. Maybe this can be one of the first working group discussions :)

hyp · 2022-03-09T22:54:40Z

@zoecarver please take another look. I will need to make some changes in a follow-up PR related to unified header, but I would like to land this one first.

hyp · 2022-03-09T22:54:54Z

@swift-ci please test

zoecarver · 2022-03-10T00:18:30Z

So this is still exporting everything?

hyp · 2022-03-10T03:11:59Z

everything

For now yes, I will need to resolve that in the future follow-up PRs

hyp · 2022-03-10T03:12:55Z

@swift-ci please test linux platform

… on windows

hyp · 2022-03-10T04:45:08Z

@swift-ci please test windows platform

hyp · 2022-03-10T05:17:25Z

@swift-ci please test linux platform

…correctly

hyp · 2022-03-10T16:06:28Z

@swift-ci please test

… with /MD msvc mode

hyp · 2022-03-10T17:27:33Z

@swift-ci please test

hyp · 2022-03-10T18:19:43Z

@swift-ci please test

hyp · 2022-03-10T18:24:59Z

@swift-ci please test linux platform

hyp · 2022-03-10T19:04:26Z

@swift-ci please test linux platform

hyp · 2022-03-10T19:30:15Z

@swift-ci please test linux platform

hyp · 2022-03-10T21:06:32Z

@swift-ci please smoke test

hyp · 2022-03-10T22:07:24Z

@swift-ci please smoke test

hyp · 2022-03-10T23:32:42Z

@swift-ci please smoke test linux platform

hyp added the c++ interop Feature: Interoperability with C++ label Jan 25, 2022

hyp requested review from egorzhdan, guitard0g and zoecarver January 25, 2022 02:32

zoecarver reviewed Jan 25, 2022

View reviewed changes

[interop][SwiftToC++] print C++ interface for top-level Swift functions

fcdcb2a

hyp force-pushed the function-to-cxx-thunk branch from db66120 to fcdcb2a Compare March 9, 2022 22:54

[interop][SwiftToC++] fix execution tests to hopefully link correctly…

1d3b682

… on windows

[interop] add target-build-swift-link-cxx option for windows to link …

846fdb5

…correctly

[interop] add target-interop-clangxx test expansion to build C++ file…

07a1f56

… with /MD msvc mode

[interop] fixup the interop build test substitutions

ebcf2f7

hyp merged commit 5fb34b4 into swiftlang:main Mar 11, 2022

	FuncDecl *FD, const FuncionSwiftABIInformation &funcABI) {
	FuncDecl *FD, const FuncionSwiftABIInformation &funcABI) {
	assert(outputLang == OutputLanguageMode::Cxx);

		@@ -0,0 +1,19 @@
		// RUN: %empty-directory(%t)
		// RUN: %target-swift-frontend(mock-sdk: %clang-importer-sdk) %s -typecheck -module-name CdeclFunctions -emit-cxx-header-path %t/empty.h

		// RUN: %check-cxx-header-in-clang -std=c++14 %t/empty.h
		// RUN: %check-cxx-header-in-clang -std=c++17 %t/empty.h

[interop][SwiftToC++] print C++ interface for top-level Swift functions #40995

[interop][SwiftToC++] print C++ interface for top-level Swift functions #40995

Uh oh!

Conversation

hyp commented Jan 25, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

hyp commented Jan 25, 2022

Uh oh!

swift-ci commented Jan 25, 2022

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

hyp Mar 8, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

zoecarver Jan 25, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

zoecarver left a comment

Choose a reason for hiding this comment

Uh oh!

zoecarver commented Jan 25, 2022

Uh oh!

plotfi commented Feb 2, 2022

Uh oh!

zoecarver commented Feb 2, 2022

Uh oh!

hyp commented Mar 9, 2022

Uh oh!

hyp commented Mar 9, 2022

Uh oh!

zoecarver commented Mar 10, 2022

Uh oh!

hyp commented Mar 10, 2022

Uh oh!

hyp commented Mar 10, 2022

Uh oh!

hyp commented Mar 10, 2022

Uh oh!

hyp commented Mar 10, 2022

Uh oh!

hyp commented Mar 10, 2022

Uh oh!

hyp commented Jan 25, 2022 •

edited

Loading

hyp Mar 8, 2022 •

edited

Loading

zoecarver Jan 25, 2022 •

edited

Loading