Merge pull request #15666 from slavapestov/se-0193-default-args

slavapestov · web-flow · commit a4b91d252dc8 · 2018-04-05T19:46:20.000-07:00
SE-0193: Public default argument generators cannot reference declarations that are @usableFromInline
diff --git a/include/swift/AST/DiagnosticsSema.def b/include/swift/AST/DiagnosticsSema.def
@@ -3765,10 +3765,10 @@ ERROR(resilience_decl_unavailable,
 
 #undef FRAGILE_FUNC_KIND
 
-NOTE(resilience_decl_declared_here,
+NOTE(resilience_decl_declared_here_public,
      none, "%0 %1 is not public", (DescriptiveDeclKind, DeclName))
 
-NOTE(resilience_decl_declared_here_versioned,
+NOTE(resilience_decl_declared_here,
      none, "%0 %1 is not '@usableFromInline' or public", (DescriptiveDeclKind, DeclName))
 
 ERROR(class_designated_init_inlinable_resilient,none,
diff --git a/lib/AST/Decl.cpp b/lib/AST/Decl.cpp
@@ -2140,8 +2140,9 @@ static AccessLevel getTestableAccess(const ValueDecl *decl) {
 }
 
 AccessLevel ValueDecl::getEffectiveAccess() const {
-  auto effectiveAccess = getFormalAccess(/*useDC=*/nullptr,
-                                         /*respectVersionedAttr=*/true);
+  auto effectiveAccess =
+    getFormalAccess(/*useDC=*/nullptr,
+                    /*treatUsableFromInlineAsPublic=*/true);
 
   // Handle @testable.
   switch (effectiveAccess) {
diff --git a/lib/AST/DeclContext.cpp b/lib/AST/DeclContext.cpp
@@ -364,8 +364,10 @@ ResilienceExpansion DeclContext::getResilienceExpansion() const {
     // if the type is formally fixed layout.
     if (isa<PatternBindingInitializer>(dc)) {
       if (auto *NTD = dyn_cast<NominalTypeDecl>(dc->getParent())) {
-        if (!NTD->getFormalAccessScope(/*useDC=*/nullptr,
-                                       /*respectVersionedAttr=*/true).isPublic())
+        auto nominalAccess =
+          NTD->getFormalAccessScope(/*useDC=*/nullptr,
+                                    /*treatUsableFromInlineAsPublic=*/true);
+        if (!nominalAccess.isPublic())
           return ResilienceExpansion::Maximal;
 
         if (NTD->isFormallyResilient())
@@ -386,10 +388,13 @@ ResilienceExpansion DeclContext::getResilienceExpansion() const {
       if (!AFD->hasAccess())
         break;
 
+      auto funcAccess =
+        AFD->getFormalAccessScope(/*useDC=*/nullptr,
+                                  /*treatUsableFromInlineAsPublic=*/true);
+
       // If the function is not externally visible, we will not be serializing
       // its body.
-      if (!AFD->getFormalAccessScope(/*useDC=*/nullptr,
-                                     /*respectVersionedAttr=*/true).isPublic())
+      if (!funcAccess.isPublic())
         break;
 
       // Bodies of public transparent and always-inline functions are
diff --git a/lib/SIL/SILDeclRef.cpp b/lib/SIL/SILDeclRef.cpp
@@ -456,8 +456,9 @@ IsSerialized_t SILDeclRef::isSerialized() const {
     // marked as @_fixed_layout.
     if (isStoredPropertyInitializer()) {
       auto *nominal = cast<NominalTypeDecl>(d->getDeclContext());
-      auto scope = nominal->getFormalAccessScope(/*useDC=*/nullptr,
-                                                 /*respectVersionedAttr=*/true);
+      auto scope =
+        nominal->getFormalAccessScope(/*useDC=*/nullptr,
+                                      /*treatUsableFromInlineAsPublic=*/true);
       if (!scope.isPublic())
         return IsNotSerialized;
       if (nominal->isFormallyResilient())
diff --git a/lib/Sema/ResilienceDiagnostics.cpp b/lib/Sema/ResilienceDiagnostics.cpp
@@ -23,13 +23,24 @@
 using namespace swift;
 using FragileFunctionKind = TypeChecker::FragileFunctionKind;
 
-FragileFunctionKind TypeChecker::getFragileFunctionKind(const DeclContext *DC) {
+std::pair<FragileFunctionKind, bool>
+TypeChecker::getFragileFunctionKind(const DeclContext *DC) {
   for (; DC->isLocalContext(); DC = DC->getParent()) {
-    if (isa<DefaultArgumentInitializer>(DC))
-      return FragileFunctionKind::DefaultArgument;
+    if (isa<DefaultArgumentInitializer>(DC)) {
+      // Default argument generators of public functions cannot reference
+      // @usableFromInline declarations; all other fragile function kinds
+      // can.
+      auto *VD = cast<ValueDecl>(DC->getInnermostDeclarationDeclContext());
+      auto access =
+        VD->getFormalAccessScope(/*useDC=*/nullptr,
+                                 /*treatUsableFromInlineAsPublic=*/false);
+      return std::make_pair(FragileFunctionKind::DefaultArgument,
+                            !access.isPublic());
+    }
 
     if (isa<PatternBindingInitializer>(DC))
-      return FragileFunctionKind::PropertyInitializer;
+      return std::make_pair(FragileFunctionKind::PropertyInitializer,
+                            /*treatUsableFromInlineAsPublic=*/true);
 
     if (auto *AFD = dyn_cast<AbstractFunctionDecl>(DC)) {
       // If the function is a nested function, we will serialize its body if
@@ -40,20 +51,24 @@ FragileFunctionKind TypeChecker::getFragileFunctionKind(const DeclContext *DC) {
       // Bodies of public transparent and always-inline functions are
       // serialized, so use conservative access patterns.
       if (AFD->isTransparent())
-        return FragileFunctionKind::Transparent;
+        return std::make_pair(FragileFunctionKind::Transparent,
+                              /*treatUsableFromInlineAsPublic=*/true);
 
       if (AFD->getAttrs().hasAttribute<InlinableAttr>())
-        return FragileFunctionKind::Inlinable;
+        return std::make_pair(FragileFunctionKind::Inlinable,
+                              /*treatUsableFromInlineAsPublic=*/true);
 
       if (auto attr = AFD->getAttrs().getAttribute<InlineAttr>())
         if (attr->getKind() == InlineKind::Always)
-          return FragileFunctionKind::InlineAlways;
+          return std::make_pair(FragileFunctionKind::InlineAlways,
+                                /*treatUsableFromInlineAsPublic=*/true);
 
       // If a property or subscript is @inlinable, the accessors are
       // @inlinable also.
       if (auto accessor = dyn_cast<AccessorDecl>(AFD))
         if (accessor->getStorage()->getAttrs().getAttribute<InlinableAttr>())
-          return FragileFunctionKind::Inlinable;
+          return std::make_pair(FragileFunctionKind::Inlinable,
+                                /*treatUsableFromInlineAsPublic=*/true);
     }
   }
 
@@ -64,21 +79,18 @@ void TypeChecker::diagnoseInlinableLocalType(const NominalTypeDecl *NTD) {
   auto *DC = NTD->getDeclContext();
   auto expansion = DC->getResilienceExpansion();
   if (expansion == ResilienceExpansion::Minimal) {
+    auto kind = getFragileFunctionKind(DC);
     diagnose(NTD, diag::local_type_in_inlinable_function,
              NTD->getFullName(),
-             static_cast<unsigned>(getFragileFunctionKind(DC)));
+             static_cast<unsigned>(kind.first));
   }
 }
 
 bool TypeChecker::diagnoseInlinableDeclRef(SourceLoc loc,
                                            const ValueDecl *D,
-                                           const DeclContext *DC) {
-  auto expansion = DC->getResilienceExpansion();
-
-  // Internal declarations referenced from non-inlinable contexts are OK.
-  if (expansion == ResilienceExpansion::Maximal)
-    return false;
-
+                                           const DeclContext *DC,
+                                           FragileFunctionKind Kind,
+                                           bool TreatUsableFromInlineAsPublic) {
   // Local declarations are OK.
   if (D->getDeclContext()->isLocalContext())
     return false;
@@ -89,7 +101,7 @@ bool TypeChecker::diagnoseInlinableDeclRef(SourceLoc loc,
 
   // Public declarations are OK.
   if (D->getFormalAccessScope(/*useDC=*/nullptr,
-                              /*respectVersionedAttr=*/true).isPublic())
+                              TreatUsableFromInlineAsPublic).isPublic())
     return false;
 
   // Enum cases are handled as part of their containing enum.
@@ -114,23 +126,13 @@ bool TypeChecker::diagnoseInlinableDeclRef(SourceLoc loc,
   diagnose(loc, diag::resilience_decl_unavailable,
            D->getDescriptiveKind(), D->getFullName(),
            D->getFormalAccessScope().accessLevelForDiagnostics(),
-           static_cast<unsigned>(getFragileFunctionKind(DC)));
-
-  bool isDefaultArgument = false;
-  while (DC->isLocalContext()) {
-    if (isa<DefaultArgumentInitializer>(DC)) {
-      isDefaultArgument = true;
-      break;
-    }
-
-    DC = DC->getParent();
-  }
+           static_cast<unsigned>(Kind));
 
-  if (isDefaultArgument) {
+  if (TreatUsableFromInlineAsPublic) {
     diagnose(D, diag::resilience_decl_declared_here,
              D->getDescriptiveKind(), D->getFullName());
   } else {
-    diagnose(D, diag::resilience_decl_declared_here_versioned,
+    diagnose(D, diag::resilience_decl_declared_here_public,
              D->getDescriptiveKind(), D->getFullName());
   }
 
diff --git a/lib/Sema/TypeCheckAvailability.cpp b/lib/Sema/TypeCheckAvailability.cpp
@@ -2233,10 +2233,18 @@ class AvailabilityWalker : public ASTWalker {
   DeclContext *DC;
   MemberAccessContext AccessContext = MemberAccessContext::Getter;
   SmallVector<const Expr *, 16> ExprStack;
+  ResilienceExpansion Expansion;
+  Optional<TypeChecker::FragileFunctionKind> FragileKind;
+  bool TreatUsableFromInlineAsPublic = false;
 
 public:
   AvailabilityWalker(
-      TypeChecker &TC, DeclContext *DC) : TC(TC), DC(DC) {}
+      TypeChecker &TC, DeclContext *DC) : TC(TC), DC(DC) {
+    Expansion = DC->getResilienceExpansion();
+    if (Expansion == ResilienceExpansion::Minimal)
+      std::tie(FragileKind, TreatUsableFromInlineAsPublic)
+        = TC.getFragileFunctionKind(DC);
+  }
 
   std::pair<bool, Expr *> walkToExprPre(Expr *E) override {
     ExprStack.push_back(E);
@@ -2457,9 +2465,11 @@ bool AvailabilityWalker::diagAvailability(const ValueDecl *D, SourceRange R,
       return true;
   }
 
-  if (R.isValid())
-    if (TC.diagnoseInlinableDeclRef(R.Start, D, DC))
-      return true;
+  if (FragileKind)
+    if (R.isValid())
+      if (TC.diagnoseInlinableDeclRef(R.Start, D, DC, *FragileKind,
+                                      TreatUsableFromInlineAsPublic))
+        return true;
 
   if (TC.diagnoseExplicitUnavailability(D, R, DC, call))
     return true;
diff --git a/lib/Sema/TypeCheckStmt.cpp b/lib/Sema/TypeCheckStmt.cpp
@@ -1397,29 +1397,27 @@ static void checkDefaultArguments(TypeChecker &tc,
 /// Check the default arguments that occur within this pattern.
 void TypeChecker::checkDefaultArguments(ArrayRef<ParameterList *> paramLists,
                                         ValueDecl *VD) {
+  auto access =
+    VD->getFormalAccessScope(/*useDC=*/nullptr,
+                             /*treatUsableFromInlineAsPublic=*/true);
+
   // In Swift 4 mode, default argument bodies are inlined into the
   // caller.
   if (auto *func = dyn_cast<AbstractFunctionDecl>(VD)) {
     auto expansion = func->getResilienceExpansion();
-    if (!Context.isSwiftVersion3() &&
-        func->getFormalAccessScope(/*useDC=*/nullptr,
-                                   /*respectVersionedAttr=*/true).isPublic())
+    if (!Context.isSwiftVersion3() && access.isPublic())
       expansion = ResilienceExpansion::Minimal;
 
     func->setDefaultArgumentResilienceExpansion(expansion);
   } else {
     auto *EED = cast<EnumElementDecl>(VD);
-    auto expansion = EED->getParentEnum()->getResilienceExpansion();
-    // Enum payloads parameter lists may have default arguments as of Swift 5.
-    if (Context.isSwiftVersionAtLeast(5) &&
-        EED->getFormalAccessScope(/*useDC=*/nullptr,
-                                  /*respectVersionedAttr=*/true).isPublic())
+    auto expansion = ResilienceExpansion::Maximal;
+    if (access.isPublic())
       expansion = ResilienceExpansion::Minimal;
 
     EED->setDefaultArgumentResilienceExpansion(expansion);
   }
 
-
   unsigned nextArgIndex = 0;
   for (auto *paramList : paramLists)
     ::checkDefaultArguments(*this, paramList, nextArgIndex);
@@ -1671,9 +1669,10 @@ bool TypeChecker::typeCheckConstructorBodyUntil(ConstructorDecl *ctor,
     // declarations.
     if (!isDelegating && ClassD->isResilient() &&
         ctor->getResilienceExpansion() == ResilienceExpansion::Minimal) {
+      auto kind = getFragileFunctionKind(ctor);
       diagnose(ctor, diag::class_designated_init_inlinable_resilient,
                ClassD->getDeclaredInterfaceType(),
-               static_cast<unsigned>(getFragileFunctionKind(ctor)));
+               static_cast<unsigned>(kind.first));
     }
   }
 
diff --git a/lib/Sema/TypeChecker.h b/lib/Sema/TypeChecker.h
@@ -2298,9 +2298,6 @@ class TypeChecker final : public LazyResolver {
 
   void diagnoseInlinableLocalType(const NominalTypeDecl *NTD);
 
-  bool diagnoseInlinableDeclRef(SourceLoc loc, const ValueDecl *D,
-                                const DeclContext *DC);
-
   /// Used in diagnostic %selects.
   enum class FragileFunctionKind : unsigned {
     Transparent,
@@ -2310,11 +2307,20 @@ class TypeChecker final : public LazyResolver {
     PropertyInitializer
   };
 
+  bool diagnoseInlinableDeclRef(SourceLoc loc, const ValueDecl *D,
+                                const DeclContext *DC,
+                                FragileFunctionKind Kind,
+                                bool TreatUsableFromInlineAsPublic);
+
   /// Given that \p DC is within a fragile context for some reason, describe
   /// why.
   ///
+  /// The second element of the pair is true if references to @usableFromInline
+  /// declarations are permitted.
+  ///
   /// \see FragileFunctionKind
-  FragileFunctionKind getFragileFunctionKind(const DeclContext *DC);
+  std::pair<FragileFunctionKind, bool>
+  getFragileFunctionKind(const DeclContext *DC);
 
   /// \name Availability checking
   ///
diff --git a/test/decl/func/default-values-swift4.swift b/test/decl/func/default-values-swift4.swift
@@ -2,16 +2,27 @@
 // RUN: %target-typecheck-verify-swift -swift-version 4 -enable-testing
 
 private func privateFunction() {}
-// expected-note@-1 4{{global function 'privateFunction()' is not public}}
+// expected-note@-1 2{{global function 'privateFunction()' is not public}}
 fileprivate func fileprivateFunction() {}
-// expected-note@-1 4{{global function 'fileprivateFunction()' is not public}}
+// expected-note@-1 2{{global function 'fileprivateFunction()' is not public}}
 func internalFunction() {}
-// expected-note@-1 4{{global function 'internalFunction()' is not public}}
+// expected-note@-1 2{{global function 'internalFunction()' is not public}}
 @usableFromInline func versionedFunction() {}
+// expected-note@-1 5{{global function 'versionedFunction()' is not public}}
 public func publicFunction() {}
 
 func internalIntFunction() -> Int {}
-// expected-note@-1 2{{global function 'internalIntFunction()' is not public}}
+// expected-note@-1 {{global function 'internalIntFunction()' is not public}}
+
+private func privateFunction2() {}
+// expected-note@-1 2{{global function 'privateFunction2()' is not '@usableFromInline' or public}}
+fileprivate func fileprivateFunction2() {}
+// expected-note@-1 2{{global function 'fileprivateFunction2()' is not '@usableFromInline' or public}}
+func internalFunction2() {}
+// expected-note@-1 2{{global function 'internalFunction2()' is not '@usableFromInline' or public}}
+
+func internalIntFunction2() -> Int {}
+// expected-note@-1 {{global function 'internalIntFunction2()' is not '@usableFromInline' or public}}
 
 func internalFunctionWithDefaultValue(
     x: Int = {
@@ -44,17 +55,17 @@ func internalFunctionWithDefaultValue(
       // OK
       versionedFunction()
       // OK
-      internalFunction()
-      // expected-error@-1 2{{global function 'internalFunction()' is internal and cannot be referenced from a default argument value}}
-      fileprivateFunction()
-      // expected-error@-1 2{{global function 'fileprivateFunction()' is fileprivate and cannot be referenced from a default argument value}}
-      privateFunction()
-      // expected-error@-1 2{{global function 'privateFunction()' is private and cannot be referenced from a default argument value}}
+      internalFunction2()
+      // expected-error@-1 2{{global function 'internalFunction2()' is internal and cannot be referenced from a default argument value}}
+      fileprivateFunction2()
+      // expected-error@-1 2{{global function 'fileprivateFunction2()' is fileprivate and cannot be referenced from a default argument value}}
+      privateFunction2()
+      // expected-error@-1 2{{global function 'privateFunction2()' is private and cannot be referenced from a default argument value}}
 
       return 0
     }(),
-    y: Int = internalIntFunction()) {}
-    // expected-error@-1 {{global function 'internalIntFunction()' is internal and cannot be referenced from a default argument value}}
+    y: Int = internalIntFunction2()) {}
+    // expected-error@-1 {{global function 'internalIntFunction2()' is internal and cannot be referenced from a default argument value}}
 
 public func publicFunctionWithDefaultValue(
     x: Int = {
@@ -64,13 +75,16 @@ public func publicFunctionWithDefaultValue(
       // FIXME: Some errors below are diagnosed twice
 
       publicFunction()
-      // OK
+
       versionedFunction()
-      // OK
+      // expected-error@-1 2{{global function 'versionedFunction()' is internal and cannot be referenced from a default argument value}}
+
       internalFunction()
       // expected-error@-1 2{{global function 'internalFunction()' is internal and cannot be referenced from a default argument value}}
+
       fileprivateFunction()
       // expected-error@-1 2{{global function 'fileprivateFunction()' is fileprivate and cannot be referenced from a default argument value}}
+
       privateFunction()
       // expected-error@-1 2{{global function 'privateFunction()' is private and cannot be referenced from a default argument value}}
 
@@ -83,3 +97,16 @@ public func publicFunctionWithDefaultValue(
 public class MyClass {
   public func method<T>(_: T.Type = T.self) -> T { }
 }
+
+public func evilCode(
+  x: Int = {
+    let _ = publicFunction()
+    let _ = versionedFunction()
+    // expected-error@-1 2{{global function 'versionedFunction()' is internal and cannot be referenced from a default argument value}}
+
+    func localFunction() {
+      publicFunction()
+      versionedFunction()
+      // expected-error@-1 {{global function 'versionedFunction()' is internal and cannot be referenced from a default argument value}}
+    }
+  }()) {}
