Correct getAccessStrategy for class extension storage decls. #10050
Conversation
Class extension methods are non-polymorphic unless @objc. Fixes rdar://problem/32434652.
|
@slavapestov @jrose-apple Are there any other edge cases to account for here? |
|
@swift-ci Please smoke test |
|
LGTM. |
| return false; | ||
| // Extension properties are statically dispatched, unless they're @objc. | ||
| if (storage->getDeclContext()->isExtensionContext() | ||
| && !storage->isObjC()) |
There was a problem hiding this comment.
This seems like a coincidence rather than a design decision. I'd rather not put that into key path logic.
There was a problem hiding this comment.
I fully expect us to get overridable methods in extensions some day, and that will include computed properties.
There was a problem hiding this comment.
Also, is there a distinction to be made between "imported from ObjC" and "declared @objc" and "@objc dynamic"?
There was a problem hiding this comment.
This isn't keypath-specific, it's part of AbstractStorageDecl::getAccessStrategy. When we change the language, this seems like the right place to centralize that logic.
There was a problem hiding this comment.
I'd rather just be implicitly marking extension declarations final than put special cases for them all over the place. This might be the right place to put one, but it's not the only one.
There was a problem hiding this comment.
I agree that making them final is a better fix, but might be a bit too invasive for 4.0. Do we have a bug for that?
Class extension methods are non-polymorphic unless @objc. Fixes rdar://problem/32434652.