[Interop][SwiftToCxx] Introduces swift::Expected #61823

Robertorosmaninho · 2022-10-31T19:51:38Z

This PR introduces the swift::Expected class.
The main idea is to have another approach to handling Swift Errors in C++ besides the already implemented throwing/catching exceptions.

lib/PrintAsClang/_SwiftCxxInteroperability.h

hyp · 2022-11-01T22:43:28Z

This needs some testing. You can add a Cpp interop test that just tries to use this class with some values.

hyp · 2022-11-09T19:18:24Z

lib/PrintAsClang/_SwiftCxxInteroperability.h

The destructor should destroy the value or the error.

hyp · 2022-11-09T19:18:42Z

lib/PrintAsClang/_SwiftCxxInteroperability.h

this is noexcept.

hyp · 2022-11-09T19:18:54Z

lib/PrintAsClang/_SwiftCxxInteroperability.h

This should be noexcept.

hyp · 2022-11-09T19:19:09Z

lib/PrintAsClang/_SwiftCxxInteroperability.h

The parameter should probably be const T &val.

hyp · 2022-11-09T19:19:39Z

lib/PrintAsClang/_SwiftCxxInteroperability.h

the move assignment should be deleted as well.

hyp · 2022-11-09T19:20:18Z

lib/PrintAsClang/_SwiftCxxInteroperability.h

Can we check has_value explicitly like in value instead of specifying the precondition in the comments?

if it doesn't have a value can we abort?

hyp · 2022-11-09T19:20:47Z

lib/PrintAsClang/_SwiftCxxInteroperability.h

we don't want to throw exceptions here.

hyp · 2022-11-09T19:22:51Z

lib/PrintAsClang/_SwiftCxxInteroperability.h

The error and the T can be stored in one memory buffer, e.g.

