Skip to content

CoreFoundation: remove private headers from public directory #2395

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
Closed
wants to merge 1 commit into from
Closed

CoreFoundation: remove private headers from public directory #2395

wants to merge 1 commit into from

Conversation

compnerd
Copy link
Member

@compnerd compnerd commented Jul 7, 2019

Clean up the long standing issue with private headers being pushed into
the public headers directory. Add a private modulemap so that we can
still access the private headers.

@compnerd
Copy link
Member Author

compnerd commented Jul 7, 2019

@swift-ci please test

@millenomi
Copy link
Contributor

It nope'd.

@compnerd
Copy link
Member Author

compnerd commented Jul 9, 2019

@swift-ci please test

@compnerd compnerd force-pushed the privacy-is-a-right branch from 9023bf5 to 2da5472 Compare July 10, 2019 05:49
@compnerd
Copy link
Member Author

Okay, there are some hacks here but, this might be better than what we had previously.

  • I desugared some types on Windows as the entire WinSDK is not fully modularized yet
  • I had to change some bool to Boolean (could just include <stdbool.h>)
  • I had to include <synchapi.h> explicitly to deal with the partial modular headers :-(
  • -Xcc -DDEPLOYMENT_RUNTIME_SWIFT needs to be propagated for ForFoundationOnly.h to not include <objc/message.h> (could use __has_include)

Otherwise, we should no longer be exposing any of the private headers from the import.

@compnerd
Copy link
Member Author

@swift-ci please test

@compnerd compnerd force-pushed the privacy-is-a-right branch from 2da5472 to 57ad1bd Compare July 11, 2019 00:16
@compnerd
Copy link
Member Author

@swift-ci please test Linux platform

@compnerd compnerd force-pushed the privacy-is-a-right branch from 57ad1bd to 256dd9a Compare July 11, 2019 03:57
@compnerd
Copy link
Member Author

@swift-ci please test Linux platform

@compnerd compnerd force-pushed the privacy-is-a-right branch from 256dd9a to e629239 Compare July 14, 2019 02:58
@compnerd
Copy link
Member Author

@swift-ci please test Linux platform

@compnerd compnerd force-pushed the privacy-is-a-right branch from e629239 to 0a25a94 Compare July 14, 2019 03:50
@compnerd
Copy link
Member Author

@swift-ci please test Linux platform

@compnerd compnerd force-pushed the privacy-is-a-right branch from 0a25a94 to 353c7f7 Compare July 14, 2019 05:25
@compnerd
Copy link
Member Author

@swift-ci please test Linux platform

@compnerd compnerd force-pushed the privacy-is-a-right branch from 353c7f7 to 2900ad9 Compare July 14, 2019 23:24
@compnerd
Copy link
Member Author

@swift-ci please test Linux platform

2 similar comments
@compnerd
Copy link
Member Author

@swift-ci please test Linux platform

@millenomi
Copy link
Contributor

@swift-ci please test Linux platform

@compnerd compnerd force-pushed the privacy-is-a-right branch from 2900ad9 to c72dbe0 Compare July 26, 2019 00:48
@compnerd
Copy link
Member Author

@swift-ci please test Linux platform

@compnerd compnerd force-pushed the privacy-is-a-right branch from c72dbe0 to a7141da Compare July 26, 2019 22:41
@compnerd
Copy link
Member Author

@swift-ci please test Linux platform

@compnerd
Copy link
Member Author

@swift-ci please test Linux platform

@compnerd
Copy link
Member Author

@swift-ci please test

@compnerd
Copy link
Member Author

@swift-ci please test

@compnerd
Copy link
Member Author

@swift-ci please test Linux platform

@compnerd
Copy link
Member Author

@swift-ci please test Linux platform

@compnerd
Copy link
Member Author

@swift-ci please test Linux platform

1 similar comment
@compnerd
Copy link
Member Author

@swift-ci please test Linux platform

millenomi
millenomi reviewed Mar 4, 2020
View reviewed changes
Sources/Foundation/Data.swift
@@ -2083,7 +2083,6 @@ public struct Data : ReferenceConvertible, Equatable, Hashable, RandomAccessColl
}

// slightly faster paths for common sequences
@inlinable // This is @inlinable as an important generic funnel point, despite being a non-trivial initializer.
Copy link
Contributor

Choose a reason for hiding this comment

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

Why remove inlineability here?

Copy link
Member Author

@compnerd compnerd Mar 4, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

It was a cascading effect due to the use of _withStackOrHeapBuffer getting inlined, making this inlinable would expose the @_implementationOnly interface. Of course it is possible something may have changed since.

@spevans
Copy link
Contributor

spevans commented Mar 5, 2020

This fails to build under Xcode

/Applications/Xcode-beta.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX10.15.sdk/System/Library/Frameworks/CoreFoundation.framework/Modules/module.modulemap:1:18: error: redefinition of module 'CoreFoundation'
framework module CoreFoundation [extern_c] [system] {
                 ^
/Users/spse/src/swift/swift-corelibs-foundation/DerivedData/Foundation/Build/Products/Debug/usr/local/include/CoreFoundation/module.map:1:8: note: previously defined here
module CoreFoundation [extern_c] [system] {
       ^
/Users/spse/src/swift/swift-corelibs-foundation/Sources/Foundation/NSArray.swift:13:8: error: no such module 'CoreFoundation_Private'
import CoreFoundation_Private

@compnerd
Copy link
Member Author

compnerd commented Mar 5, 2020

@spevans thats odd - that should have failed previously as well. I'm merely switching everything over to the @_implementationOnly import for the private side of the framework.

@compnerd
remove private headers from public directory
e1fb376 
Clean up the long standing issue with private headers being pushed into
the public headers directory.  Add a private modulemap so that we can
still access the private headers.
@compnerd compnerd force-pushed the privacy-is-a-right branch from eaa74f1 to e1fb376 Compare April 10, 2020 03:36
@compnerd
Copy link
Member Author

Please test with following PRs:
swiftlang/llvm-project#1057

@swift-ci please test Linux platform

@shahmishal shahmishal closed this Oct 6, 2020
@shahmishal
Copy link
Member

The Swift project moved the default branch to main and deleted master branch, so GitHub automatically closed the PR. Please re-create pull request with main branch.

More detail about the branch update - https://forums.swift.org/t/updating-branch-names/40412

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@millenomi millenomi millenomi left review comments

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

Successfully merging this pull request may close these issues.
4 participants
@compnerd @millenomi @spevans @shahmishal