stdlib: tweak import declarations #1724
Conversation
|
We really don't want these symbols to become a part of the public module API. We might need to bounce off a .m file instead. See |
|
I wasn't aware of that file. Ill update to use that instead. Thanks for the pointer! |
compnerd
force-pushed
the
correct-linkage
branch
from
2309174 to
50dbd47
Compare
Ensure that the functions which are declared with @_silgen_name for importing from external libraries are described with the correct linkage. If the declared functions are marked with internal or no linkage, the declaration for the import will be given internal linkage. However, this is incorrect if the definition is not part of the image. The remaining uses were filtered on the assumption that the swift standard library is statically linked and provides the definitions for those symbols and thus will be part of the image. This was noticed by manual inspection of the IR generated. Thanks to Dmitri Gribenko for the hint about the trampoline construction.
compnerd
force-pushed
the
correct-linkage
branch
from
50dbd47 to
42678c1
Compare
|
The original change was a bit overzealous. Ive adjusted the current version to use trampolines as you suggested and agree that this is a much better option than expanding the API surface. |
| @@ -196,10 +196,10 @@ typealias Zone = NSZone | |||
| //===----------------------------------------------------------------------===// | |||
|
|
|||
| @warn_unused_result | |||
| @_silgen_name("objc_autoreleasePoolPush") | |||
| @_silgen_name("_swift_objc_autoreleasePoolPush") | |||
There was a problem hiding this comment.
@gribozavr Do you know if these trampolines are necessary? Is the ObjectiveC module far enough long in the build that we could reliably import these from Clang instead?
There was a problem hiding this comment.
It does not look like these functions are declared in the SDK, so there's no header that we could use.
|
@swift-ci Please test |
|
Anything else to be done for this change? |
|
LGTM! |
stdlib: tweak import declarations
What's in this pull request?
Resolved bug number: (SR-)
Before merging this pull request to apple/swift repository:
Triggering Swift CI
The swift-ci is triggered by writing a comment on this PR addressed to the GitHub user @swift-ci. Different tests will run depending on the specific comment that you use. The currently available comments are:
Smoke Testing
Validation Testing
Note: Only members of the Apple organization can trigger swift-ci.
Ensure that the functions which are declared with @_silgen_name for importing
from external libraries are described with the correct linkage. If the declared
functions are marked with internal or no linkage, the declaration for the import
will be given internal linkage. However, this is incorrect if the definition is
not part of the image.
The remaining uses were filtered on the assumption that the swift standard
library is statically linked and provides the definitions for those symbols and
thus will be part of the image.
This was noticed by manual inspection of the IR generated.