Skip to content

CxxInterop: Swift does not synthesize CxxConvertibleToContainer iterator operator== in some cases #77607

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

Jump to bottom
Open
ADKaster opened this issue Nov 14, 2024 · 0 comments
Open

CxxInterop: Swift does not synthesize CxxConvertibleToContainer iterator operator== in some cases #77607

ADKaster opened this issue Nov 14, 2024 · 0 comments
Labels
bug A deviation from expected or documented behavior. Also: expected but undesirable behavior. triage needed This issue needs more specific labels

Comments

@ADKaster
Copy link
Contributor

Description

In Ladybird, all iterators for our AK containers inherit from AK::SimpleIterator: https://github.com/LadybirdBrowser/ladybird/blob/3b04c983f14a3838870f202fe38fe081cb0771b4/AK/Iterator.h#L16

When I tried to use myVec.forEach { elem in ... } in my new code, I ran into two issues:

  • In code that was importing both AK and another module that uses AK, the compiler informed me that the forEach method on my container did not exist.

  • In code that only imports AK (a test file), the compiler confirmed that the exact type used in the other module is indeed a CxxConvertibleToContainer type, but failed with a bogus linker error.

This issue contains a repro for the second issue, since I suspect they are linked.

Reproduction

git clone https://github.com/LadybirdBrowser/ladybird.git
// install dependencies from https://github.com/LadybirdBrowser/ladybird/blob/master/Documentation/BuildInstructionsLadybird.md#build-prerequisites
// apply patch below
cmake -DCMAKE_C_COMPILER=clang -DCMAKE_CXX_COMPILER=clang++ -DENABLE_SWIFT=ON --preset default
ninja -C release TestAKBindings
./Build/release/bin/TestAKBindings

Apply this patch:

diff --git a/AK/Vector.h b/AK/Vector.h
index 41ac977fa13..5cbf6071544 100644
--- a/AK/Vector.h
+++ b/AK/Vector.h
@@ -857,6 +857,9 @@ private:
 template<class... Args>
 Vector(Args... args) -> Vector<CommonType<Args...>>;
 
+using U32Vector = Vector<u32>;
+using U32Vector2Capacity = Vector<u32, 2>;
+
 }
 
 #if USING_AK_GLOBALLY
diff --git a/Tests/AK/TestAKBindings.swift b/Tests/AK/TestAKBindings.swift
index 37211c021d4..22c0dc26d4b 100644
--- a/Tests/AK/TestAKBindings.swift
+++ b/Tests/AK/TestAKBindings.swift
@@ -14,6 +14,12 @@ private func isCxxSequenceType<T: ~Copyable>(_ type: borrowing T.Type) -> Bool {
     return CxxSequenceMarker<T>.self is ConformanceMarker.Type
 }
 
