Simulate buggy Swift 3 function-type-alias access checking. (#6418)

jrose-apple · web-flow · commit c2210991ca3d · 2016-12-20T13:31:17.000-08:00
Like 9fba89b, typealiases in parameter position were sometimes canonicalized away in Swift 3.0, leading to the compiler not diagnosing improper uses of typealiases. These only occurred in "safe" circumstances where the typealias wouldn't be leaked to clients of a library (that is, a client would never see a name they wouldn't be allowed to type), but it was inconsistent with other typealiases (which were preserved) and did not follow the intended model. This particular issue comes from typealiases for function types needing to be marked as non-escaping in the compiler when used in parameter position, which requires rebuilding the function type. This lost the original "spelling" of the parameter as using a typealias in its type, which meant the compiler did not see it. The change in a32e11a to use TypeReprs to check access meant the model was now correctly enforced, but broke source compatibility for this "accidental feature". The fix is to track when a typealias for a function type is being used in parameter position, and to not check its access in that case. rdar://problem/29739577
diff --git a/lib/Sema/TypeCheckDecl.cpp b/lib/Sema/TypeCheckDecl.cpp
@@ -1145,7 +1145,7 @@ class AccessScopeChecker {
         Cache(caches[File]),
         Scope(AccessScope::getPublic()) {}
 
-  bool visitDecl(ValueDecl *VD) {
+  bool visitDecl(ValueDecl *VD, bool isInParameter = false) {
     if (!VD || isa<GenericTypeParamDecl>(VD))
       return true;
 
@@ -1156,11 +1156,22 @@ class AccessScopeChecker {
         return true;
     }
 
-    // Simulation for Swift 3 bug.
-    if (isa<TypeAliasDecl>(VD) &&
-        VD->getInterfaceType()->hasTypeParameter() &&
-        Context.isSwiftVersion3())
-      return true;
+    // Simulation for Swift 3 bugs where typealiases got canonicalized away and
+    // thus a reference to a private typealias would not be diagnosed.
+    if (auto *TAD = dyn_cast<TypeAliasDecl>(VD)) {
+      if (Context.isSwiftVersion3()) {
+        // - If the typealias was a dependent type, Swift 3 resolved it down to
+        //   its underlying type.
+        if (VD->getInterfaceType()->hasTypeParameter())
+          return true;
+        // - If the typealias was a function type in parameter position, Swift 3
+        //   would rebuild the type to mark it non-escaping, losing the sugar.
+        if (isInParameter &&
+            TAD->getDeclaredInterfaceType()->getAs<AnyFunctionType>()) {
+          return true;
+        }
+      }
+    }
 
     auto cached = Cache.find(VD);
     if (cached != Cache.end()) {
@@ -1179,27 +1190,52 @@ class AccessScopeChecker {
 };
 
 class TypeReprAccessScopeChecker : private ASTWalker, AccessScopeChecker {
+  SmallVector<const TypeRepr *, 4> ParamParents;
+
   TypeReprAccessScopeChecker(const DeclContext *useDC,
-                             decltype(TypeChecker::TypeAccessScopeCache) &caches)
-      : AccessScopeChecker(useDC, caches) {}
+                             decltype(TypeChecker::TypeAccessScopeCache) &caches,
+                             bool isParameter)
+      : AccessScopeChecker(useDC, caches) {
+    if (isParameter)
+      ParamParents.push_back(nullptr);
+  }
+
+  bool isParamParent(const TypeRepr *TR) const {
+    return !ParamParents.empty() && ParamParents.back() == TR;
+  }
 
   bool walkToTypeReprPre(TypeRepr *TR) override {
-    auto CITR = dyn_cast<ComponentIdentTypeRepr>(TR);
-    if (!CITR)
-      return true;
+    if (auto CITR = dyn_cast<ComponentIdentTypeRepr>(TR)) {
+      return visitDecl(CITR->getBoundDecl(),
+                       isParamParent(Parent.getAsTypeRepr()));
+    }
+
+    auto parentTR = Parent.getAsTypeRepr();
+    if (auto parentFn = dyn_cast_or_null<FunctionTypeRepr>(parentTR)) {
+      // The argument tuple for a function is a parent of parameter TRs.
+      if (parentFn->getArgsTypeRepr() == TR)
+        ParamParents.push_back(TR);
+    } else if (isa<CompoundIdentTypeRepr>(TR) && isParamParent(parentTR)) {
+      // Compound TRs that would have been parameter TRs contain parameter
+      // TRs.
+      ParamParents.push_back(TR);
+    }
 
-    return visitDecl(CITR->getBoundDecl());
+    return true;
   }
 
   bool walkToTypeReprPost(TypeRepr *TR) override {
+    if (isParamParent(TR))
+      ParamParents.pop_back();
     return Scope.hasValue();
   }
 
 public:
   static Optional<AccessScope>
   getAccessScope(TypeRepr *TR, const DeclContext *useDC,
-                 decltype(TypeChecker::TypeAccessScopeCache) &caches) {
-    TypeReprAccessScopeChecker checker(useDC, caches);
+                 decltype(TypeChecker::TypeAccessScopeCache) &caches,
+                 bool isParameter) {
+    TypeReprAccessScopeChecker checker(useDC, caches, isParameter);
     TR->walk(checker);
     return checker.Scope;
   }
@@ -1262,7 +1298,8 @@ void TypeChecker::computeDefaultAccessibility(ExtensionDecl *ED) {
       auto accessScope =
           TypeReprAccessScopeChecker::getAccessScope(TL.getTypeRepr(),
                                                      ED->getDeclContext(),
-                                                     TypeAccessScopeCache);
+                                                     TypeAccessScopeCache,
+                                                     /*isParameter*/false);
       // This is an error case and will be diagnosed elsewhere.
       if (!accessScope.hasValue())
         return Accessibility::Public;
@@ -1475,7 +1512,7 @@ enum class DowngradeToWarning: bool {
 /// is never null.
 static void checkTypeAccessibilityImpl(
     TypeChecker &TC, TypeLoc TL, AccessScope contextAccessScope,
-    const DeclContext *useDC,
+    const DeclContext *useDC, bool isParameter,
     llvm::function_ref<void(AccessScope, const TypeRepr *)> diagnose) {
   if (!TC.getLangOpts().EnableAccessControl)
     return;
@@ -1493,7 +1530,8 @@ static void checkTypeAccessibilityImpl(
   auto typeAccessScope =
     (TL.getTypeRepr()
      ? TypeReprAccessScopeChecker::getAccessScope(TL.getTypeRepr(), useDC,
-                                                  TC.TypeAccessScopeCache)
+                                                  TC.TypeAccessScopeCache,
+                                                  isParameter)
      : TypeAccessScopeChecker::getAccessScope(TL.getType(), useDC,
                                               TC.TypeAccessScopeCache));
 
@@ -1528,9 +1566,18 @@ static void checkTypeAccessibility(
     TypeChecker &TC, TypeLoc TL, const ValueDecl *context,
     llvm::function_ref<void(AccessScope, const TypeRepr *,
                             DowngradeToWarning)> diagnose) {
+  const DeclContext *DC = context->getDeclContext();
+  bool isParam = false;
+  if (auto *param = dyn_cast<ParamDecl>(context)) {
+    isParam = true;
+    context = dyn_cast<AbstractFunctionDecl>(DC);
+    if (!context)
+      context = cast<SubscriptDecl>(DC);
+    DC = context->getDeclContext();
+  }
+
   AccessScope contextAccessScope = context->getFormalAccessScope();
-  checkTypeAccessibilityImpl(TC, TL, contextAccessScope,
-                             context->getDeclContext(),
+  checkTypeAccessibilityImpl(TC, TL, contextAccessScope, DC, isParam,
                              [=](AccessScope requiredAccessScope,
                                  const TypeRepr *offendingTR) {
     auto downgradeToWarning = DowngradeToWarning::No;
@@ -1591,6 +1638,7 @@ static void checkGenericParamAccessibility(TypeChecker &TC,
     assert(param->getInherited().size() == 1);
     checkTypeAccessibilityImpl(TC, param->getInherited().front(), accessScope,
                                owner->getDeclContext(),
+                               /*isParameter*/false,
                                [&](AccessScope typeAccessScope,
                                    const TypeRepr *thisComplainRepr) {
       if (typeAccessScope.isChildOf(minAccessScope) ||
@@ -1618,18 +1666,18 @@ static void checkGenericParamAccessibility(TypeChecker &TC,
     case RequirementReprKind::TypeConstraint:
       checkTypeAccessibilityImpl(TC, requirement.getSubjectLoc(),
                                  accessScope, owner->getDeclContext(),
-                                 callback);
+                                 /*isParameter*/false, callback);
       checkTypeAccessibilityImpl(TC, requirement.getConstraintLoc(),
                                  accessScope, owner->getDeclContext(),
-                                 callback);
+                                 /*isParameter*/false, callback);
       break;
     case RequirementReprKind::SameType:
       checkTypeAccessibilityImpl(TC, requirement.getFirstTypeLoc(),
                                  accessScope, owner->getDeclContext(),
-                                 callback);
+                                 /*isParameter*/false, callback);
       checkTypeAccessibilityImpl(TC, requirement.getSecondTypeLoc(),
                                  accessScope, owner->getDeclContext(),
-                                 callback);
+                                 /*isParameter*/false, callback);
       break;
     }
   }
@@ -1975,7 +2023,7 @@ static void checkAccessibility(TypeChecker &TC, const Decl *D) {
     bool problemIsElement = false;
 
     for (auto &P : *SD->getIndices()) {
-      checkTypeAccessibility(TC, P->getTypeLoc(), SD,
+      checkTypeAccessibility(TC, P->getTypeLoc(), P,
                              [&](AccessScope typeAccessScope,
                                  const TypeRepr *thisComplainRepr,
                                  DowngradeToWarning downgradeDiag) {
@@ -2047,7 +2095,7 @@ static void checkAccessibility(TypeChecker &TC, const Decl *D) {
 
     for (auto *PL : fn->getParameterLists().slice(isTypeContext)) {
       for (auto &P : *PL) {
-        checkTypeAccessibility(TC, P->getTypeLoc(), fn,
+        checkTypeAccessibility(TC, P->getTypeLoc(), P,
                                [&](AccessScope typeAccessScope,
                                    const TypeRepr *thisComplainRepr,
                                    DowngradeToWarning downgradeDiag) {
diff --git a/test/Compatibility/accessibility_typealias.swift b/test/Compatibility/accessibility_typealias.swift
@@ -1,4 +1,4 @@
-// RUN: %target-typecheck-verify-swift -swift-version 3
+// RUN: %target-typecheck-verify-swift -swift-version 3 -module-name main
 
 public protocol P {
   associatedtype Element
@@ -40,3 +40,52 @@ private func privateFuncWithFileprivateAlias() -> Generic<Int>.Dependent {
 }
 
 var y = privateFuncWithFileprivateAlias()
+
+
+private typealias FnType = (_ x: Int) -> Void // expected-note * {{type declared here}}
+
+public var fn1: (FnType) -> Void = { _ in }
+public var fn2: (_ x: FnType) -> Void = { _ in }
+public var fn3: (main.FnType) -> Void = { _ in }
+public var fn4: (_ x: main.FnType) -> Void = { _ in }
+public var nested1: (_ x: (FnType) -> Void) -> Void = { _ in }
+public var nested2: (_ x: (main.FnType) -> Void) -> Void = { _ in }
+public func test1(x: FnType) {}
+public func test2(x: main.FnType) {}
+
+
+public func reject1(x: FnType?) {} // expected-error {{cannot be declared public}}
+public func reject2(x: main.FnType?) {} // expected-error {{cannot be declared public}}
+public func reject3() -> FnType { fatalError() } // expected-error {{cannot be declared public}}
+public func reject4() -> main.FnType { fatalError() } // expected-error {{cannot be declared public}}
+public var rejectVar1: FnType = {_ in } // expected-error {{cannot be declared public}}
+public var rejectVar2: main.FnType = {_ in } // expected-error {{cannot be declared public}}
+public var rejectVar3: FnType? // expected-error {{cannot be declared public}}
+public var rejectVar4: main.FnType? // expected-error {{cannot be declared public}}
+public var escaping1: (@escaping FnType) -> Void = { _ in } // expected-error {{cannot be declared public}}
+public var escaping2: (_ x: @escaping FnType) -> Void = { _ in } // expected-error {{cannot be declared public}}
+public var escaping3: (@escaping main.FnType) -> Void = { _ in } // expected-error {{cannot be declared public}}
+public var escaping4: (_ x: @escaping main.FnType) -> Void = { _ in } // expected-error {{cannot be declared public}}
+
+public struct SubscriptTest {
+  public subscript(test1 x: FnType) -> () { return }
+  public subscript(test2 x: main.FnType) -> () { return }
+
+  public subscript(reject1 x: FnType?) -> () { return } // expected-error {{cannot be declared public}}
+  public subscript(reject2 x: main.FnType?) -> () { return } // expected-error {{cannot be declared public}}
+  public subscript(reject3 x: Int) -> FnType { fatalError() } // expected-error {{cannot be declared public}}
+  public subscript(reject4 x: Int) -> main.FnType { fatalError() } // expected-error {{cannot be declared public}}
+}
+
+private struct ActuallyPrivate {} // expected-note * {{declared here}}
+private typealias ActuallyPrivateAlias = ActuallyPrivate
+
+public var failFn: (ActuallyPrivate) -> Void = { _ in } // expected-error {{cannot be declared public}}
+public var failFn2: (_ x: ActuallyPrivate) -> Void = { _ in } // expected-error {{cannot be declared public}}
+public var failFn3: (main.ActuallyPrivate) -> Void = { _ in } // expected-error {{cannot be declared public}}
+public var failFn4: (_ x: main.ActuallyPrivate) -> Void = { _ in } // expected-error {{cannot be declared public}}
+public var failNested1: (_ x: (ActuallyPrivate) -> Void) -> Void = { _ in } // expected-error {{cannot be declared public}}
+public var failNested2: (_ x: (main.ActuallyPrivate) -> Void) -> Void = { _ in } // expected-error {{cannot be declared public}}
+public func failTest(x: ActuallyPrivate) {} // expected-error {{cannot be declared public}}
+public func failTest2(x: main.ActuallyPrivate) {} // expected-error {{cannot be declared public}}
+
diff --git a/test/Sema/accessibility_typealias.swift b/test/Sema/accessibility_typealias.swift
@@ -1,4 +1,4 @@
-// RUN: %target-typecheck-verify-swift -swift-version 4
+// RUN: %target-typecheck-verify-swift -swift-version 4 -module-name main
 
 public protocol P {
   associatedtype Element
@@ -44,3 +44,52 @@ private func privateFuncWithFileprivateAlias() -> Generic<Int>.Dependent {
 
 // FIXME: No error here
 var y = privateFuncWithFileprivateAlias()
+
+
+private typealias FnType = (_ x: Int) -> Void // expected-note * {{type declared here}}
+
+public var fn1: (FnType) -> Void = { _ in } // expected-error {{cannot be declared public}}
+public var fn2: (_ x: FnType) -> Void = { _ in } // expected-error {{cannot be declared public}}
+public var fn3: (main.FnType) -> Void = { _ in } // expected-error {{cannot be declared public}}
+public var fn4: (_ x: main.FnType) -> Void = { _ in } // expected-error {{cannot be declared public}}
+public var nested1: (_ x: (FnType) -> Void) -> Void = { _ in } // expected-error {{cannot be declared public}}
+public var nested2: (_ x: (main.FnType) -> Void) -> Void = { _ in } // expected-error {{cannot be declared public}}
+public func test1(x: FnType) {} // expected-error {{cannot be declared public}}
+public func test2(x: main.FnType) {} // expected-error {{cannot be declared public}}
+
+
+public func reject1(x: FnType?) {} // expected-error {{cannot be declared public}}
+public func reject2(x: main.FnType?) {} // expected-error {{cannot be declared public}}
+public func reject3() -> FnType { fatalError() } // expected-error {{cannot be declared public}}
+public func reject4() -> main.FnType { fatalError() } // expected-error {{cannot be declared public}}
+public var rejectVar1: FnType = {_ in } // expected-error {{cannot be declared public}}
+public var rejectVar2: main.FnType = {_ in } // expected-error {{cannot be declared public}}
+public var rejectVar3: FnType? // expected-error {{cannot be declared public}}
+public var rejectVar4: main.FnType? // expected-error {{cannot be declared public}}
+public var escaping1: (@escaping FnType) -> Void = { _ in } // expected-error {{cannot be declared public}}
+public var escaping2: (_ x: @escaping FnType) -> Void = { _ in } // expected-error {{cannot be declared public}}
+public var escaping3: (@escaping main.FnType) -> Void = { _ in } // expected-error {{cannot be declared public}}
+public var escaping4: (_ x: @escaping main.FnType) -> Void = { _ in } // expected-error {{cannot be declared public}}
+
+public struct SubscriptTest {
+  public subscript(test1 x: FnType) -> () { return } // expected-error {{cannot be declared public}}
+  public subscript(test2 x: main.FnType) -> () { return } // expected-error {{cannot be declared public}}
+
+  public subscript(reject1 x: FnType?) -> () { return } // expected-error {{cannot be declared public}}
+  public subscript(reject2 x: main.FnType?) -> () { return } // expected-error {{cannot be declared public}}
+  public subscript(reject3 x: Int) -> FnType { fatalError() } // expected-error {{cannot be declared public}}
+  public subscript(reject4 x: Int) -> main.FnType { fatalError() } // expected-error {{cannot be declared public}}
+}
+
+private struct ActuallyPrivate {} // expected-note * {{declared here}}
+private typealias ActuallyPrivateAlias = ActuallyPrivate
+
+public var failFn: (ActuallyPrivate) -> Void = { _ in } // expected-error {{cannot be declared public}}
+public var failFn2: (_ x: ActuallyPrivate) -> Void = { _ in } // expected-error {{cannot be declared public}}
+public var failFn3: (main.ActuallyPrivate) -> Void = { _ in } // expected-error {{cannot be declared public}}
+public var failFn4: (_ x: main.ActuallyPrivate) -> Void = { _ in } // expected-error {{cannot be declared public}}
+public var failNested1: (_ x: (ActuallyPrivate) -> Void) -> Void = { _ in } // expected-error {{cannot be declared public}}
+public var failNested2: (_ x: (main.ActuallyPrivate) -> Void) -> Void = { _ in } // expected-error {{cannot be declared public}}
+public func failTest(x: ActuallyPrivate) {} // expected-error {{cannot be declared public}}
+public func failTest2(x: main.ActuallyPrivate) {} // expected-error {{cannot be declared public}}
+
