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

Windows BlocksRuntime #362

Merged
merged 1 commit into from Oct 5, 2018
Merged

Windows BlocksRuntime #362

merged 1 commit into from Oct 5, 2018

Conversation

compnerd
Copy link
Collaborator

No description provided.

@compnerd
Copy link
Collaborator Author

CC: @MadCoder @millenomi

@MadCoder
Copy link
Member

how does this relate to #361 ?

@compnerd
Copy link
Collaborator Author

The only way it really relates to #361 is my laziness. I didn't pull this onto a separate branch so it is sitting on top. I can remove the underlying commit (I figure that I would have to rebase it anyways).

@MadCoder
Copy link
Member

ok that's a lot of mojo jumbo cmake and I understand very little of it, anyone feeling like reviewing this?

@MadCoder MadCoder requested a review from dgrove-oss June 20, 2018 17:47
@compnerd
Copy link
Collaborator Author

@MadCoder - rebased. It should be primarily Blocks.h that is changed. The CMake magic here is to add a .def to the Windows build to export _NSConcreteGlobalBlock and _NSConcreteStackBlock.

@MadCoder
Copy link
Member

@swift-ci please test

@compnerd
Copy link
Collaborator Author

Please test with following PR:
apple/swift#17451

@swift-ci please test

@jrose-apple
Copy link
Contributor

Doesn't the static build of libdispatch still need the static blocks runtime?

@jrose-apple
Copy link
Contributor

I also thought the blocks runtime was designed to be included statically. @rjmccall?

@rjmccall
Copy link
Member

I also thought the blocks runtime was designed to be included statically. @rjmccall?

IIRC, it's designed to be linked into the program image. I don't know how well it would work with multiple copies in an application; each copy would try to export the same strong symbols, which might annoy the static linker (depending on how you link it in) and might annoy the loader (depending on how much it enforces), but if you somehow got around that I don't know that anything would break — I don't think there are any global registries or anything like that. Certainly some of the ObjC-interop features wouldn't work, but that's probably okay on targets without ObjC interop.

@compnerd
Copy link
Collaborator Author

Please test with following PR:
apple/swift#17451
apple/swift-corelibs-foundation#1618

@swift-ci please test

@compnerd
Copy link
Collaborator Author

@rjmccall - yeah, the multiple definitions wouldn't work very well for parts of the runtime, so, although this adds an additional library, having it shared seems better.

@compnerd
Copy link
Collaborator Author

Please test with following PR:
apple/swift#17451
apple/swift-corelibs-foundation#1618

@swift-ci please test

@compnerd
Copy link
Collaborator Author

Please test with following PR:
apple/swift#17451
apple/swift-corelibs-foundation#1618
apple/swift-package-manager#1631

@swift-ci please test

@compnerd
Copy link
Collaborator Author

Please test with following PR:
apple/swift#17451
apple/swift-corelibs-foundation#1618
apple/swift-package-manager#1631

@swift-ci please test

1 similar comment
@compnerd
Copy link
Collaborator Author

compnerd commented Jul 3, 2018

Please test with following PR:
apple/swift#17451
apple/swift-corelibs-foundation#1618
apple/swift-package-manager#1631

@swift-ci please test

@compnerd
Copy link
Collaborator Author

Please test with following PR:
apple/swift#17451

@swift-ci please test

@compnerd compnerd mentioned this pull request Oct 2, 2018
Closed
@compnerd
Copy link
Collaborator Author

compnerd commented Oct 2, 2018

Please test with following PR:
apple/swift#17451

@swift-ci please test

@compnerd
Copy link
Collaborator Author

compnerd commented Oct 3, 2018

Okay, this splits out the Windows handling now, but is rebased on top of #395. Once that is handled, we can come back to this.

@compnerd compnerd changed the title Shared BlocksRuntime Windows BlocksRuntime Oct 3, 2018
@compnerd compnerd force-pushed the shared branch 3 times, most recently from b476511 to 8088e28 Compare October 4, 2018 21:56
@compnerd
build: enable usage on Windows
23ce6f6 
Adjust the build for Windows to permit the isa pointer to successfully
link.  They will be off by a level of indirection and need to be patched
up at runtime.
@compnerd
Copy link
Collaborator Author

compnerd commented Oct 4, 2018

@swift-ci please test

@compnerd compnerd merged commit c4a7149 into apple:master Oct 5, 2018
@compnerd compnerd deleted the shared branch October 5, 2018 17:22
rokhinip pushed a commit that referenced this pull request Nov 5, 2021
@compnerd @rokhinip
Merge pull request #362 from compnerd/shared
cf9fd05 
Windows BlocksRuntime

Signed-off-by: Kim Topley <ktopley@apple.com>
@swift-ci swift-ci mentioned this pull request May 5, 2022
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@MadCoder MadCoder MadCoder approved these changes

@dgrove-oss dgrove-oss Awaiting requested review from dgrove-oss

@rjmccall rjmccall Awaiting requested review from rjmccall

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

Successfully merging this pull request may close these issues.

None yet
5 participants
@compnerd @MadCoder @jrose-apple @rjmccall @ktopley-apple