Skip to content

[4.0] [ClangImporter] Add direct access for import-as-member types. #11013

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
Merged
merged 5 commits into from
Jul 19, 2017
Merged

[4.0] [ClangImporter] Add direct access for import-as-member types. #11013

merged 5 commits into from
Jul 19, 2017

Conversation

jrose-apple
Copy link
Contributor

@jrose-apple jrose-apple commented Jul 17, 2017 •
edited
Loading

  • Explanation: If a Swift library contains an extension of Objective-C type A that has a method that mentions Objective-C type A.B (using the "import-as-member" feature), performing the lookup to find A.B can lead to a circular dependency between deserialization and the importer, which resulted in a compiler crash. This is not a new problem, but it's more important with the release of Swift 4, where a number of Apple SDK types are now newly imported as member types. (The one in the original bug was NSView.AutoresizingMask, formerly NSAutoresizingMaskOptions.) This change adds a "fast path" for direct lookup of a single, unambiguous type nested in an Objective-C type.
  • Scope: Affects references in Swift to nested types imported from Objective-C.
  • Issue: SR-5284 / rdar://problem/32926560
  • Reviewed by: @milseman (ClangImporter), @jckarter (Serialization)
  • Risk: Medium-low. These types are not uncommon, but this is just a "fast path"; it should fall back to the old code if it fails.
  • Testing: Added regression tests, verified that the original test case now passes, passed the source compatibility suite on master.

@jrose-apple
[Serialization] Add extra counters and PrettyStackTraces.
f0b00cf 
And consolidate two slightly different subclasses of PrettyStackTrace
that mostly did the same thing.

(cherry picked from commit 7e043cf)
@jrose-apple
Merge pull request swiftlang#10956 from jrose-apple/lookupNestedType-…
121ebb0 
…for-Clang

[ClangImporter] Add direct access for import-as-member types.
@jrose-apple
[CMake] Cope with circular dependencies by tweaking link order.
c8e2a96 
Linux (rather, gold) is stricter than macOS (ld64) about this, so
until we can eliminate the rest of the dependencies this is the
best workaround we have.

(cherry picked from commit 9e2577d)
@jrose-apple
Copy link
Contributor Author

jrose-apple commented Jul 17, 2017 •
edited
Loading

I thought of a case I hadn't tested yet, #11018. Holding off on tests and such until I can get that in.

@jrose-apple
Copy link
Contributor Author

@swift-ci Please test

@jrose-apple
Copy link
Contributor Author

@swift-ci Please test source compatibility

@swift-ci
Copy link
Contributor

Build failed
Jenkins build - Swift Test Linux Platform
Git Commit - c8e2a96
Test requested by - @jrose-apple

@swift-ci
Copy link
Contributor

Build failed
Jenkins build - Swift Test OS X Platform
Git Commit - c8e2a96
Test requested by - @jrose-apple

@jrose-apple
Copy link
Contributor Author

@swift-ci Please test source compatibility

@jrose-apple
Copy link
Contributor Author 

/Users/buildnode/jenkins/workspace-private/swift-PR-source-compat-suite/swift/lib/ClangImporter/ClangImporter.cpp:2464:41: error: use of undeclared identifier 'SerializedSwiftName'
  for (auto entry : lookupTable->lookup(SerializedSwiftName(name.str()),
                                        ^

Right, this needs a bit of backporting. My bad.

@jrose-apple
Copy link
Contributor Author

@swift-ci Please test

@jrose-apple
Copy link
Contributor Author

@swift-ci Please test source compatibility

@swift-ci
Copy link
Contributor

Build failed
Jenkins build - Swift Test OS X Platform
Git Commit - 58d7868
Test requested by - @jrose-apple

@swift-ci
Copy link
Contributor

Build failed
Jenkins build - Swift Test Linux Platform
Git Commit - 58d7868
Test requested by - @jrose-apple

@jrose-apple
Copy link
Contributor Author

@swift-ci Please test source compatibility

@jrose-apple jrose-apple merged commit cdc3e6c into swiftlang:swift-4.0-branch Jul 19, 2017
@jrose-apple jrose-apple deleted the 4.0-lookupNestedType-for-Clang branch July 19, 2017 00:28
@belkadan belkadan mentioned this pull request Jul 18, 2017
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@jrose-apple @swift-ci