Skip to content

[cxx-interop] Add support for operator! overloading #41434

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

Merged
merged 1 commit into from
Mar 14, 2022

Conversation

bulbazord
Copy link
Contributor

This patch enables operator overloading for operator! when cxx interop is enabled. This is my first patch related to cxx interop, let me know if I should be doing something differently.

@plotfi @zoecarver @hyp

@plotfi
Copy link
Contributor

plotfi commented Feb 17, 2022

Nice work Alex. :-)

@bulbazord
Copy link
Contributor Author

@swift-ci please test

Copy link
Contributor

@zoecarver zoecarver left a comment

Choose a reason for hiding this comment

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

Seems good. Thanks.

@zoecarver
Copy link
Contributor

Actually, can you please also update the tests in test/Interop/Cxx/operators/member-inline-typechecker.swift and test/Interop/Cxx/operators/member-inline-module-interface.swift?

@bulbazord bulbazord force-pushed the cxx-interop-operator-bang branch from 9149b78 to d09de50 Compare February 18, 2022 21:21
@bulbazord
Copy link
Contributor Author

Okay, I updated the PR. Here's the changelog:

  • Removed unneeded operator overload from LoadableBoolWrapper
  • Modified the signature of LoadableBoolWrapper::operator! to return a LoadableBoolWrapper. This makes it more consistent with the other tests.
  • Added tests to member-inline-typechecker.swift and member-inline-module-interface.swift as requested. I also updated member-inline-silgen.test to keep it consistent.

Let me know if there's anything else you'd like to see changed.

@zoecarver
Copy link
Contributor

This looks great. Thanks again!

@zoecarver
Copy link
Contributor

I'll start the bots once you fix the Windows mangling. If you want me to run the bots so you can see the failure (it will print out the correct mangling) let me know and I can do that.

@bulbazord
Copy link
Contributor Author

Yes, if you could please trigger the bots to show me the Windows mangling that would be great! I don't have access to a windows machine currently.

@bulbazord
Copy link
Contributor Author

@swift-ci please test

@bulbazord
Copy link
Contributor Author

@zoecarver Could you trigger the test bots so I can get an idea of what the windows manglings will be?

@drodriguez
Copy link
Contributor

@swift-ci please smoke test

@bulbazord bulbazord force-pushed the cxx-interop-operator-bang branch from d09de50 to 365f75d Compare March 9, 2022 21:32
@bulbazord
Copy link
Contributor Author

Ok, got the test fixed up for windows. @drodriguez Can you trigger the tests again for me please? Thanks!

@drodriguez
Copy link
Contributor

@swift-ci please smoke test

@bulbazord
Copy link
Contributor Author

@zoecarver Think this is good to go in now? Or should we wait for the Linux Platform to be unborked?

@drodriguez
Copy link
Contributor

@swift-ci please test Linux platform

@drodriguez drodriguez merged commit 78a410c into swiftlang:main Mar 14, 2022
@bulbazord bulbazord deleted the cxx-interop-operator-bang branch March 14, 2022 17:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants