swift icon indicating copy to clipboard operation
swift copied to clipboard

[cxx-interop] Import default ctor of C++ foreign ref types as Swift Initializer

Open fahadnayyar opened this issue 10 months ago • 4 comments

Building on top of PR #79288, this update synthesizes a static factory method using the default new operator, with a call to the default constructor expression for C++ foreign reference types, and imports them as Swift initializers.

rdar://147529406

fahadnayyar avatar Mar 13 '25 09:03 fahadnayyar

We might want to add the the NoDebug attribute to avoid crashes:

newMethod->addAttr(clang::NoDebugAttr::CreateImplicit(clangCtx));

Similar to what Susana did here: https://github.com/swiftlang/swift/pull/80094

Also, I think we might want to set the generated factory function implicit and implicitly inline.

Xazax-hun avatar Mar 19 '25 11:03 Xazax-hun

Some pointers from an offline discussion with @j-hui :

  • We need to think more about access specifiers of the synthesized factory based on access specifier of operator new and ctor. Checking for everything to be public might be too restrictive. But we can do that in a separate follow up patch if that seems a sizable work.
  • Another thing to explore might be interaction with C++ inheritance: like inherited operator new or inherited ctors. Access specifiers in case of inheritance might also be interesting.
  • We should add -g and -O0 flags to the lit tests.
  • It might be worth splitting the swift lit tests into smaller tests to make triaging of future CI failures easy.

fahadnayyar avatar Mar 26 '25 18:03 fahadnayyar

like inherited operator new

That should be handled by the FindAllocationFunctions method of Sema.

Xazax-hun avatar Mar 26 '25 19:03 Xazax-hun

@swift-ci please smoke test

fahadnayyar avatar Mar 31 '25 05:03 fahadnayyar

@swift-ci please smoke test macos

fahadnayyar avatar Mar 31 '25 15:03 fahadnayyar

@swift-ci please smoke test

fahadnayyar avatar Mar 31 '25 22:03 fahadnayyar

@swift-ci please test macOS

fahadnayyar avatar Mar 31 '25 22:03 fahadnayyar

Enabling auto-merge on this patch with these points in mind:

  • Currently the feature is hidden under -enable-experimental-feature CXXForeignReferenceTypeInitializers. I have added sufficient tests to demonstrate that the feature is stable for most common cases of public default ctor. Before removing this feature flag we need to have these 3 follow up patches (probably in that order):
    • 1.) Extend the support of this patch to allow calling non-default or parameterized ctors of c++ frts as swift initializers. rdar://148285251
    • 2.) Handle cases of private and protected operator new. That patch will also add test cases for scenarios where C++ frt (in various scenarios) are referred by Swift but its initializer is not called explicitly. We should assert that no diagnostics are triggered for cases of only referring to a frt. rdar://148284888
    • 3.) Add support and test for private and protected ctor and operator new in conjunction with @j-hui's recent features about importing private C++ members into Swift. rdar://148285637
  • Stretch goal: Explore how this feature works in conjunction of C++ inheritance. rdar://148285715
  • The removing of feature flag would go through some internal qualifications on adopter projects. rdar://148285972

All comments which are not hidden or marked as resolved could be revisited either for refactoring, stress testing or other improvements later. All resolved/hidden comments have been addressed. rdar://148286196

fahadnayyar avatar Mar 31 '25 23:03 fahadnayyar