Skip to content

[ClangImporter] Prefer available enum elements over unavailable ones. #6990

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

Merged

Conversation

jrose-apple
Copy link
Contributor

...and avoid making aliases from one unavailable declaration to another. If it's unavailable, we can just import it as a normal case and not worry about it. This fixes an issue where Sema would try to diagnose the body of an "alias" for referring to unavailable declarations.

(Background: enum cases in Swift have to have unique values, so we import any duplicate values as static properties. Pattern matching logic has a hack to recognize these particular static properties as being "case-like".)

This commit also sinks enum element uniqueness checking into importing the enum, instead of keeping a global map we never consult again. This should save a small bit of memory.

rdar://problem/30025723

I stuck two other cleanup commits in here as well; they should have no change in behavior.

Don't bail out early when there's no deprecated-as-unavailable mode;
we also use the loop to check for `availability(swift, unavailable)`.
No test because the only callers are for Objective-C declarations.
The function is a callback parameter that does not escape, therefore
function_ref is the right choice.

No functionality change.
...and avoid making aliases from one unavailable declaration to another.
If it's unavailable, we can just import it as a normal case and not
worry about it. This fixes an issue where Sema would try to diagnose
the body of an "alias" for referring to unavailable declarations.

(Background: enum cases in Swift have to have unique values, so we
import any duplicate values as static properties. Pattern matching
logic has a hack to recognize these particular static properties as
being "case-like".)

This commit also sinks enum element uniqueness checking into importing
the enum, instead of keeping a global map we never consult again. This
should save a small bit of memory.

rdar://problem/30025723
@jrose-apple jrose-apple requested a review from milseman January 23, 2017 23:40
@jrose-apple
Copy link
Contributor Author

This whole area still deserves a cleanup around "Swift 2 names", but that'll come after I get back to #6720.

@swift-ci Please test

Copy link
Member

@DougGregor DougGregor left a comment

Choose a reason for hiding this comment

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

LGTM, thank you!

@@ -1792,6 +1792,26 @@ applyPropertyOwnership(VarDecl *prop,
}

namespace {
/// Customized llvm::DenseMapInfo for storing borrowed APSInts.
Copy link
Member

Choose a reason for hiding this comment

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

This is very... clever...alright.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's probably not even necessary, but it's not worse than what was there before, and it's boxed up pretty neatly.

@jrose-apple jrose-apple merged commit 6fed3d1 into swiftlang:master Jan 26, 2017
@jrose-apple jrose-apple deleted the available-is-better-than-unavailable branch January 26, 2017 19:52
jrose-apple added a commit to jrose-apple/swift that referenced this pull request Jan 26, 2017
…r-than-unavailable

[ClangImporter] Prefer available enum elements over unavailable ones.
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.

2 participants