Skip to content

Conversation

@amraboelela
Copy link
Contributor

No description provided.

@MadCoder
Copy link
Contributor

MadCoder commented Aug 6, 2016

@seabaylea / @dgrove-oss to review this

@MadCoder
Copy link
Contributor

MadCoder commented Aug 6, 2016

I think this patch isn't correct and there's discussion to enable -fblocks in swiftc by default, but I haven't followed the discussions closely enough.

@amraboelela
Copy link
Contributor Author

amraboelela commented Aug 6, 2016

-fblocks are getting added with clang and swiftc in the build process, (maybe they already made it to be added by default now).

/home/amr/swift/build/buildbot_linux/llvm-linux-x86_64/bin/clang -DHAVE_CONFIG_H -I. -I/home/amr/swift/swift-corelibs-libdispatch/tests -I../config -I.. -I/home/amr/swift/swift-corelibs-libdispatch -DDISPATCH_USE_DTRACE=0 -Wall -Wno-deprecated-declarations -fblocks -I/usr/include/kqueue -isystem /usr/include/bsd -DLIBBSD_OVERLAY -O2 -c -o dispatch_select.o /home/amr/swift/swift-corelibs-libdispatch/tests/dispatch_select.c

If you keep the “link blocks” line in the dispatch/module.modulemap file then you would get the bellow error, plus you will also get the same error during the build.

1> import Dispatch
error: module 'CDispatch' requires feature 'blocks'
error: could not build Objective-C module ‘CDispatch'

On Aug 5, 2016, at 6:57 PM, Pierre Habouzit notifications@github.com wrote:

I think this patch isn't correct and there's discussion to enable -fblocks in swiftc by default, but I haven't followed the discussions closely enough.


You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub #137 (comment), or mute the thread https://github.com/notifications/unsubscribe-auth/AA15Cv4iBFi5h0SKhj7n6kjJWoraMPJdks5qc-oIgaJpZM4JeKBi.

@dgrove-oss
Copy link
Contributor

As I understand it, we need to import the C header files for libdispatch with block support. Thus, the requires blocks is "correct" (because we want to have block support enabled).

I've lost track of the resolution (if any) of the enabling blocks by default on Linux discussion. I'll try to get that sorted then update this issue.

@seabaylea
Copy link
Contributor

Whilst it certainly compiles and runs with the patch, I'd err on the side of caution and continue to require blocks to make sure that the compiler does the right thing.

We've already added -fblocks to the builds of Foundation, TestFoundation, XCTest and SwiftPM themselves. The last remaining step is to add it by default to swiftc on Linux so that its added for applications.

Before we can do that however, we need to resolve how we ensure that the BlocksRuntime is available.

@dgrove-oss
Copy link
Contributor

I'm closing this pull request because it will instead be handled by adding -fblocks as a default argument to the ClangImporter (swiftlang/swift#3835)

@dgrove-oss dgrove-oss closed this Aug 15, 2016
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.

4 participants