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.

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

Runtime: add UnknownObject on non-ObjC builds #20309

Merged
merged 1 commit into from Nov 5, 2018
Merged

Conversation

compnerd
Copy link
Collaborator

@compnerd compnerd commented Nov 4, 2018

The UnknownObject type is used for the block convention block pointers.
This is used in libdispatch and we cannot build for Windows without it.

Replace this paragraph with a description of your changes and rationale. Provide links to external references/discussions if appropriate.

Resolves SR-NNNN.

The UnknownObject type is used for the block convention block pointers.
This is used in libdispatch and we cannot build for Windows without it.
@compnerd
Copy link
Collaborator Author

compnerd commented Nov 4, 2018

CC: @jckarter @mikeash @gparker42

@compnerd
Copy link
Collaborator Author

compnerd commented Nov 4, 2018

@swift-ci please test

@swift-ci
Copy link
Collaborator

swift-ci commented Nov 4, 2018

Build failed
Swift Test OS X Platform
Git Sha - 5ee3566

Copy link
Member

@jckarter jckarter left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This shouldn't be necessary. How does libdispatch work on Linux? You may need the value witness table, but the type should not have to exist.

@compnerd
Copy link
Collaborator Author

compnerd commented Nov 4, 2018

@jckarter pretty sure that it is getting away since not all symbols need to be fulfilled by default on ELF (-z defs is not the default).

@troughton
Copy link
Contributor

FWIW, when I built the Swift Dispatch overlays on Windows against DrPizza's libdispatch port I didn't need to make any compiler changes, although I did need to enable support for and link in the Blocks runtime to the libdispatch library. I'm not sure what's different to cause you to need changes here.

@compnerd
Copy link
Collaborator Author

compnerd commented Nov 4, 2018

@troughton this is specifically the SDK overlay which is not part of that port I believe.

@jckarter btw, this is a smaller reduced case:

class C {
  var _block: @convention(block) () -> Void
  init(block: @escaping @convention(block) () -> Void) {
    self._block = block
  }
}

You're correct in that Linux for some reason does not generate that reference! The reference is generated in the type metadata completion function for C. I don't see that being generated on Linux for some reason?

@troughton
Copy link
Contributor

troughton commented Nov 4, 2018

If it helps to narrow down the problem:

By the Dispatch overlays I mean these files, along with the C stubs reimplemented in DispatchStubs.cc.

When linked against the Windows SDK and built for x64 Windows this did not require any compiler changes for a Swift 4.1-era built-from-master toolchain.

@compnerd
Copy link
Collaborator Author

compnerd commented Nov 4, 2018

@jckarter I wonder if this is a result of the lazy initialized metadata ... which may now emit the completion function for the initialization?

@compnerd
Copy link
Collaborator Author

compnerd commented Nov 4, 2018

Okay, verified, this is the result of the lazy initialization of the type metadata. This in fact would be needed on ELF if we moved to the lazy initialization of the type metadata.

@jckarter
Copy link
Member

jckarter commented Nov 4, 2018

@compnerd Are you sure it's not only looking for the value witness table? We really want to eliminate the UnknownObject builtin type even for Darwin targets.

@compnerd
Copy link
Collaborator Author

compnerd commented Nov 4, 2018

@jckarter yes, it is looking for the VWT. The changes here enable the emission of the metadata into the KnownMetadata.cpp.

@jckarter
Copy link
Member

jckarter commented Nov 4, 2018

I see. It would be nice to only include the value witness table, but that's splitting hairs a bit.

@compnerd
Copy link
Collaborator Author

compnerd commented Nov 5, 2018

@swift-ci please test macOS platform

@swift-ci
Copy link
Collaborator

swift-ci commented Nov 5, 2018

Build failed
Swift Test OS X Platform
Git Sha - 5ee3566

@compnerd
Copy link
Collaborator Author

compnerd commented Nov 5, 2018

@swift-ci please clean test macOS platform

@compnerd compnerd merged commit 99d9f43 into apple:master Nov 5, 2018
@compnerd compnerd deleted the BO branch November 5, 2018 04:04
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.

None yet

4 participants