-
Notifications
You must be signed in to change notification settings - Fork 10.5k
[android][test] Fix a handful of tests and disable one CxxToSwiftToCxx bridging test #65968
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -51,6 +51,8 @@ | |
// ASM-NOT: .section | ||
// ASM: $s7section3fooyyF: | ||
// ASM-linux-gnu: .section{{.*}}__TEXT,__mysection | ||
// ASM-linux-android: .section{{.*}}__TEXT,__mysection | ||
// ASM-linux-androideabi: .section{{.*}}__TEXT,__mysection | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This looks great. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I have now tested this and made sure it works. |
||
// ASM-NOT: .section | ||
// ASM: $s7section2g0Sivp: | ||
// ASM-NOT: .section | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -9,13 +9,9 @@ import Constructors | |
import TypeClassification | ||
|
||
// ITANIUM_ARM-LABEL: define protected swiftcc void @"$s7MySwift35copyWithUserProvidedCopyConstructorySo03Has{{cdeF0V_ACtACF|efgH0V_ADtADF}}" | ||
// ITANIUM_ARM-SAME: (%TSo30HasUserProvidedCopyConstructorV* {{.*}}[[ARG0:%.*]], %TSo30HasUserProvidedCopyConstructorV* {{.*}}[[ARG1:%.*]], %TSo30HasUserProvidedCopyConstructorV* {{.*}}[[ARG2:%.*]]) | ||
// ITANIUM_ARM: [[ARG0_AS_STRUCT:%.*]] = bitcast %TSo30HasUserProvidedCopyConstructorV* [[ARG0]] to %struct.HasUserProvidedCopyConstructor* | ||
// ITANIUM_ARM: [[ARG2_AS_STRUCT:%.*]] = bitcast %TSo30HasUserProvidedCopyConstructorV* [[ARG2]] to %struct.HasUserProvidedCopyConstructor* | ||
// ITANIUM_ARM: call void @_ZN30HasUserProvidedCopyConstructorC2ERKS_(%struct.HasUserProvidedCopyConstructor* [[ARG0_AS_STRUCT]], %struct.HasUserProvidedCopyConstructor* [[ARG2_AS_STRUCT]]) | ||
// ITANIUM_ARM: [[ARG1_AS_STRUCT:%.*]] = bitcast %TSo30HasUserProvidedCopyConstructorV* [[ARG1]] to %struct.HasUserProvidedCopyConstructor* | ||
// ITANIUM_ARM: [[ARG2_AS_STRUCT:%.*]] = bitcast %TSo30HasUserProvidedCopyConstructorV* [[ARG2]] to %struct.HasUserProvidedCopyConstructor* | ||
// ITANIUM_ARM: call void @_ZN30HasUserProvidedCopyConstructorC2ERKS_(%struct.HasUserProvidedCopyConstructor* [[ARG1_AS_STRUCT]], %struct.HasUserProvidedCopyConstructor* [[ARG2_AS_STRUCT]]) | ||
// ITANIUM_ARM-SAME: (ptr {{.*}}[[ARG0:%.*]], ptr {{.*}}[[ARG1:%.*]], ptr {{.*}}[[ARG2:%.*]]) | ||
// ITANIUM_ARM: call void @_ZN30HasUserProvidedCopyConstructorC2ERKS_(ptr [[ARG0]], ptr [[ARG2]]) | ||
// ITANIUM_ARM: call void @_ZN30HasUserProvidedCopyConstructorC2ERKS_(ptr [[ARG1]], ptr [[ARG2]]) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. These constructor tests started failing recently and had to be updated for Android similarly to #67092. |
||
// ITANIUM_ARM: ret void | ||
|
||
public func copyWithUserProvidedCopyConstructor(_ x: HasUserProvidedCopyConstructor) | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -40,7 +40,7 @@ public: | |
std::vector<SimplePOD * _Nullable> getMutPODPtrItems() const; | ||
}; | ||
|
||
// CHECK: func getPODItems() -> std{{\.__1\.|\.}}vector<SimplePOD, allocator<SimplePOD>> | ||
// CHECK: func getFRTItems() -> std{{\.__1\.|\.}}vector<FRTType, allocator<FRTType>> | ||
// CHECK: func getPODPtrItems() -> std{{\.__1\.|\.}}vector<UnsafePointer<SimplePOD>, allocator<UnsafePointer<SimplePOD>>> | ||
// CHECK: func getMutPODPtrItems() -> std{{\.__1\.|\.}}vector<UnsafeMutablePointer<SimplePOD>, allocator<UnsafeMutablePointer<SimplePOD>>> | ||
// CHECK: func getPODItems() -> std{{\.__(ndk)?1\.|\.}}vector<SimplePOD, allocator<SimplePOD>> | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Otherwise fails on Android with this error:
|
||
// CHECK: func getFRTItems() -> std{{\.__(ndk)?1\.|\.}}vector<FRTType, allocator<FRTType>> | ||
// CHECK: func getPODPtrItems() -> std{{\.__(ndk)?1\.|\.}}vector<UnsafePointer<SimplePOD>, allocator<UnsafePointer<SimplePOD>>> | ||
// CHECK: func getMutPODPtrItems() -> std{{\.__(ndk)?1\.|\.}}vector<UnsafeMutablePointer<SimplePOD>, allocator<UnsafeMutablePointer<SimplePOD>>> |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -5,5 +5,5 @@ import ClassTemplateNonTypeParameter | |
let p = MagicIntPair() | ||
let t = MagicIntTriple() | ||
|
||
// CHECK: @"${{s4main1pSo0034MagicArrayInt32_UInt_2_zoAFhhiEngcVvp|s4main1pSo0036MagicArrayInt32_UInt64_2_JsAEiFiuomcVvp}}" | ||
// CHECK: @"${{s4main1tSo0034MagicArrayInt32_UInt_3_zoAFhhiEngcVvp|s4main1tSo0036MagicArrayInt32_UInt64_3_JsAEiFiuomcVvp}}" | ||
// CHECK: @"${{s4main1pSo0034MagicArrayInt32_UInt_2_zoAFhhiEngcVvp|s4main1pSo0036MagicArrayInt32_UInt(64|32)_2_JsAEiFiuomcVvp}}" | ||
// CHECK: @"${{s4main1tSo0034MagicArrayInt32_UInt_3_zoAFhhiEngcVvp|s4main1tSo0036MagicArrayInt32_UInt(64|32)_3_JsAEiFiuomcVvp}}" | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This new test fails on 32-bit arches like Android armv7, but these two changes got it passing there too. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @egorzhdan, you added this new test recently, please review this change to get it working on 32-bit platforms. |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,7 +1,7 @@ | ||
// UNSUPPORTED: OS=windows-msvc | ||
// static library is not well supported yet on Windows | ||
|
||
// REQUIRES: lld_lto | ||
// XFAIL: OS=linux-android, OS=linux-androideabi | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This started failing recently on the community Android CI, as the LLVM 14 toolchain in the Android LTS NDK 25c doesn't support opaque pointers much:
That flag wasn't added till clang 15, llvm/llvm-project@d69e9f9d89783, so this won't work till the next NDK, which will include clang 17. It currently works natively in the Termux app, where I use LLVM 16. |
||
|
||
// For LTO, the linker dlopen()'s the libLTO library, which is a scenario that | ||
// ASan cannot work in ("Interceptors are not working, AddressSanitizer is | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -13,7 +13,7 @@ | |
|
||
// CHECK: CONFORMANCES: | ||
// CHECK: ============= | ||
// CHECK-DAG: 16ConformanceCheck10SomeStructV6${{[0-9a-f]*}}yXZ0c6NestedD06${{[0-9a-f]*}}LLV (ConformanceCheck.SomeStruct.(unknown context at ${{[0-9a-f]*}}).(SomeNestedStruct in ${{[0-9a-f]*}})) : ConformanceCheck.MyProto | ||
// CHECK-DAG: 16ConformanceCheck10SomeStructV{{[56]}}${{[0-9a-f]*}}yXZ0c6NestedD0{{[56]}}${{[0-9a-f]*}}LLV (ConformanceCheck.SomeStruct.(unknown context at ${{[0-9a-f]*}}).(SomeNestedStruct in ${{[0-9a-f]*}})) : ConformanceCheck.MyProto | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't know why this started failing all of a sudden, as the length of this symbol on Android was 6 and it passed before, but lately it is 5, so this change gets it passing again. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @artemcm, this test is disabled on all other arm64 platforms, is this the reason? Should I just disable it for Android AArch64 too? |
||
// CHECK-DAG: 16ConformanceCheck3fooV3barV3bazV3quxV4quuxV5corgeV6graultV6garplyV5waldoV4fredV5plughV5xyzzyV4thudV18SomeConformingTypeV (ConformanceCheck.foo.bar.baz.qux.quux.corge.grault.garply.waldo.fred.plugh.xyzzy.thud.SomeConformingType) : ConformanceCheck.MyProto | ||
// CHECK-DAG: 16ConformanceCheck7StructAV (ConformanceCheck.StructA) : ConformanceCheck.MyProto, Swift.Hashable, Swift.Equatable | ||
// CHECK-DAG: 16ConformanceCheck2E4O (ConformanceCheck.E4) : ConformanceCheck.P1, ConformanceCheck.P2, ConformanceCheck.P3 | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -501,8 +501,6 @@ swift_backtrace = os.environ.get('SWIFT_BACKTRACE') | |
if swift_backtrace: | ||
config.environment['SWIFT_BACKTRACE'] = swift_backtrace | ||
|
||
config.available_features.add('lld_lto') | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This was used to avoid issues on Android by @kateinoigakukun in #32430, but I enabled it for Android too when the NDK switched to lld, #39921. I left this feature in then because I didn't know if anyone else needed it, but looking into that commit history for the first time now, it should be fine to remove this feature, since it's always enabled now. |
||
|
||
threading = lit_config.params.get('threading', 'none') | ||
config.available_features.add('threading_{}'.format(threading)) | ||
if threading != "none": | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a translation of swiftlang/swift-driver#1372, see that pull for more info.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wonder if we can use
llvm::Triple::getOSName
instead.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just looked into this: clang uses
ToolChain::getOSLibName()
instead, which has this same darwin override. We can't call that without instantiating aclang::ToolChain()
though, and given all the ceremony you added to do so elsewhere, that would end up with more lines.@compnerd, I don't think that clang method is worth invoking, let me know if you disagree.