Skip to content

[4.1] [ClangImporter] Handle subscript getters redeclared in subclasses #13505

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 1 commit into from
Dec 18, 2017
Merged

[4.1] [ClangImporter] Handle subscript getters redeclared in subclasses #13505

merged 1 commit into from
Dec 18, 2017

Conversation

jrose-apple
Copy link
Contributor

@jrose-apple jrose-apple commented Dec 18, 2017 •
edited
Loading

  • Explanation: Swift accidentally assumed that whenever an Objective-C subscript getter method and setter method were declared in different places, the setter was always going to be in a subclass (turning a read-only subscript into a read/write one). That's not true if there are redeclarations involved. The fix is to check if the two methods are in different classes and prefer the accessor we started with.
  • Scope: Affects how Objective-C subscripts are imported if the getter and setter are declared in different places.
  • Issue: rdar://problem/36033356
  • Reviewed by: @DougGregor
  • Risk: Medium-low. This kind of change for Objective-C subscripts isn't super common.
  • Testing: Added compiler regression tests, passed the source compatibility suite

@jrose-apple
[ClangImporter] Handle subscript getters redeclared in subclasses (sw…
cfceff4 
…iftlang#13470)

Objective-C subscripts don't have special declarations like
properties; they're just specially-named methods (or method pairs),
and we have to make an independent SubscriptDecl in Swift. This means
that when a subclass wants to make a subscript settable, they just add
the appropriately-named setter method.

Swift handled this by detecting when the getter and setter weren't
declared in the same type, and assuming this meant it was a subclass
adding a setter. Unfortunately, the same condition /also/ picked up
the case where the getter (and only the getter) is /redeclared/ in a
subclass (perhaps to add an attribute), and the new subscript was
getting added to the base class instead of the subclass.

The fix relies on the fact that the original decl we provide is what
we use to look up the other accessor. If the getter and setter are in
different types, whichever one we started with must be the
more-derived one. So the final change is just "did we start with the
setter?" rather than "is there a setter at all?".

I'm not sure why this is only just now causing problems, given that we
seem to have been getting this wrong for years, but it definitely
/was/ wrong and now it's not.

rdar://problem/36033356
(cherry picked from commit e1f46b5)
@jrose-apple
Copy link
Contributor Author

@swift-ci Please test

@jrose-apple jrose-apple changed the title [ClangImporter] Handle subscript getters redeclared in subclasses [4.1] [ClangImporter] Handle subscript getters redeclared in subclasses Dec 18, 2017
@jrose-apple jrose-apple merged commit bea9dc3 into swiftlang:swift-4.1-branch Dec 18, 2017
@jrose-apple jrose-apple deleted the 4.1-suboptimal-subscripts branch December 18, 2017 20:56
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.
1 participant
@jrose-apple