Skip to content
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

Sema: Targeted fix for bad interaction between resilience checks and -enable-testing [4.0] #10179

Conversation

slavapestov
Copy link
Contributor

  • Description: New checks introduced in Swift 4.0 mode are intended to prevent public default argument expressions from referencing internal and private symbols. The diagnostic code used the wrong frontend APIs however, and as a result with -enable-testing we had the totally unexpected behavior that internal default arguments could not reference private symbols.

  • Scope of the issue: Affects users migrating code from Swift 3.2 to 4.0.

  • Origination: The checks are new to Swift 4.0.

  • Risk: Low.

  • Tested: Existing tests now all run with and without -enable-testing to ensure the right APIs are used.

  • Reviewed by: @jrose-apple

  • Radar: rdar://problem/32592973

…-enable-testing

The -enable-testing flag makes ValueDecl::getEffectiveAccess()
say that internal declarations are public.

This would lead us to emit spurious diagnostics if a default
argument of an internal function referenced a private symbol,
for example, which is something we actually want to allow.

This is a second revision of the patch -- instead of changing
getEffectiveAccess() to take an extra parameter, this changes
getFormalAccessScope() instead.

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

@jrose-apple This is the second version of the patch from master; is it OK to go into 4.0?

@slavapestov
Copy link
Contributor Author

@swift-ci Please test

@jrose-apple
Copy link
Contributor

Yep, it's okay.

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