unittests/runtime: Add more stubs related to protocol conformance #14787
Conversation
There was a problem hiding this comment.
This doesn't make much sense, these are type definitions, not actual variables. There is no associated storage for these. Instances of anonymous structures with those names would be fine, though that is an a MSVC/GNU extension. Just create a tag type for them to be portable.
unittests/runtime/Stdlib.cpp
Outdated
| @@ -29,6 +29,9 @@ void _swift_makeAnyHashableUsingDefaultRepresentation( | |||
| abort(); | |||
| } | |||
|
|
|||
| SWIFT_CC(swift) SWIFT_RUNTIME_STDLIB_INTERNAL | |||
There was a problem hiding this comment.
This shouldn't have a CC associated with it, it is a structure.
|
I see. How does this look now? |
|
They are duplicated with an anonymous and tagged type. |
|
🤦♂️ |
|
Seems plausible |
|
@swift-ci please test |
|
CC: @jckarter |
|
Build failed |
|
Build failed |
unittests/runtime/Stdlib.cpp
Outdated
| SWIFT_RUNTIME_STDLIB_INTERNAL | ||
| const char $Ss3SetVMn[1] = {0}; | ||
|
|
||
| // Mirror |
There was a problem hiding this comment.
I think that we should go ahead and use a macro here
// stub symbols for the protocol witness table and the type metadata accessor
#define STUB_TYPE_INFO(type) \
SWIFT_RUNTIME_STDLIB_INTERNAL const char $Ss12_##type##VMa[1] = {0}; \
SWIFT_RUNTIME_STDLIB_INTERNAL const char $Ss12_##type##Vs01_B0sWP[1] = {0};
STUB_TYPE_INFO(ClassMirror)
STUB_TYPE_INFO(EnumMirror)
STUB_TYPE_INFO(MetatypeMirror)
STUB_TYPE_INFO(OpaqueMirror)
STUB_TYPE_INFO(StructMirror)
STUB_TYPE_INFO(TupleMirror)
There was a problem hiding this comment.
12 is the identifier length, so it can't be buried in the macro unfortunately. We used to have a macro for this when we were staging in the new mangling, but it looks like it's been taken out since.
unittests/runtime/Stdlib.cpp
Outdated
| // Set | ||
|
|
||
| SWIFT_RUNTIME_STDLIB_INTERNAL | ||
| const char $Ss3SetVMn[1] = {0}; |
There was a problem hiding this comment.
If these are char they may not end up sufficiently aligned to meet the expectations of runtime or stdlib code that assume these structures are 4- or 8-byte aligned. Stubbing them out as long long would be safer.
Found whilst attempting to build the runtime tests on Windows
|
@jckarter |
|
Thanks! This is reasonable in order to get the existing infrastructure working, but maybe we should rework how the runtime unittests get built, so that they can link against only the parts of the runtime they test and not the runtime as a whole. That would make these hacks unnecessary. |
|
Yeah, I was thinking the same thing while implementing this. Maybe we can explore using the headers in the runtime, instead of stubbing out the symbols used. I don't know why the tests are built the way they are though - there probably is a reason - I'm just missing a bunch of context in the area 😛 Can we re-run CI? It looks like there was a spurious failure when @compnerd asked |
|
@swift-ci Please test |
|
I don't think there's any reason for the current build process except for inertia. These symbols come from the standard library written in Swift, so they don't really have a header you can include, but the parts of the runtime that have unit tests should be fairly well decoupled from the runtime as a whole. |
|
Build failed |
|
Build failed |
|
Agreed, I wasn't trying to hold up this patch, just suggesting that further cleanup was possible. |
AndrewSB
deleted the
mangling🤦♂️-stubs-for-protocol-conformances🤷♂️
branch
Found whilst attempting to build the runtime tests on Windows