Skip to content

Remove blocks runtime when building with libdispatch #938

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

Merged
merged 1 commit into from
May 19, 2017

Conversation

spevans
Copy link
Contributor

@spevans spevans commented Apr 7, 2017

  • The blocks runtime is already contained in libdispatch
    so removing this 2nd copy from libFoundation removes duplicate
    code.

  • The libdispatch version is also slightly more optimal as it
    doesnt compile functions only needed when HAVE_OBJC is defined.

This is also needed to avoid duplicate symbol errors when statically linking Foundation using #873

Also see discussion about blocks runtime in #874

An alternative, which I did test, is just to remove data.c and runtime.c although this means libdispatch always has to be linked in and Im not sure if it is guaranteed to always be a dependancy.

- The blocks runtime is already contained in libdispatch
  so removing this 2nd copy from libFoundation removes duplicate
  code.

- The libdispatch version is also slightly more optimal as it
  doesnt compile functions only needed when HAVE_OBJC is defined.
@parkera
Copy link
Contributor

parkera commented Apr 18, 2017

@swift-ci please test

@spevans
Copy link
Contributor Author

spevans commented May 18, 2017

@parkera Can this be merged now?

@parkera
Copy link
Contributor

parkera commented May 19, 2017

Sure; let me kick off another test since it's been a while.

@parkera
Copy link
Contributor

parkera commented May 19, 2017

@swift-ci test and merge

@swift-ci swift-ci merged commit 425ea5e into swiftlang:master May 19, 2017
@spevans spevans deleted the pr_blocks_runtime branch August 9, 2017 06:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants