Skip to content
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

[Debug] Introduce use of DebugDescription macro in stdlib #71425

Merged
merged 8 commits into from
Feb 21, 2024
Merged

[Debug] Introduce use of DebugDescription macro in stdlib #71425

merged 8 commits into from
Feb 21, 2024

Conversation

kastiglione
Copy link
Contributor

@kastiglione kastiglione commented Feb 6, 2024 •
edited
Loading

Introduces the first use of @_DebugDescription in the standard library, applying it to ObjectIdentifier. In order to use the DebugDescription macro in the stdlib, the following changes are required:

  1. Compilation must reference the just built macros (ie libSwiftMacros.dylib), not those from the SDK. This is addressed by adding an explicit -external-plugin-path flag that overrides the defaults generated by the compiler (which uses SDK paths, where the macro may or may not exist, and may not be the current version).
  2. As DebugDescription uses @_section, compilation enables the SymbolLinkageMarkers feature.

Note that the use of DebugDescription is conditionally enabled for the following reasons:

  1. Use is disabled in freestanding builds, where the stdlib macros are not built.
  2. Use is temporarily disabled in Linux builds due to a dynamic loader issue that needs further investigation

The dynamic loader error causing issues with Linux CI is:

swift-plugin-server: error while loading shared libraries: libswiftGlibc.so: cannot open shared object file: No such file or directory

This PR depended on #71639, #71588, and #71685.

@kastiglione kastiglione requested a review from a team as a code owner February 6, 2024 23:20
@kastiglione kastiglione marked this pull request as draft February 6, 2024 23:20
@kastiglione
Copy link
Contributor Author

@swift-ci smoke test

@kastiglione kastiglione changed the title [Debug] Introduce use of DebugDescription in stdlib [Debug] Introduce use of DebugDescription macro in stdlib Feb 6, 2024
@kastiglione
Copy link
Contributor Author

@swift-ci test

3 similar comments
@kastiglione
Copy link
Contributor Author

@swift-ci test

@kastiglione
Copy link
Contributor Author

@swift-ci test

@kastiglione
Copy link
Contributor Author

@swift-ci test

@kastiglione
Copy link
Contributor Author

@swift-ci Please clean test

@kastiglione
Copy link
Contributor Author

It seems the failures are somehow caused by the macro emitting @_section(...) attributes. If I change the macro to not use a @_section, the serialization failure stops happening.

@kastiglione
Copy link
Contributor Author

@swift-ci test

3 similar comments
@kastiglione
Copy link
Contributor Author

@swift-ci test

@kastiglione
Copy link
Contributor Author

@swift-ci test

@kastiglione
Copy link
Contributor Author

@swift-ci test

@kastiglione kastiglione mentioned this pull request Feb 13, 2024
Merged
@kastiglione
Copy link
Contributor Author

@swift-ci test windows

1 similar comment
@kastiglione
Copy link
Contributor Author

@swift-ci test windows

@kastiglione
Copy link
Contributor Author

@swift-ci test

1 similar comment
@kastiglione
Copy link
Contributor Author

@swift-ci test

@kastiglione
Copy link
Contributor Author

@swift-ci test linux

@kastiglione
Copy link
Contributor Author

@swift-ci test macOS

@kastiglione kastiglione marked this pull request as ready for review February 15, 2024 17:36
kastiglione added a commit that referenced this pull request Feb 21, 2024
@kastiglione
Add deserialization support for section attribute (#71588)
e021534 
While working on #71425, I hit an assert indicating the `@_section` attribute was not 
handled in `DeclDeserializer::deserializeDeclCommon`. This change adds the missing 
deserialization for the `@_section` attribute.
@kastiglione kastiglione force-pushed the dl/Debug-Introduce-use-of-DebugDescription-in-stdlib branch from d202bc9 to 2736660 Compare February 21, 2024 04:26
@kastiglione
Copy link
Contributor Author

@swift-ci test

@kastiglione
Remove availability marker
81efa4a 
The macro is a compile time only dependency.
@kastiglione
Copy link
Contributor Author

@swift-ci test

@kastiglione kastiglione merged commit 5b35938 into main Feb 21, 2024
5 checks passed
@kastiglione kastiglione deleted the dl/Debug-Introduce-use-of-DebugDescription-in-stdlib branch February 21, 2024 17:17
kateinoigakukun
kateinoigakukun reviewed Feb 22, 2024
View reviewed changes
stdlib/public/core/CMakeLists.txt
@@ -310,6 +315,10 @@ list(APPEND swift_stdlib_compile_flags "-enable-experimental-feature" "Freestand
list(APPEND swift_stdlib_compile_flags "-enable-experimental-feature" "Extern")
list(APPEND swift_stdlib_compile_flags "-enable-experimental-feature" "BitwiseCopyable")

list(APPEND swift_stdlib_compile_flags "-external-plugin-path"
"${CMAKE_BINARY_DIR}/lib/swift/host/plugins#${CMAKE_BINARY_DIR}/bin/swift-plugin-server")
Copy link
Member

Choose a reason for hiding this comment

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

It seems this change broke cross-host builds: https://ci.swift.org/job/oss-swift-package-macos/2718/console

sandbox-exec: execvp() of '/Users/ec2-user/jenkins/workspace/oss-swift-package-macos/build/buildbot_osx/swift-macosx-arm64/bin/swift-plugin-server' failed: Bad CPU type in executable

I guess we need to use swift-plugin-server under SWIFT_NATIVE_SWIFT_TOOLS_PATH if it's specified.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

thanks, working on it

Copy link
Contributor Author

Choose a reason for hiding this comment

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

#71814

Is there a way to validate the cross-host builds from the PR?

Copy link
Member

Choose a reason for hiding this comment

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

Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kateinoigakukun kateinoigakukun kateinoigakukun left review comments

@adrian-prantl adrian-prantl adrian-prantl approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@kastiglione @adrian-prantl @kateinoigakukun