alignas(std::max(alignof(T), sizeof(swift::Error))) char buffer[std::max(sizeof(T), sizeof(swift::Error)];

Cool, I'll try to use it!

hyp · 2022-11-18T18:10:39Z

lib/PrintAsClang/_SwiftCxxInteroperability.h

Please move this to a detail namespace like _impl.

hyp · 2022-11-18T18:10:50Z

@swift-ci please test

hyp · 2022-11-18T23:54:34Z

@swift-ci please test

hyp · 2022-12-09T18:07:01Z

@swift-ci please test

hyp · 2022-12-09T18:07:09Z

@swift-ci please test source compatibility

lib/PrintAsClang/_SwiftStdlibCxxOverlay.h

hyp · 2022-12-09T18:09:45Z

lib/PrintAsClang/PrintClangFunction.cpp

Maybe we can extract the return type printing code into a closure in this function, to avoid repeating this code?

hyp · 2022-12-13T20:22:39Z

@swift-ci please test Windows platform

hyp · 2022-12-13T22:31:19Z

@swift-ci please test

…a type or an Error and fixing minor errors

…d assigning `swift-expected-execution.cpp` as unsupported by Windows.

… exceptions are not available.

…erlay.h and fixing code duplication on PrintClangFunction.cpp

hyp · 2022-12-14T19:42:20Z

@swift-ci please test

hyp · 2022-12-15T15:16:44Z

@swift-ci please test Windows platform

hyp · 2022-12-15T21:48:56Z

@swift-ci please test Windows platform

hyp · 2022-12-15T23:30:15Z

@swift-ci please test Windows platform

…n SwiftToCxxToSwift/hide-swift-module-namespace-in-swift.swift

hyp · 2022-12-16T19:56:46Z

@swift-ci please test Windows platform

hyp · 2022-12-16T20:58:57Z

@swift-ci please test

hyp · 2022-12-16T20:59:06Z

@swift-ci please test source compatibility

hyp added the c++ interop Feature: Interoperability with C++ label Nov 1, 2022

hyp reviewed Nov 1, 2022

View reviewed changes

lib/PrintAsClang/_SwiftCxxInteroperability.h Outdated Show resolved Hide resolved

hyp reviewed Nov 1, 2022

View reviewed changes

lib/PrintAsClang/_SwiftCxxInteroperability.h Outdated Show resolved Hide resolved

Robertorosmaninho force-pushed the interop/ExpectedClassForErrorHandling branch from ee27a92 to bbe669d Compare November 2, 2022 20:49

Robertorosmaninho marked this pull request as ready for review November 5, 2022 21:42

Robertorosmaninho requested review from egorzhdan and zoecarver as code owners November 5, 2022 21:42

Robertorosmaninho requested review from hyp and removed request for egorzhdan and zoecarver November 5, 2022 21:42

hyp reviewed Nov 9, 2022

View reviewed changes

lib/PrintAsClang/_SwiftCxxInteroperability.h Outdated

Copy link

Contributor

hyp Nov 9, 2022

Choose a reason for hiding this comment

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

The destructor should destroy the value or the error.

hyp reviewed Nov 9, 2022

View reviewed changes

lib/PrintAsClang/_SwiftCxxInteroperability.h Outdated

Copy link

Contributor

hyp Nov 9, 2022

Choose a reason for hiding this comment

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

this is noexcept.

Robertorosmaninho reacted with thumbs up emoji

hyp reviewed Nov 9, 2022

View reviewed changes

lib/PrintAsClang/_SwiftCxxInteroperability.h Outdated

Copy link

Contributor

hyp Nov 9, 2022

Choose a reason for hiding this comment

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

This should be noexcept.

Robertorosmaninho reacted with thumbs up emoji

hyp reviewed Nov 9, 2022

View reviewed changes

Robertorosmaninho requested a review from hyp November 18, 2022 02:46

hyp reviewed Nov 18, 2022

View reviewed changes

lib/PrintAsClang/_SwiftCxxInteroperability.h Outdated

Copy link

Contributor

hyp Nov 18, 2022

Choose a reason for hiding this comment

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

Please move this to a detail namespace like _impl.

Robertorosmaninho mentioned this pull request Nov 18, 2022

[Interop] Error trying to dynamically cast a swift::Error to a user-defined Error on Windows #61797

Open

Robertorosmaninho force-pushed the interop/ExpectedClassForErrorHandling branch from 3cc9cd6 to 788edb9 Compare November 18, 2022 23:39

Robertorosmaninho force-pushed the interop/ExpectedClassForErrorHandling branch from 788edb9 to c409935 Compare November 30, 2022 01:39

Robertorosmaninho force-pushed the interop/ExpectedClassForErrorHandling branch 2 times, most recently from 97cf443 to 709a75b Compare December 8, 2022 22:35

Robertorosmaninho requested a review from hyp December 8, 2022 22:58

hyp reviewed Dec 9, 2022

View reviewed changes

lib/PrintAsClang/_SwiftStdlibCxxOverlay.h Outdated Show resolved Hide resolved

hyp reviewed Dec 9, 2022

View reviewed changes

Robertorosmaninho added 11 commits December 14, 2022 12:07

[Interop][SwiftToCxx] Initial implementation

0b217b9

[Interop][SwiftToCxx] Improving std::Expected constructors

c9d15ae

[Interop][SwiftToCxx] Creating test cases for swift::Expected

c42dfea

[Interop][SwiftToCxx] Add new line

3f6351d

[Interop] [SwiftToCxx] Using buffer to create an instance for either …

6d16962

…a type or an Error and fixing minor errors

[Interop][SwiftToCxx] Moving the max function to _impl namespace an…

74450be

…d assigning `swift-expected-execution.cpp` as unsupported by Windows.

[Interop][SwiftToCxx] Moving swift::Expected to _SwiftStdlibCxxOverlay.h

782bc6a

[Interop][SwiftToCxx] Fixing swift-expected-execution.cpp test

a2633d9

[Interop][SwiftToCxx] Adding support to return Swift::Expected when…

ce666ac

… exceptions are not available.

[Interop][SwiftToCxx] Replacing static by inline on _SwiftStdlibCxxOv…

80e7546

…erlay.h and fixing code duplication on PrintClangFunction.cpp

Add <new> into every bridge header

5fc55b6

Robertorosmaninho force-pushed the interop/ExpectedClassForErrorHandling branch from 2f40d11 to 5fc55b6 Compare December 14, 2022 15:59

test a potential modules-related fix for windows test failure

878f162

another speculative fix to windows test failure

2c9e807

[interop][SwiftToCxx] workaround windows failure with Error new use i…

6d0dd0e

…n SwiftToCxxToSwift/hide-swift-module-namespace-in-swift.swift

hyp merged commit 2c83ff7 into swiftlang:main Dec 17, 2022

Robertorosmaninho deleted the interop/ExpectedClassForErrorHandling branch January 3, 2023 22:39

[Interop][SwiftToCxx] Introduces swift::Expected #61823

[Interop][SwiftToCxx] Introduces swift::Expected #61823

Uh oh!

Conversation

Robertorosmaninho commented Oct 31, 2022

Uh oh!

Uh oh!

Uh oh!

hyp commented Nov 1, 2022

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Robertorosmaninho Nov 9, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

hyp commented Nov 18, 2022

Uh oh!

hyp commented Nov 18, 2022

Uh oh!

hyp commented Dec 9, 2022

Uh oh!

hyp commented Dec 9, 2022

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

hyp commented Dec 13, 2022

Uh oh!

hyp commented Dec 13, 2022

Uh oh!

hyp commented Dec 14, 2022

Uh oh!

hyp commented Dec 15, 2022

Uh oh!

hyp commented Dec 15, 2022

Uh oh!

hyp commented Dec 15, 2022

Uh oh!

hyp commented Dec 16, 2022

Uh oh!

hyp commented Dec 16, 2022

Uh oh!

hyp commented Dec 16, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Robertorosmaninho Nov 9, 2022 •

edited

Loading