-
Notifications
You must be signed in to change notification settings - Fork 10.5k
[CMake] Use separate namespace for demangling symbols used by runtime #78490
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
[CMake] Use separate namespace for demangling symbols used by runtime #78490
Conversation
@swift-ci please smoke test |
Runtimes/Core/CMakeLists.txt
Outdated
# the inline namespace to distinguish symbols from those built for the | ||
# compiler, in order to avoid possible ODR violations if both are statically | ||
# linked into the same binary. (see also commit message for 5b1daa9055c99904c84862ecc313641fd9b26e63) | ||
$<$<COMPILE_LANGUAGE:C,CXX>:-DSWIFT_INLINE_NAMESPACE=__runtime> # Demangling, runtime, Concurrency, RemoteInspection, SwiftRemoteMirror, swift-reflection-test |
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.
Since this is only needed by things that pull in libDemangling
, we should make this a PUBLIC
define coming from the libDemangling library. Then anything that links Demangling will pick up the appropriate setting.
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.
Thanks for the guidance -- I admit I blindly matched the existing code, without seeing its underlying purpose and translating it to proper modern CMake patterns.
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 global stash of definitions should eventually have the definition attached to the appropriate libraries. This was mostly for expedience when deciphering the logic in add_swift_target_library
was taking too long.
This matches the behavior introduced with swiftlang#30733 -- quoting the explanation here for the sake of convenience: > Since libDemangling is included in the Swift standard library, > ODR violations can occur on platforms that allow statically > linking stdlib if Swift code is linked with other compiler > libraries that also transitively pull in libDemangling, and if > the stdlib version and compiler version do not match exactly > (even down to commit drift between releases). This lets the > runtime conditionally segregate its copies of the libDemangling > symbols from those in the compiler using an inline namespace > without affecting usage throughout source. Addresses rdar://142550635
00a01e8
to
d546222
Compare
@swift-ci please smoke test |
This matches the behavior introduced with #30733 -- quoting the
explanation here for the sake of convenience:
Addresses rdar://142550635