Skip to content

Generic type alias access level regression [4.2] #16584

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

I noticed this while working on better checking of @usableForInline access control.

If a property had an inferred type that is a generic type alias, we weren't checking the access level of the generic arguments.

This used to work in 4.1 because we didn't have a sugared representation for generic type aliases, so we were checking the substituted underlying type here. But now that NameAliasType can store generic arguments, make sure we visit them here.

Fixes rdar://problem/40188409.

In Swift 3 mode, the canonical private DeclContext should
not look through extensions.

The only way I can reproduce this is with a missing warning
and we don't really care about missing warnings in Swift 3 mode.

However, the next patch in this PR regresses more things so
let's fix it properly.
If a property had an inferred type that is a generic type alias,
we weren't checking the access level of the generic arguments.

This used to work in 4.1 because we didn't have a sugared
representation for generic type aliases, so we were checking
the substituted underlying type here. But now that NameAliasType
can store generic arguments, make sure we visit them here.

Fixes <rdar://problem/40188409>.
@slavapestov
Copy link
Contributor Author

@swift-ci Please test

@slavapestov slavapestov requested a review from jrose-apple May 14, 2018 01:02
@slavapestov slavapestov merged commit f58f528 into swiftlang:swift-4.2-branch May 14, 2018
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.

Good catch, LGTM. Glad you caught this before we had to provide compatibility support.

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