Skip to content

[interop][SwiftToCxx] trap on moves of Swift values for now #61224

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.

Sign up for GitHub

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
Sep 21, 2022
Merged

[interop][SwiftToCxx] trap on moves of Swift values for now #61224

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions lib/PrintAsClang/PrintClangValueType.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -263,12 +263,12 @@ void ClangValueTypePrinter::printValueTypeDecl(
"*>(other._getOpaquePointer()), metadata._0);\n";
os << " }\n";

// FIXME: the move constructor should be hidden somehow.
os << " inline ";
// FIXME: implement the move constructor.
os << " [[noreturn]] inline ";
printer.printBaseName(typeDecl);
os << "(";
printer.printBaseName(typeDecl);
os << " &&) = default;\n";
os << " &&) { abort(); }\n";
Copy link
Contributor

Choose a reason for hiding this comment

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

Could we emit = delete instead? That might be more helpful for the user since accidentally using the move constructor would be a compile error instead of a runtime error.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I can't emit delete, as C++ will not compile the elided moves that are not actually runtime moves, that I need to return such type from the thunk wrapper .

Copy link
Contributor Author

Choose a reason for hiding this comment

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

inline SwiftType thunk() {
  auto result = SwiftType::make();
  $swiftFunction(&result);
  return result; // <-- this will not compile if the move constructor is deleted, but it's elided according to C++ rules so it does not trap.
}

Copy link
Contributor

Choose a reason for hiding this comment

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

I see, makes sense, thanks.


bodyPrinter();
if (typeDecl->isStdlibDecl())
Expand Down
7 changes: 5 additions & 2 deletions stdlib/public/SwiftShims/swift/shims/_SwiftCxxInteroperability.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -103,8 +103,11 @@ class RefCountedClass {
_opaquePointer = other._opaquePointer;
return *this;
}
// FIXME: What to do in 'move'?
inline RefCountedClass(RefCountedClass &&) noexcept = default;
#pragma clang diagnostic push
#pragma clang diagnostic ignored "-Wmissing-noreturn"
// FIXME: implement 'move'?
inline RefCountedClass(RefCountedClass &&) noexcept { abort(); }
#pragma clang diagnostic pop

protected:
inline RefCountedClass(void *_Nonnull ptr) noexcept : _opaquePointer(ptr) {}
Expand Down
2 changes: 1 addition & 1 deletion test/Interop/SwiftToCxx/expose-attr/expose-swift-decls-to-cxx.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ public final class ExposedClass {
// CHECK: class ExposedClass final
// CHECK: class ExposedStruct final {
// CHECK: class ExposedStruct2 final {
// CHECK: ExposedStruct2(ExposedStruct2 &&) = default;
// CHECK: ExposedStruct2(ExposedStruct2 &&)
// CHECK-NEXT: swift::Int getY() const;
// CHECK-NEXT: void setY(swift::Int value);
// CHECK-NEXT: static inline ExposedStruct2 init();
Expand Down
2 changes: 1 addition & 1 deletion test/Interop/SwiftToCxx/initializers/init-in-cxx.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ public struct FirstSmallStruct {

// CHECK: class FirstSmallStruct final {
// CHECK-NEXT: public:
// CHECK: inline FirstSmallStruct(FirstSmallStruct &&) = default;
// CHECK: inline FirstSmallStruct(FirstSmallStruct &&)
// CHECK-NEXT: inline uint32_t getX() const;
// CHECK-NEXT: static inline FirstSmallStruct init();
// CHECK-NEXT: static inline FirstSmallStruct init(swift::Int x);
Expand Down
2 changes: 1 addition & 1 deletion test/Interop/SwiftToCxx/methods/method-in-cxx.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ public final class PassStructInClassMethod {
// CHECK-NEXT: inline ClassWithMethods deepCopy(swift::Int x);

// CHECK: class LargeStruct final {
// CHECK: inline LargeStruct(LargeStruct &&) = default;
// CHECK: inline LargeStruct(LargeStruct &&)
// CHECK-NEXT: inline LargeStruct doubled() const;
// CHECK-NEXT: inline void dump() const;
// CHECK-NEXT: inline LargeStruct scaled(swift::Int x, swift::Int y) const;
Expand Down
4 changes: 2 additions & 2 deletions test/Interop/SwiftToCxx/methods/mutating-method-in-cxx.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,14 +57,14 @@ public struct SmallStruct {
// CHECK: SWIFT_EXTERN void $s7Methods11SmallStructV6invertyyF(SWIFT_CONTEXT void * _Nonnull _self) SWIFT_NOEXCEPT SWIFT_CALL; // invert()

// CHECK: class LargeStruct final {
// CHECK: inline LargeStruct(LargeStruct &&) = default;
// CHECK: inline LargeStruct(LargeStruct &&)
// CHECK-NEXT: inline void dump() const;
// CHECK-NEXT: inline void double_();
// CHECK-NEXT: inline LargeStruct scale(swift::Int x, swift::Int y);
// CHECK-NEXT: private

// CHECK: class SmallStruct final {
// CHECK: inline SmallStruct(SmallStruct &&) = default;
// CHECK: inline SmallStruct(SmallStruct &&)
// CHECK-NEXT: inline void dump() const;
// CHECK-NEXT: inline SmallStruct scale(float y);
// CHECK-NEXT: inline void invert();
Expand Down
6 changes: 3 additions & 3 deletions test/Interop/SwiftToCxx/properties/getter-in-cxx.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ public struct FirstSmallStruct {

// CHECK: class FirstSmallStruct final {
// CHECK: public:
// CHECK: inline FirstSmallStruct(FirstSmallStruct &&) = default;
// CHECK: inline FirstSmallStruct(FirstSmallStruct &&)
// CHECK-NEXT: inline uint32_t getX() const;
// CHECK-NEXT: private:

Expand All @@ -28,7 +28,7 @@ public struct LargeStruct {

// CHECK: class LargeStruct final {
// CHECK: public:
// CHECK: inline LargeStruct(LargeStruct &&) = default;
// CHECK: inline LargeStruct(LargeStruct &&)
// CHECK-NEXT: inline swift::Int getX1() const;
// CHECK-NEXT: inline swift::Int getX2() const;
// CHECK-NEXT: inline swift::Int getX3() const;
Expand Down Expand Up @@ -82,7 +82,7 @@ public struct SmallStructWithGetters {

// CHECK: class SmallStructWithGetters final {
// CHECK: public:
// CHECK: inline SmallStructWithGetters(SmallStructWithGetters &&) = default;
// CHECK: inline SmallStructWithGetters(SmallStructWithGetters &&)
// CHECK-NEXT: inline uint32_t getStoredInt() const;
// CHECK-NEXT: inline swift::Int getComputedInt() const;
// CHECK-NEXT: inline LargeStruct getLargeStruct() const;
Expand Down
6 changes: 3 additions & 3 deletions test/Interop/SwiftToCxx/properties/setter-in-cxx.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ public struct FirstSmallStruct {

// CHECK: class FirstSmallStruct final {
// CHECK: public:
// CHECK: inline FirstSmallStruct(FirstSmallStruct &&) = default;
// CHECK: inline FirstSmallStruct(FirstSmallStruct &&)
// CHECK-NEXT: inline uint32_t getX() const;
// CHECK-NEXT: inline void setX(uint32_t value);
// CHECK-NEXT: private:
Expand All @@ -21,7 +21,7 @@ public struct LargeStruct {

// CHECK: class LargeStruct final {
// CHECK: public:
// CHECK: inline LargeStruct(LargeStruct &&) = default;
// CHECK: inline LargeStruct(LargeStruct &&)
// CHECK-NEXT: inline swift::Int getX1() const;
// CHECK-NEXT: inline void setX1(swift::Int value);
// CHECK-NEXT: inline swift::Int getX2() const;
Expand Down Expand Up @@ -97,7 +97,7 @@ public struct SmallStructWithProps {

// CHECK: class SmallStructWithProps final {
// CHECK: public:
// CHECK: inline SmallStructWithProps(SmallStructWithProps &&) = default;
// CHECK: inline SmallStructWithProps(SmallStructWithProps &&)
// CHECK-NEXT: inline uint32_t getStoredInt() const;
// CHECK-NEXT: inline void setStoredInt(uint32_t value);
// CHECK-NEXT: inline swift::Int getComputedInt() const;
Expand Down
2 changes: 1 addition & 1 deletion test/Interop/SwiftToCxx/stdlib/swift-stdlib-in-cxx.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@
// CHECK: }
// CHECK-NEXT: inline String(const String &other) {
// CHECK: }
// CHECK-NEXT: inline String(String &&) = default;
// CHECK-NEXT: inline String(String &&) { abort(); }
// CHECK-NEXT: static inline String init();
// CHECK-NEXT: #if defined(__OBJC__)
// CHECK-NEXT: inline __attribute__((always_inline)) operator NSString * _Nonnull () const noexcept {
Expand Down
2 changes: 1 addition & 1 deletion test/Interop/SwiftToCxx/structs/struct-with-refcounted-member.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,5 +47,5 @@ public func printBreak(_ x: Int) {
// CHECK-NEXT: #endif
// CHECK-NEXT: vwTable->initializeWithCopy(_getOpaquePointer(), const_cast<char *>(other._getOpaquePointer()), metadata._0);
// CHECK-NEXT: }
// CHECK-NEXT: inline StructWithRefcountedMember(StructWithRefcountedMember &&) = default;
// CHECK-NEXT: inline StructWithRefcountedMember(StructWithRefcountedMember &&) { abort(); }
// CHECK-NEXT: private:
2 changes: 1 addition & 1 deletion test/Interop/SwiftToCxx/structs/swift-struct-in-cxx.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@
// CHECK: }
// CHECK-NEXT: inline StructWithIntField(const StructWithIntField &other) {
// CHECK: }
// CHECK-NEXT: inline StructWithIntField(StructWithIntField &&) = default;
// CHECK-NEXT: noreturn]] inline StructWithIntField(StructWithIntField &&) { abort(); }
// CHECK-NEXT: private:
// CHECK-NEXT: inline StructWithIntField() {}
// CHECK-NEXT: static inline StructWithIntField _make() { return StructWithIntField(); }
Expand Down