-
Notifications
You must be signed in to change notification settings - Fork 469
Use SWIFT_SDK_OVERLAY_DISPATCH_EPOCH to detect that an overlay is bei… #97
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
Use SWIFT_SDK_OVERLAY_DISPATCH_EPOCH to detect that an overlay is bei… #97
Conversation
Seems reasonable but I'd like for @mwwa to double check. |
This doesn't seem correct to me. Won't it be set whenever any Swift code is used, not just when building the overlay? |
The problem I'm trying to work around could be a symptom of me not teaching the makefiles how to build the Dispatch and CDispatch modules correctly. I've tried a bunch of things, but I could have missed something. The problem is that the current approach of defining DISPATCH_BUILDING_SWIFT_MODULE only when actually building the overlay doesn't work very well. When a user imports Dispatch into their Swift program, they also need to define DISPATCH_BUILDING_SWIFT_MODULE or the import of the dispatch header files fails. It appears that when the Dispatch module is imported by a user program, the Swift compiler goes off, looks at the modulemap file that defines the Dispatch and CDispatch modules, and attempts to reimport the C dispatch header files for CDispatch. This import fails because DISPATCH_BUILDING_SWIFT_MODULE isn't defined (unless the user magically knows to do that via a command line argument to swiftc). If there were a way to build the swiftmodule that would cut off this import when users import the module, that would be great and we could do that instead. A related thing that would be great to fix, when the user imports Dispatch, they currently need to add -Xcc -fblocks to the swiftc command line manually. I'd really like to make that automatic (triggered by importing Dispatch), but don't know a good way of doing that. |
So DISPATCH_BUILDING_SWIFT_MODULE really just means "Dispatch is going to be used from Swift"? :-/ We've mostly been avoiding providing a general query of that kind—so that people don't accidentally break the Swift side of that branch, or accidentally not expose some API—but maybe we need to give up on that. (I wouldn't want to make it Dispatch-specific, though.) On the related thing: we're already considering just turning on blocks all the time for Swift-on-Linux, but we're wondering if we need to make them AnyObject-compatible first. I'm not in most of those discussions, though. |
@jrose-apple there's no way you can have something like dispatch work with swift without an equivalent of IOW I argue that the swift importer should have a |
The original idea was that swift-corelibs-dispatch and any other framework not shipped with the OS should always be Swifty, at least on non-Apple platforms. In particular, since you're only using this guard to protect against the code being considered non-modular, checking for |
(As an aside: The Darwin version of this check uses it to alter the class hierarchy appearance under Swift, something that really does need to be checking for Swift-and-not-C, so that we don't alter the API that we've historically presented to Obj-C clients) |
Yeah, we probably will need this at some point. I just want to do it properly when we get there, including discussing it on swift-evolution. |
@jrose-apple the problem with this is that the codebase is shared with macOS where it is not the case, and I don't think we should break people using libdispatch as a C library outside of swift on Linux. It actually is somehow broken today because objects are wrapped, but for the sake of interoperating with say, CF level APIs, being able to address libdispatch in C is valuable even on non macOS platforms. so I don't think the intent works in practice, however pure and noble it is :) Just for the sake of porting and testing libdispatch on new platforms it's much easier if you can isolate dispatch from Swift. |
also |
fwiw, I think having a SWIFT defined by the importer would be quite reasonable. In the very short term, the has_feature(modules) does work (as one would expect) and is inside a __linux. So we would take that as a an interim fix if you don't want to define the epoch off Darwin and getting SWIFT would take a full swift evolution. I'd like to get the Swift3 overlay workable as soon as possible so we can start using it for corelibs-foundation. |
Right since it's inside a |
I'll bring up |
Instead of using a special macro, check for __has_feature(modules) to activate including stdio.h in dispatch.h on Linux. Also, fix one place where OS_OBJECT_SWIFT_3 was used outside of an OS_OBJECT_USE_OBJC block (to prepare for SWIFT_SDK_OVERLAY_DISPATCH_EPOCH being defined by the clang importer on non-Darwin platforms).
8c5a6af
to
0f402b8
Compare
updated to modules features test & squashed to single commit. |
…-on-linux Use SWIFT_SDK_OVERLAY_DISPATCH_EPOCH to detect that an overlay is bei… Signed-off-by: Daniel A. Steffen <dsteffen@apple.com>
…ng built
Instead of defining the special macro DISPATCH_BUILDING_SWIFT_MODULE
to detect that dispatch headers are being processed by the clang importer,
check to see if SWIFT_SDK_OVERLAY_DISPATCH_EPOCH is defined.
The prime motivation for doing this is to avoid needing to define DISPATCH_BUILDING_SWIFT_MODULE when compiling a Swift program that imports Dispatch. A secondary benefit is that it makes additional functions unavailable for Swift3 (eg dispatch_once).
This depends on a pull request (swiftlang/swift#3346) to Swift to change the importer to define SWIFT_SDK_OVERLAY_DISPATCH_EPOCH on non-Darwin platforms.