Skip to content

Conversation

@hyp
Copy link
Contributor

@hyp hyp commented May 11, 2023

…ent modules to enable C++ interoperability

Fixes #65831

@hyp hyp added the c++ interop Feature: Interoperability with C++ label May 11, 2023
@hyp hyp requested a review from ravikandhadai May 11, 2023 22:17
@hyp hyp requested review from egorzhdan and zoecarver as code owners May 11, 2023 22:17
@hyp
Copy link
Contributor Author

hyp commented May 11, 2023

@swift-ci please test

@hyp
Copy link
Contributor Author

hyp commented May 11, 2023

@swift-ci please test source compatibility

Copy link
Contributor

@egorzhdan egorzhdan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would libcxxstdlibshim.h also require a similar fix?

@hyp
Copy link
Contributor Author

hyp commented May 14, 2023

libcxxstdlibshim

No, the compiler doesn't implicitly add it as an import when C++ interop is enabled.

Copy link
Contributor

@egorzhdan egorzhdan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think there is a less intrusive way to fix this: can we make the implicit import of CxxShim @_implementationOnly?

In Frontend.cpp:

if (Invocation.getLangOptions().EnableCXXInterop && canImportCxxShim()) {
  pushImport(CXX_SHIM_NAME, {ImportFlags::ImplementationOnly});
}

That way IIUC the compiler would also refuse to compile such modules with cross-module optimization enabled instead of crashing.

@hyp
Copy link
Contributor Author

hyp commented May 15, 2023

Yes, I can try that.

@hyp hyp force-pushed the eng/impl-only-letsgoo branch from b30b61a to b17ade9 Compare May 15, 2023 17:11
@hyp
Copy link
Contributor Author

hyp commented May 15, 2023

@swift-ci please test

@hyp
Copy link
Contributor Author

hyp commented May 15, 2023

@swift-ci please test source compatibility

@hyp
Copy link
Contributor Author

hyp commented May 15, 2023

@egorzhdan updated :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

c++ interop Feature: Interoperability with C++

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[cxx-interop] a Swift module that enables interop should not force client Swift module to fail to import CxxShim

2 participants