Skip to content

[cxx-interop] Add a new mode that no longer renames unsafe functions - #91384

Open
Xazax-hun wants to merge 1 commit into
swiftlang:mainfrom
Xazax-hun:import-unsafe-as-always-unsafe
Open

[cxx-interop] Add a new mode that no longer renames unsafe functions#91384
Xazax-hun wants to merge 1 commit into
swiftlang:mainfrom
Xazax-hun:import-unsafe-as-always-unsafe

Conversation

@Xazax-hun

Copy link
Copy Markdown
Contributor

With the introduction of @unsafe(always) we have a new tool to handle functions that are very hard to use correctly. This PR introduces an experimental language feature. When this feature is on we will no longer rename methods to be __methodUnsafe but import them as @unsafe(always). This PR does not change any of the heuristics for renaming/adding @unsafe(always) just yet.

This PR will import the functions over both spellings and add an unavailable note to the renamed one to guide users to use the original name.

There are a couple of caveats/corner cases:

  • The renamed declarations might still be referenced from the overlays. For this reason those declarations are not marked unavaliable/deprecated.
  • We did not clone the unsafe attributes for inherited functions.
  • Added some lookup and cloning logic to properly handle alternate declarations.
  • If a C++ function is already deprecated, the deprecation message from C++ wins.
  • Had to avoid some duplicate work for default arguments in alternate declarations.

With the introduction of @unsafe(always) we have a new tool to handle
functions that are very hard to use correctly. This PR introduces an
experimental language feature. When this feature is on we will no longer
rename methods to be __methodUnsafe but import them as @unsafe(always).
This PR does not change any of the heuristics for renaming/adding
@unsafe(always) just yet.

This PR will import the functions over both spellings and add an
unavailable note to the renamed one to guide users to use the original
name.

There are a couple of caveats/corner cases:
* The renamed declarations might still be referenced from the overlays.
  For this reason those declarations are not marked
  unavaliable/deprecated.
* We did not clone the unsafe attributes for inherited functions.
* Added some lookup and cloning logic to properly handle alternate
  declarations.
* If a C++ function is already deprecated, the deprecation message from
  C++ wins.
* Had to avoid some duplicate work for default arguments in alternate
  declarations.
@Xazax-hun

Copy link
Copy Markdown
Contributor Author

@swift-ci please smoke test

@egorzhdan egorzhdan left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM, thanks!

// RUN: %target-swift-emit-silgen %s -I %S/Inputs -cxx-interoperability-mode=default -enable-experimental-feature ImportUnsafeCxxMethodsAsAlwaysUnsafe | %FileCheck %s

// REQUIRES: swift_feature_ImportUnsafeCxxMethodsAsAlwaysUnsafe

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Minor: existing tests for default arguments are under test/Interop/Cxx/function, should we move this test there too for consistency?

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.

2 participants