-
Notifications
You must be signed in to change notification settings - Fork 10.5k
AST: Fix accessibility checking in opaque type archetype substitution logic #30847
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
AST: Fix accessibility checking in opaque type archetype substitution logic #30847
Conversation
@swift-ci Please smoke test |
@swift-ci Please test source compatibility |
844c705
to
b94c7ce
Compare
@swift-ci Please test source compatibility |
@swift-ci Please test source compatibility Debug |
… logic We were failing to replace opaque types with their underlying type upon encountering an internal type from the current module. This could happen when the internal type appeared in generic substitutions, for example when calling a protocol extension method. Fixes <rdar://problem/60951353>.
b94c7ce
to
dbaa619
Compare
@swift-ci Please test source compatibility |
@swift-ci Please smoke test |
1 similar comment
@swift-ci Please smoke test |
@swift-ci Please test source compatibility |
if (dc->getParentModule() == typeDecl->getDeclContext()->getParentModule()) | ||
return true; | ||
|
||
return typeDecl->getEffectiveAccess() > AccessLevel::Internal; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for fixing this! Is there an existing resilience API we can use here to avoid duplicating visibility logic?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
None that I'm aware of at the moment. I have a follow-up PR with a couple of cleanups for a few other things I noticed while looking around at opaque type lowering, but not this function in particular. It's something we should chip away at over time.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me. Thanks for fixing.
We were failing to replace opaque types with their underlying type
upon encountering an internal type from the current module. This
could happen when the internal type appeared in generic substitutions,
for example when calling a protocol extension method.
Fixes rdar://problem/60951353.