+enum CxxConvertibleToCollectionMarker<T: ~Copyable> {}
+extension CxxConvertibleToCollectionMarker: ConformanceMarker where T: CxxConvertibleToCollection {}
+private func isCxxConvertibleToCollectionType<T: ~Copyable>(_ type: borrowing T.Type) -> Bool {
+    return CxxConvertibleToCollectionMarker<T>.self is ConformanceMarker.Type
+}
+
 class StandardError: TextOutputStream {
     func write(_ string: Swift.String) {
         try! FileHandle.standardError.write(contentsOf: Data(string.utf8))
@@ -26,7 +32,7 @@ struct TestAKBindings {
         var standardError = StandardError()
         print("Testing CxxSequence types...", to: &standardError)
 
-        precondition(isCxxSequenceType(AK.StringView.self))
+        //precondition(isCxxSequenceType(AK.StringView.self))
         precondition(isCxxSequenceType(AK.Bytes.self))
         precondition(isCxxSequenceType(AK.ReadonlyBytes.self))
         precondition(isCxxSequenceType(AK.Utf16Data.self))
@@ -35,14 +41,33 @@ struct TestAKBindings {
         precondition(!isCxxSequenceType(AK.String.self))
 
         precondition(!isCxxSequenceType(AK.Error.self))
+        precondition(isCxxSequenceType(AK.U32Vector.self))
+        precondition(isCxxSequenceType(AK.U32Vector2Capacity.self))
 
         print("CxxSequence types pass", to: &standardError)
     }
 
+    static func testCollectionTypesAreBound() {
+        var standardError = StandardError()
+        print ("Testing CxxConvertibleToCollection types...", to: &standardError)
+
+        precondition(isCxxConvertibleToCollectionType(AK.U32Vector.self))
+        precondition(isCxxConvertibleToCollectionType(AK.U32Vector2Capacity.self))
+
+        let u32Vector = AK.U32Vector()
+        u32Vector.forEach { _ in }
+
+        let u32Vector2Capacity = AK.U32Vector2Capacity()
+        u32Vector2Capacity.forEach { _ in }
+
+        print("CxxConvertibleToCollection types pass", to: &standardError)
+    }
+
     static func main() {
         var standardError = StandardError()
         print("Starting test suite...", to: &standardError)
         testSequenceTypesAreBound()
+        testCollectionTypesAreBound()
 
         print("All tests pass", to: &standardError)
     }

Result:

ninja: Entering directory `Build/release/'
[0/2] Re-checking globbed directories...
[1/2] Linking Swift executable bin/TestAKBindings
FAILED: bin/TestAKBindings 
: && /home/andrew/.local/bin/swiftc -j 32 -num-threads 32 -emit-executable -o bin/TestAKBindings -O -g Lagom/Tests/AK/CMakeFiles/TestAKBindings.dir/TestAKBindings.swift.o /home/andrew/.local/share/swiftly/toolchains/main-snapshot-2024-10-30/usr/lib/swift/linux/x86_64/swiftrt.o -use-ld=lld -L /home/andrew/ladybird-org/ladybird-browser/Build/release/lib   -L /home/andrew/.local/share/swiftly/toolchains/main-snapshot-2024-10-30/usr/lib/swift/linux   -L /home/andrew/.local/share/swiftly/toolchains/main-snapshot-2024-10-30/usr/lib/swift/linux/x86_64 -Xlinker -rpath -Xlinker /home/andrew/ladybird-org/ladybird-browser/Build/release/vcpkg_installed/x64-linux/lib:/home/andrew/.local/share/swiftly/toolchains/main-snapshot-2024-10-30/usr/lib/swift/linux:/home/andrew/.local/share/swiftly/toolchains/main-snapshot-2024-10-30/usr/lib/swift/linux/x86_64:/home/andrew/ladybird-org/ladybird-browser/Build/release/lib  lib/liblagom-ak.so.0.0.0 && :
error: link command failed with exit code 1 (use -v to see invocation)
ld.lld: error: undefined symbol: AK::SimpleIterator<AK::Vector<unsigned int, 0ul> const, unsigned int const>::operator==(AK::SimpleIterator<AK::Vector<unsigned int, 0ul> const, unsigned int const> const&) const
>>> referenced by TestAKBindings.swift.o
>>>               Lagom/Tests/AK/CMakeFiles/TestAKBindings.dir/TestAKBindings.swift.o:($sSo2AKO0095SimpleIteratorVectorCUnsignedInt_CUnsignedLong_0_constCUnsignedInt_const_mrDAEawaAJfxaDAjgaCqAaVSQSCSQ2eeoiySbx_xtFZTW)
>>> referenced by <compiler-generated>:0 (/<compiler-generated>:0)
>>>               Lagom/Tests/AK/CMakeFiles/TestAKBindings.dir/TestAKBindings.swift.o:($sSTsE21_copySequenceContents12initializing8IteratorQz_SitSry7ElementQzG_tFSo2AKO0047VectorCUnsignedInt_CUnsignedLong_0_sfBJmlmajtndV_Tgq5)
>>> referenced by <stdin>:0
>>>               Lagom/Tests/AK/CMakeFiles/TestAKBindings.dir/TestAKBindings.swift.o:($s14TestAKBindingsAAV27testCollectionTypesAreBoundyyFZTf4d_n)
>>> referenced 1 more times

ld.lld: error: undefined symbol: AK::SimpleIterator<AK::Vector<unsigned int, 2ul> const, unsigned int const>::operator==(AK::SimpleIterator<AK::Vector<unsigned int, 2ul> const, unsigned int const> const&) const
>>> referenced by TestAKBindings.swift.o
>>>               Lagom/Tests/AK/CMakeFiles/TestAKBindings.dir/TestAKBindings.swift.o:($sSo2AKO0095SimpleIteratorVectorCUnsignedInt_CUnsignedLong_2_constCUnsignedInt_const_mrDAEawaAJfxaDAjgaCqAaVSQSCSQ2eeoiySbx_xtFZTW)
>>> referenced by <compiler-generated>:0 (/<compiler-generated>:0)
>>>               Lagom/Tests/AK/CMakeFiles/TestAKBindings.dir/TestAKBindings.swift.o:($sSTsE21_copySequenceContents12initializing8IteratorQz_SitSry7ElementQzG_tFSo2AKO0047VectorCUnsignedInt_CUnsignedLong_2_sfBJmlmajtndV_Tgq5)
>>> referenced by <stdin>:0
>>>               Lagom/Tests/AK/CMakeFiles/TestAKBindings.dir/TestAKBindings.swift.o:($s14TestAKBindingsAAV27testCollectionTypesAreBoundyyFZTf4d_n)
>>> referenced 1 more times
clang: error: linker command failed with exit code 1 (use -v to see invocation)
ninja: build stopped: subcommand failed.

Expected behavior

The test should compile, link, and run without issue.

Environment

$ swiftc -version
Swift version 6.1-dev (LLVM 9d655fdc6103926, Swift 0d16cbf)
Target: x86_64-unknown-linux-gnu

Distro: Ubuntu 24.04
uname -a: Linux Andrew-Workstation 6.8.0-48-generic #48-Ubuntu SMP PREEMPT_DYNAMIC Fri Sep 27 14:04:52 UTC 2024 x86_64 x86_64 x86_64 GNU/Linux

Additional information

operator== on the iterator type is defined as

    constexpr bool operator==(SimpleIterator const& other) const = default;
    constexpr auto operator<=>(SimpleIterator const& other) const = default;

Removing constexpr has no effect.

Adding a translation unit to force generation of the operator== (see details pane below) adds the operator== as LOCAL HIDDEN symbols. Clang seems to expect each TU to generate these symbols if it needs them.

Patch to liblagom-ak.so to contain the missing symbols 
diff --git a/AK/AK+Swift.cpp b/AK/AK+Swift.cpp
new file mode 100644
index 00000000000..f9599211337
--- /dev/null
+++ b/AK/AK+Swift.cpp
@@ -0,0 +1,32 @@
+/*
+ * Copyright (c) 2024, Andrew Kaster <[email protected]>
+ *
+ * SPDX-License-Identifier: BSD-2-Clause
+ */
+
+#include <AK/Format.h>
+#include <AK/Vector.h>
+
+#pragma clang optimize off
+
+namespace AK {
+
+void __swift_use_vector();
+
+[[gnu::used, clang::optnone]] void __swift_use_vector() {
+    auto x = Vector<u32> {};
+    x.append(1);
+
+    for (auto i : x) {
+        dbgln("{}", i);
+    }
+
+    auto x2 = Vector<u32, 2> {};
+    x2.append(1);
+    x2.append(2);
+    for (auto i : x2) {
+        dbgln("{}", i);
+    }
+}
+
+}
diff --git a/AK/CMakeLists.txt b/AK/CMakeLists.txt
index 3fbdc65b90a..21335e70efa 100644
--- a/AK/CMakeLists.txt
+++ b/AK/CMakeLists.txt
@@ -80,7 +80,7 @@ if (ENABLE_SWIFT)
         "${CMAKE_CURRENT_BINARY_DIR}/Backtrace.h"
         "${CMAKE_CURRENT_BINARY_DIR}/Debug.h"
     )
-    target_sources(AK PRIVATE AK+Swift.swift)
+    target_sources(AK PRIVATE AK+Swift.swift AK+Swift.cpp)
     add_swift_target_properties(AK)
 endif()
 
diff --git a/AK/Vector.h b/AK/Vector.h
index 41ac977fa13..5cbf6071544 100644
--- a/AK/Vector.h
+++ b/AK/Vector.h
@@ -857,6 +857,9 @@ private:
 template<class... Args>
 Vector(Args... args) -> Vector<CommonType<Args...>>;
 
+using U32Vector = Vector<u32>;
+using U32Vector2Capacity = Vector<u32, 2>;
+
 }
 
 #if USING_AK_GLOBALLY
diff --git a/Tests/AK/TestAKBindings.swift b/Tests/AK/TestAKBindings.swift
index 37211c021d4..22c0dc26d4b 100644
--- a/Tests/AK/TestAKBindings.swift
+++ b/Tests/AK/TestAKBindings.swift
@@ -14,6 +14,12 @@ private func isCxxSequenceType<T: ~Copyable>(_ type: borrowing T.Type) -> Bool {
     return CxxSequenceMarker<T>.self is ConformanceMarker.Type
 }
 
+enum CxxConvertibleToCollectionMarker<T: ~Copyable> {}
+extension CxxConvertibleToCollectionMarker: ConformanceMarker where T: CxxConvertibleToCollection {}
+private func isCxxConvertibleToCollectionType<T: ~Copyable>(_ type: borrowing T.Type) -> Bool {
+    return CxxConvertibleToCollectionMarker<T>.self is ConformanceMarker.Type
+}
+
 class StandardError: TextOutputStream {
     func write(_ string: Swift.String) {
         try! FileHandle.standardError.write(contentsOf: Data(string.utf8))
@@ -26,7 +32,7 @@ struct TestAKBindings {
         var standardError = StandardError()
         print("Testing CxxSequence types...", to: &standardError)
 
-        precondition(isCxxSequenceType(AK.StringView.self))
+        //precondition(isCxxSequenceType(AK.StringView.self))
         precondition(isCxxSequenceType(AK.Bytes.self))
         precondition(isCxxSequenceType(AK.ReadonlyBytes.self))
         precondition(isCxxSequenceType(AK.Utf16Data.self))
@@ -35,14 +41,33 @@ struct TestAKBindings {
         precondition(!isCxxSequenceType(AK.String.self))
 
         precondition(!isCxxSequenceType(AK.Error.self))
+        precondition(isCxxSequenceType(AK.U32Vector.self))
+        precondition(isCxxSequenceType(AK.U32Vector2Capacity.self))
 
         print("CxxSequence types pass", to: &standardError)
     }
 
+    static func testCollectionTypesAreBound() {
+        var standardError = StandardError()
+        print ("Testing CxxConvertibleToCollection types...", to: &standardError)
+
+        precondition(isCxxConvertibleToCollectionType(AK.U32Vector.self))
+        precondition(isCxxConvertibleToCollectionType(AK.U32Vector2Capacity.self))
+
+        let u32Vector = AK.U32Vector()
+        u32Vector.forEach { _ in }
+
+        let u32Vector2Capacity = AK.U32Vector2Capacity()
+        u32Vector2Capacity.forEach { _ in }
+
+        print("CxxConvertibleToCollection types pass", to: &standardError)
+    }
+
     static func main() {
         var standardError = StandardError()
         print("Starting test suite...", to: &standardError)
         testSequenceTypesAreBound()
+        testCollectionTypesAreBound()
 
         print("All tests pass", to: &standardError)
     }

The symbols appear like so, with that patch applied:

readelf -Ws Build/release/lib/liblagom-ak.so.0.0.0 | llvm-cxxfilt-18 |  grep "operator==(AK::SimpleIterator"
  1253: 0000000000064c70    21 FUNC    LOCAL  HIDDEN    27 AK::SimpleIterator<AK::Vector<unsigned int, 0ul>, unsigned int>::operator==(AK::SimpleIterator<AK::Vector<unsigned int, 0ul>, unsigned int> const&) const
  1259: 0000000000064d50    21 FUNC    LOCAL  HIDDEN    27 AK::SimpleIterator<AK::Vector<unsigned int, 2ul>, unsigned int>::operator==(AK::SimpleIterator<AK::Vector<unsigned int, 2ul>, unsigned int> const&) const
@ADKaster ADKaster added bug A deviation from expected or documented behavior. Also: expected but undesirable behavior. triage needed This issue needs more specific labels labels Nov 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug A deviation from expected or documented behavior. Also: expected but undesirable behavior. triage needed This issue needs more specific labels
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
1 participant
@ADKaster