Skip to content

[cxx-interop] Add fix for corner case where NS_OPTIONS typedef has to be desugared #66696

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

Conversation

plotfi
Copy link
Contributor

@plotfi plotfi commented Jun 16, 2023

This patch is an add-on to #64043. Essentially when encountering NS_OPTIONS enums, in C++-Interop mode if they are not specially handled then they can mangle differently than they do without C++-Interop. This patch adds logic to handle when a typedef and enum have additional clang::ElaboratedType sugar, but otherwise it does the same as the existing 64043 patch.

The test case provided was encountered in a real app build. The problem came from when two modules are each compiled one with and one without C++-Interop. For the test case code provided the mangling of the protocol conformance is not consistent and the code in SILGenLazyConformance.cpp crashes on an invalid conformance with reason "Invalid conformance in type-checked AST".

(cherry picked from commit fe6ccd7)

Release Branch Checklist:

  • Explanation: This patch aims to ensure that names mangled for NS_OPTIONS when C++-Interop is enabled mangle the same way as they do when it is off. It builds on and handles a missing corner case for an existing patch.
  • Scope: C++-Interop mangling consistency
  • Issue: Some NS_OPTIONS types mangled differently between C++-Interop and without #66602
  • Risk: low, only intends to alter mangling for a very specific NF_OPTIONS case and checks for attribute associated with NS_OPTIONS
  • Testing: lit test, checks that mangled name is as expected
  • Reviewer: @hyp @zoecarver @NuriAmari

… be desugared

This patch is an add-on to swiftlang#64043.
Essentially when encountering NS_OPTIONS enums, in C++-Interop mode
if they are not specially handled then they can mangle differently than
they do without C++-Interop. This patch adds logic to handle when a
typedef and enum have additional clang::ElaboratedType sugar, but
otherwise it does the same as the existing 64043 patch.

The test case provided was encountered in a real app build. The problem
came from when two modules are each compiled one with and one without
C++-Interop. For the test case code provided the mangling of the
protocol conformance is not consistent and the code in
SILGenLazyConformance.cpp crashes on an invalid conformance with reason
"Invalid conformance in type-checked AST".

(cherry picked from commit fe6ccd7)
@plotfi plotfi requested a review from a team as a code owner June 16, 2023 03:08
@plotfi plotfi requested review from hyp and zoecarver June 16, 2023 03:08
@plotfi plotfi added the c++ interop Feature: Interoperability with C++ label Jun 16, 2023
@plotfi
Copy link
Contributor Author

plotfi commented Jun 16, 2023

@rjmccall Hi John, would you be able to review this release branch PR? Thanks

@DougGregor
Copy link
Member

@swift-ci please test

@plotfi plotfi merged commit 197dc4b into swiftlang:release/5.9 Jun 29, 2023
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