Merge pull request swiftlang#21783 from xedin/rdar-30933988

[AST] `Decl::is*AccessibleFrom` methods should respect access control…

Author: xedin

include/swift/AST/ASTContext.h

@@ -936,6 +936,12 @@ class ASTContext final {
     return LangOpts.isSwiftVersionAtLeast(major, minor);
   }
 
+  /// Check whether it's important to respect access control restrictions
+  /// in current context.
+  bool isAccessControlDisabled() const {
+    return !LangOpts.EnableAccessControl;
+  }
+
 private:
   friend Decl;
   Optional<RawComment> getRawComment(const Decl *D);

lib/AST/Decl.cpp

@@ -2788,6 +2788,9 @@ ValueDecl::getFormalAccessScope(const DeclContext *useDC,
 static bool checkAccessUsingAccessScopes(const DeclContext *useDC,
                                          const ValueDecl *VD,
                                          AccessLevel access) {
+  if (VD->getASTContext().isAccessControlDisabled())
+    return true;
+
   AccessScope accessScope =
       getAccessScopeForFormalAccess(VD, access, useDC,
                                     /*treatUsableFromInlineAsPublic*/false);
@@ -2806,7 +2809,12 @@ static bool checkAccessUsingAccessScopes(const DeclContext *useDC,
 ///
 /// See ValueDecl::isAccessibleFrom for a description of \p forConformance.
 static bool checkAccess(const DeclContext *useDC, const ValueDecl *VD,
-                        AccessLevel access, bool forConformance) {
+                        bool forConformance,
+                        llvm::function_ref<AccessLevel()> getAccessLevel) {
+  if (VD->getASTContext().isAccessControlDisabled())
+    return true;
+
+  auto access = getAccessLevel();
   auto *sourceDC = VD->getDeclContext();
 
   if (!forConformance) {
@@ -2868,8 +2876,8 @@ static bool checkAccess(const DeclContext *useDC, const ValueDecl *VD,
 
 bool ValueDecl::isAccessibleFrom(const DeclContext *useDC,
                                  bool forConformance) const {
-  auto access = getFormalAccess();
-  bool result = checkAccess(useDC, this, access, forConformance);
+  bool result = checkAccess(useDC, this, forConformance,
+                            [&]() { return getFormalAccess(); });
 
   // For everything outside of protocols and operators, we should get the same
   // result using either implementation of checkAccess, because useDC must
@@ -2878,9 +2886,9 @@ bool ValueDecl::isAccessibleFrom(const DeclContext *useDC,
   // because we're finding internal operators within private types. Fortunately
   // we have a requirement that a member operator take the enclosing type as an
   // argument, so it won't ever match.
-  assert(getDeclContext()->getSelfProtocolDecl() ||
-         isOperator() ||
-         result == checkAccessUsingAccessScopes(useDC, this, access));
+  assert(getDeclContext()->getSelfProtocolDecl() || isOperator() ||
+         result ==
+             checkAccessUsingAccessScopes(useDC, this, getFormalAccess()));
 
   return result;
 }
@@ -2898,8 +2906,8 @@ bool AbstractStorageDecl::isSetterAccessibleFrom(const DeclContext *DC,
   if (isa<ParamDecl>(this))
     return true;
 
-  auto access = getSetterFormalAccess();
-  return checkAccess(DC, this, access, forConformance);
+  return checkAccess(DC, this, forConformance,
+                     [&]() { return getSetterFormalAccess(); });
 }
 
 void ValueDecl::copyFormalAccessFrom(const ValueDecl *source,

lib/AST/ModuleNameLookup.cpp

@@ -258,8 +258,8 @@ void namelookup::lookupInModule(ModuleDecl *startModule,
                                 ArrayRef<ModuleDecl::ImportedModule> extraImports) {
   assert(moduleScopeContext && moduleScopeContext->isModuleScopeContext());
   ModuleLookupCache cache;
-  bool respectAccessControl = startModule->getASTContext().LangOpts
-                                .EnableAccessControl;
+  bool respectAccessControl =
+      !startModule->getASTContext().isAccessControlDisabled();
   ::lookupInModule<CanTypeSet>(startModule, topAccessPath, decls,
                                resolutionKind, /*canReturnEarly=*/true,
                                typeResolver, cache, moduleScopeContext,
@@ -282,7 +282,7 @@ void namelookup::lookupVisibleDeclsInModule(
     ArrayRef<ModuleDecl::ImportedModule> extraImports) {
   assert(moduleScopeContext && moduleScopeContext->isModuleScopeContext());
   ModuleLookupCache cache;
-  bool respectAccessControl = M->getASTContext().LangOpts.EnableAccessControl;
+  bool respectAccessControl = !M->getASTContext().isAccessControlDisabled();
   ::lookupInModule<NamedCanTypeSet>(M, accessPath, decls,
                                     resolutionKind, /*canReturnEarly=*/false,
                                     typeResolver, cache, moduleScopeContext,

lib/AST/NameLookup.cpp

@@ -65,12 +65,11 @@ void DebuggerClient::anchor() {}
 
 void AccessFilteringDeclConsumer::foundDecl(ValueDecl *D,
                                             DeclVisibilityKind reason) {
-  if (D->getASTContext().LangOpts.EnableAccessControl) {
-    if (D->isInvalid())
-      return;
-    if (!D->isAccessibleFrom(DC))
-      return;
-  }
+  if (D->isInvalid())
+    return;
+  if (!D->isAccessibleFrom(DC))
+    return;
+
   ChainedConsumer.foundDecl(D, reason);
 }
 
@@ -1843,7 +1842,7 @@ static void configureLookup(const DeclContext *dc,
                             ReferencedNameTracker *&tracker,
                             bool &isLookupCascading) {
   auto &ctx = dc->getASTContext();
-  if (!ctx.LangOpts.EnableAccessControl)
+  if (ctx.isAccessControlDisabled())
     options |= NL_IgnoreAccessControl;
 
   // Find the dependency tracker we'll need for this lookup.

lib/PrintAsObjC/PrintAsObjC.cpp

@@ -1122,7 +1122,7 @@ class ObjCPrinter : private DeclVisitor<ObjCPrinter>,
 
     ASTContext &ctx = M.getASTContext();
     bool isSettable = VD->isSettable(nullptr);
-    if (isSettable && ctx.LangOpts.EnableAccessControl)
+    if (isSettable && !ctx.isAccessControlDisabled())
       isSettable = (VD->getSetterFormalAccess() >= minRequiredAccess);
     if (!isSettable)
       os << ", readonly";

lib/Sema/CSApply.cpp

@@ -6791,9 +6791,8 @@ static Type adjustSelfTypeForMember(Type baseTy, ValueDecl *member,
   // If the base of the access is mutable, then we may be invoking a getter or
   // setter that requires the base to be mutable.
   auto *SD = cast<AbstractStorageDecl>(member);
-  bool isSettableFromHere = SD->isSettable(UseDC)
-    && (!UseDC->getASTContext().LangOpts.EnableAccessControl
-        || SD->isSetterAccessibleFrom(UseDC));
+  bool isSettableFromHere =
+      SD->isSettable(UseDC) && SD->isSetterAccessibleFrom(UseDC);
 
   // If neither the property's getter nor its setter are mutating, the base
   // can be an rvalue.

lib/Sema/ConstraintSystem.cpp

@@ -715,8 +715,7 @@ static bool doesStorageProduceLValue(TypeChecker &TC,
   if (!storage->isSettable(useDC, base))
     return false;
 
-  if (TC.Context.LangOpts.EnableAccessControl &&
-      !storage->isSetterAccessibleFrom(useDC))
+  if (!storage->isSetterAccessibleFrom(useDC))
     return false;
 
   // If there is no base, or if the base isn't being used, it is settable.

lib/Sema/LookupVisibleDecls.cpp

@@ -129,8 +129,7 @@ static bool isDeclVisibleInLookupMode(ValueDecl *Member, LookupState LS,
 
   // Check access when relevant.
   if (!Member->getDeclContext()->isLocalContext() &&
-      !isa<GenericTypeParamDecl>(Member) && !isa<ParamDecl>(Member) &&
-      FromContext->getASTContext().LangOpts.EnableAccessControl) {
+      !isa<GenericTypeParamDecl>(Member) && !isa<ParamDecl>(Member)) {
     if (!Member->isAccessibleFrom(FromContext))
       return false;
   }

lib/Sema/TypeCheckAccess.cpp

@@ -160,7 +160,7 @@ void AccessControlCheckerBase::checkTypeAccessImpl(
     Type type, TypeRepr *typeRepr, AccessScope contextAccessScope,
     const DeclContext *useDC, bool mayBeInferred,
     llvm::function_ref<CheckTypeAccessCallback> diagnose) {
-  if (!TC.getLangOpts().EnableAccessControl)
+  if (TC.Context.isAccessControlDisabled())
     return;
   if (!type)
     return;

lib/Sema/TypeCheckDecl.cpp

@@ -2775,7 +2775,7 @@ class DeclChecker : public DeclVisitor<DeclChecker> {
         }
       }
 
-      if (TC.getLangOpts().EnableAccessControl) {
+      if (!TC.Context.isAccessControlDisabled()) {
         // Require the superclass to be open if this is outside its
         // defining module.  But don't emit another diagnostic if we
         // already complained about the class being inherently