[cxx-interop] Fix not importing return type for certain functions #80500
Conversation
Xazax-hun
requested review from
zoecarver,
egorzhdan,
j-hui,
fahadnayyar,
susmonteiro,
hnrklssn,
beccadax and
ian-twilightcoder
as code owners
C++ namespaces are imported as enums. The importer triggered different code path for functions in C++ namespaces and functions in the global scope. As a result, we occasionally did not import the return type of certain C++ functions in namespace scope.
Xazax-hun
force-pushed
the
gaborh/not-imported-return-type
branch
from
a74a18f to
d254e83
Compare
|
@swift-ci please smoke test |
|
@swift-ci please smoke test macOS |
| @@ -23,7 +23,9 @@ InheritanceTestSuite.test("Templated cast to base") { | |||
| let sc: BaseT = cast(s) | |||
| expectFalse(sc.isBase) | |||
| let sx: BaseT = cxxCast(s) // should instantiate I to SubT and O to BaseT | |||
| expectFalse(sc.isBase) | |||
There was a problem hiding this comment.
Was this a typo from a previous patch?
There was a problem hiding this comment.
Yeah, I believe this was a typo previously, sx was unused.
| @@ -3862,7 +3869,8 @@ namespace { | |||
|
|
|||
| bool importFuncWithoutSignature = | |||
| isa<clang::CXXMethodDecl>(decl) && Impl.importSymbolicCXXDecls; | |||
| if (!dc->isModuleScopeContext() && !isa<clang::CXXMethodDecl>(decl)) { | |||
| if (!dc->isModuleScopeContext() && !isClangNamespace(dc) && | |||
| !isa<clang::CXXMethodDecl>(decl)) { | |||
There was a problem hiding this comment.
Can you explain what this if statement is guarding, and why we need to make sure dc is not a namespace here?
There was a problem hiding this comment.
Sure! In general, global C++ functions and C++ methods did not take this branch. This branch does not handle certain C++ constructs like template parameters. I believe this is the code path that was originally used for many scenarios in Obj-C interop. For example, importing a free function as a method (where the self is usually the first argument of the function). So the original condition checked if we are importing something as a member function (!dc->isModuleScopeContext()) but it is not a C++ method. Since C++ namespaces are represented as enums in Swift, a C++ free function that is not a method ended up inadvertently taking this code path changing the import behavior depending on whether the function is in a namespace or in global context. To make sure we trigger the same code path regardless of if the function is in a namespace scope or global scope I added this condition to check if the enum was created from a C++ namespace.
C++ namespaces are imported as enums. The importer triggered different code path for functions in C++ namespaces and functions in the global scope. As a result, we occasionally did not import the return type of certain C++ functions in namespace scope.