Skip to content

Add a test documenting the current state of C++ exception handling #30674

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 2 commits into from

Conversation

martinboehme
Copy link
Contributor

Swift currently allows C++ exceptions to propagate across Swift stack frames, which is wrong; this test documents that, but also the current behaviors that are fine.

See also the discussion here:
https://forums.swift.org/t/handling-c-exceptions/34823

I'm not sure whether this test will on all platforms, particularly Windows. I may restrict it to certain platforms if build bots show it doesn't work everywhere.

Swift currently allows C++ exceptions to propagate across Swift stack
frames, which is wrong; this test documents that, but also the current
behaviors that are fine.

See also the discussion here:
https://forums.swift.org/t/handling-c-exceptions/34823

I'm not sure whether this test will on all platforms, particularly
Windows. I may restrict it to certain platforms if build bots show it
doesn't work everywhere.
@gribozavr
Copy link
Contributor

@swift-ci Please smoke test

@gribozavr
Copy link
Contributor

@swift-ci Please test Windows

1 similar comment
@gribozavr
Copy link
Contributor

@swift-ci Please test Windows

@gribozavr
Copy link
Contributor

@swift-ci Please smoke test Windows

@martinboehme
Copy link
Contributor Author

I was hoping that linking libstdc++ explicitly in the module map would make this test work at least on macOS as well as Linux, but that doesn't seem to be the case.

So I think I'll have to do the right fix first, which is to make Swift link against the correct library for each platform when -enable-cxx-interop is specified.

Until that's done, I'm not looking for further review on this PR. What is the right way to do this -- just leave this PR open with this comment? (If I close it and reopen a new PR, the previous discussion will be lost, which seems undesirable.)

@gribozavr
Copy link
Contributor

We can leave it open while it is blocked. Just make sure to close it should plans change and you decide to take another approach.

@martinboehme
Copy link
Contributor Author

Thanks, will do!

@compnerd
Copy link
Member

compnerd commented Apr 1, 2020

Also note that there is a third C++ library that you need to be aware of: msvcprt.

@compnerd
Copy link
Member

compnerd commented Apr 1, 2020

@swift-ci please test Windows platform

@martinboehme
Copy link
Contributor Author

Also note that there is a third C++ library that you need to be aware of: msvcprt.

Thanks for the heads-up!

As I mention above, I've come to the conclusion that it's really not a sustainable approach to link to all the platform-specific libraries locally in this test. Instead, Swift should generally link against the right C++ libraries when -enable-cxx-interop is enabled:

https://bugs.swift.org/browse/SR-12469

I'm pausing this PR until that bug is fixed.

@shahmishal
Copy link
Member

Please update the base branch to main by Oct 5th otherwise the pull request will be closed automatically.

  • How to change the base branch: (Link)
  • More detail about the branch update: (Link)

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.

4 participants