Skip to content

[Type checker] Provide actual ObjC runtime name in @NSKeyedArchiveLegacy Fix-Its. #9473

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
merged 3 commits into from
May 10, 2017

Conversation

DougGregor
Copy link
Member

While here, ban users from ever uttering @ _staticInitializeObjCMetadata.

Fixes rdar://problem/32118614.

@DougGregor DougGregor requested a review from jrose-apple May 10, 2017 22:01
@DougGregor
Copy link
Member Author

@swift-ci please smoke test and merge

@@ -6048,13 +6049,16 @@ void TypeChecker::checkConformancesInContext(DeclContext *dc,
auto insertionLoc =
classDecl->getAttributeInsertionLoc(/*forModifier=*/false);
if (isFixable) {
Mangle::ASTMangler mangler;
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you leave behind a comment noting that this is deliberately using the Swift 3 mangling?

Copy link
Member Author

Choose a reason for hiding this comment

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

Sure.

Copy link
Contributor

@jrose-apple jrose-apple left a comment

Choose a reason for hiding this comment

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

Thanks, Doug!

Erik and I also noticed another inconsistency, which is that we shouldn't be doing any of this on Linux Foundation. (Neither the fix-its nor the eager initialization will work, right?) But at least no one will be using it there, because that already fails to handle non-simple mangled names.

@DougGregor DougGregor merged commit 30482a0 into swiftlang:master May 10, 2017
@DougGregor DougGregor deleted the nskeyedarchivelegacy-fixits branch May 10, 2017 23:26
@DougGregor
Copy link
Member Author

yeah, got it #9487

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