[ClangImporter] Remove importer-based NS stripping. #3880
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Contributor
Author
|
@DougGregor, mind checking the compiler parts of this? @swift-ci Please test OS X platform |
Contributor
|
Cool. |
Member
|
Compiler bits look great, thanks @jrose-apple ! |
As Tony puts it, in the end we wound up with more Foundation declarations imported as members or keeping "NS" than those that dropped it, and any further decisions will be made on a case-by-case basis. Move all of the existing cases of prefix-stripping into Foundation's API notes and drop the logic from the compiler. Tested by dumping the generated interface for Foundation and its submodules for both macOS and the iOS simulator, and comparing the results. A few cases did slip through here because of the interaction between "SwiftName" and "Availability: nonswift". The next commit will re-add "NS" to some stragglers that we missed. rdar://problem/26880017
NSKeyedUnarchiverDelegate NSKeyedArchiverDelegate NSTextCheckingTypes NSBinarySearchingOptions NSEnumerationOptions NSSortOptions More rdar://problem/26880017
No change observed in the generated interface of Foundation and its submodules. Finishes rdar://problem/26880017.
jrose-apple
force-pushed
the
stop-drop-ns
branch
from
9657c4d to
d15aa03
Compare
Contributor
Author
|
@swift-ci Please test OS X platform |
Contributor
Author
|
Over to Ted to merge. We only ever stripped from modules named "ObjectiveC" and "Foundation", so this should have no effect on Linux. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
As @parkera puts it, in the end we wound up with more Foundation declarations imported as members or keeping "NS" than those that dropped it, and any further decisions will be made on a case-by-case basis. Move all of the existing cases of prefix-stripping into Foundation's API notes and drop the logic from the compiler.
Tested by dumping the generated interface for Foundation and its submodules for both macOS and the iOS simulator, and comparing the results. A few cases did slip through here because of the interaction between
SwiftNameandAvailability: nonswift.rdar://problem/26880017
Before merging this pull request to apple/swift repository:
Triggering Swift CI
The swift-ci is triggered by writing a comment on this PR addressed to the GitHub user @swift-ci. Different tests will run depending on the specific comment that you use. The currently available comments are:
Smoke Testing
A smoke test on macOS does the following:
device standard libraries are not built.
version of these tests are not run.
A smoke test on Linux does the following:
tests are not run.
Validation Testing
Lint Testing
Note: Only members of the Apple organization can trigger swift-ci.