Skip to content

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

Conversation

slavapestov
Copy link
Contributor

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.

@slavapestov
Copy link
Contributor Author

@swift-ci Please smoke test

@slavapestov
Copy link
Contributor Author

@swift-ci Please test source compatibility

@slavapestov slavapestov force-pushed the opaque-type-lowering-access-level-fix branch from 844c705 to b94c7ce Compare April 7, 2020 04:43
@slavapestov
Copy link
Contributor Author

@swift-ci Please test source compatibility

@slavapestov
Copy link
Contributor Author

@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>.
@slavapestov slavapestov force-pushed the opaque-type-lowering-access-level-fix branch from b94c7ce to dbaa619 Compare April 7, 2020 16:14
@slavapestov
Copy link
Contributor Author

@swift-ci Please test source compatibility

@slavapestov
Copy link
Contributor Author

@swift-ci Please smoke test

1 similar comment
@slavapestov
Copy link
Contributor Author

@swift-ci Please smoke test

@slavapestov
Copy link
Contributor Author

@swift-ci Please test source compatibility

if (dc->getParentModule() == typeDecl->getDeclContext()->getParentModule())
return true;

return typeDecl->getEffectiveAccess() > AccessLevel::Internal;
Copy link
Contributor

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?

Copy link
Contributor Author

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.

Copy link
Contributor

@aschwaighofer aschwaighofer left a 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.

@slavapestov slavapestov merged commit 2af7d8a into swiftlang:master Apr 7, 2020
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.

3 participants