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

[SE-0025] Adapt accessibility for fileprivate #124

Closed
wants to merge 1 commit into from

Conversation

modocache
Copy link
Collaborator

@modocache modocache commented Jun 13, 2016

The standard migration path for SE-0025 is to find-and-replace all uses of private with fileprivate. However, this expands accessibility beyond the necessary minimum. This commit uses compiler errors to determine the minimum set of changes.

This should only be tested/merged once apple/swift#3000 has landed in apple/swift. Thanks to @CodaFi for making it easy to enable the new fileprivate errors -- to discover the necessary changes I applied the following change to apple/swift#3000 (since that pull request now aliases private to fileprivate for migration purposes):

diff --git a/lib/AST/NameLookup.cpp b/lib/AST/NameLookup.cpp
index bee6b2f..149c8a4 100644
--- a/lib/AST/NameLookup.cpp
+++ b/lib/AST/NameLookup.cpp
@@ -1017,7 +1017,7 @@ static bool checkAccessibility(const DeclContext *useDC,

   switch (access) {
   case Accessibility::Private:
-//      return useDC == sourceDC || useDC->isChildContextOf(sourceDC);
+     return useDC == sourceDC || useDC->isChildContextOf(sourceDC);
   case Accessibility::FilePrivate:
     return useDC->getModuleScopeContext() == sourceDC->getModuleScopeContext();
   case Accessibility::Internal: {

We could probably refactor a bit after this pull request, to reduce API from fileprivate to private where possible.

The standard migration path for SE-0025 is to find-and-replace all
uses of `private` with `fileprivate`. However, this expands
accessibility beyond the necessary minimum. This commit uses compiler
errors to determine the minimum set of changes.
@modocache
Copy link
Collaborator Author

apple/swift#3391 was merged, adding a fileprivate alias that acts identically to private for now. I wonder how this patch works with that alias...

@swift-ci please test

@mike-ferris
Copy link

Jordan submitted a new request that has less diffs (just one, actually) because the proposal was amended such that not all the conversions in this patch are actually needed anymore.

@modocache
Copy link
Collaborator Author

Sounds good; closing this in favor of #134. Thanks @jrose-apple!

@modocache modocache closed this Jul 11, 2016
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.

None yet

2 participants