[cxx-interop] Fix calling rvalue ref of a trivial type #80568
@swift-ci please smoke test
| 105 | 105 | ||
| 106 | ReferenceTestSuite.test("rvalue reference of trivial type") { | ||
| 107 | setStaticIntRvalueRef(consuming: 2) | ||
| 108 | expectEqual(2, getStaticInt()) |
j-hui94 days agoJust curious, how would this test have failed prior to your change?
Xazax-hun94 days agoIt crashed with bad memory access. We passed 2 directly as if it was the address of the value.
| 103 | 103 | expectEqual(53, ref.pointee) | |
| 104 | 104 | } | |
| 105 | 105 | ||
| 106 | ReferenceTestSuite.test("rvalue reference of trivial type") { |
fahadnayyar94 days agoI was seeing similar crashing in case C++ value types taking rvalue ref inside ctors in these 3 patterns:
// C++
struct cxxValTy {
public:
int val;
cxxValTy() {};
cxxValTy(int x) { val = x; }
};
struct RValRefCtor {
public:
cxxValTy val;
RValRefCtor(cxxValTy &&x) {
val = std::move(x); // <-- 1.) This crashing at runtime
// val = x; // <-- 2.) This is also crashing at runtime
}
// RValRefCtor(cxxValTy &&x) : val(std::move(x)) {} // <-- 3.) This is also crashing at runtime
};
// Swift
let x11 = cxxValTy(5);
let y11 = RValRefCtor(consuming:x11)
expectEqual(y11.val.val, 5)
Should we add test cases for these as well in thie PR to verify that the problem is fixed in these patterns?
Xazax-hun93 days agoI ended up merging this to make sure this is fixed as soon as possible. I think it is always valuable to have more test cases. Since you already have very similar tests in your PR, I think your PR will cover this as well. Or in case you end up not adding a similar test to your foreign reference constructor work let me know and I can open a follow-up.
| 1543 | 1543 | if (importer::isCxxConstReferenceType(clangTy)) |
fahadnayyar94 days agoI was reading rdar://89647503, I was wondering should we plan working on importing const ref param in C++ APIs as :borrowing . Does Swift now support immutable borrowed params?
Xazax-hun93 days ago (edited 93 days ago)I think we have everything in place to always import const references as borrowing. But this could potentially be a big change so we should make this switch when we have the bandwidth to thoroughly test it to make sure it does not break existing code. Making the change itself should be relatively straightforward.
Login to write a write a comment.
The Swift compiler used to generate a direct call to functions taking rvalue references to trivial types even though they expected an indirect calling conventions. This PR changes the calling convention on the Swift side to match C++.
rdar://148585343