Merge pull request swiftlang#34390 from slavapestov/misc-availability-fixes

Another pile of availability checking fixes

Author: slavapestov

include/swift/AST/Decl.h

@@ -4763,11 +4763,19 @@ class VarDecl : public AbstractStorageDecl {
 
   /// Determines if this var has an initializer expression that should be
   /// exposed to clients.
+  ///
   /// There's a very narrow case when we would: if the decl is an instance
   /// member with an initializer expression and the parent type is
   /// @frozen and resides in a resilient module.
   bool isInitExposedToClients() const;
-  
+
+  /// Determines if this var is exposed as part of the layout of a
+  /// @frozen struct.
+  ///
+  /// From the standpoint of access control and exportability checking, this
+  /// var will behave as if it was public, even if it is internal or private.
+  bool isLayoutExposedToClients() const;
+
   /// Is this a special debugger variable?
   bool isDebuggerVar() const { return Bits.VarDecl.IsDebuggerVar; }
   void setDebuggerVar(bool IsDebuggerVar) {

include/swift/AST/DiagnosticsSema.def

@@ -5061,12 +5061,12 @@ ERROR(local_type_in_inlinable_function,
       (Identifier, unsigned))
 
 ERROR(resilience_decl_unavailable,
-      none, DECL_OR_ACCESSOR "4 %1 is %select{private|fileprivate|internal|'@_spi'|'@_spi'}2 and "
+      none, DECL_OR_ACCESSOR "4 %1 is %select{private|fileprivate|internal|%error|%error}2 and "
       "cannot be referenced from " FRAGILE_FUNC_KIND "3",
       (DescriptiveDeclKind, DeclName, AccessLevel, unsigned, bool))
 
 WARNING(resilience_decl_unavailable_warn,
-        none, DECL_OR_ACCESSOR "4 %1 is %select{private|fileprivate|internal|'@_spi'|'@_spi'}2 and "
+        none, DECL_OR_ACCESSOR "4 %1 is %select{private|fileprivate|internal|%error|%error}2 and "
         "should not be referenced from " FRAGILE_FUNC_KIND "3",
         (DescriptiveDeclKind, DeclName, AccessLevel, unsigned, bool))
 

lib/AST/Decl.cpp

@@ -1582,12 +1582,32 @@ VarDecl *PatternBindingDecl::getAnchoringVarDecl(unsigned i) const {
 }
 
 bool VarDecl::isInitExposedToClients() const {
+  // 'lazy' initializers are emitted inside the getter, which is never
+  // @inlinable.
+  if (getAttrs().hasAttribute<LazyAttr>())
+    return false;
+
+  return hasInitialValue() && isLayoutExposedToClients();
+}
+
+bool VarDecl::isLayoutExposedToClients() const {
   auto parent = dyn_cast<NominalTypeDecl>(getDeclContext());
   if (!parent) return false;
-  if (!hasInitialValue()) return false;
   if (isStatic()) return false;
-  return parent->getAttrs().hasAttribute<FrozenAttr>() ||
-         parent->getAttrs().hasAttribute<FixedLayoutAttr>();
+
+  if (!hasStorage() &&
+      !getAttrs().hasAttribute<LazyAttr>() &&
+      !hasAttachedPropertyWrapper()) {
+    return false;
+  }
+
+  auto nominalAccess =
+    parent->getFormalAccessScope(/*useDC=*/nullptr,
+                                 /*treatUsableFromInlineAsPublic=*/true);
+  if (!nominalAccess.isPublic()) return false;
+
+  return (parent->getAttrs().hasAttribute<FrozenAttr>() ||
+          parent->getAttrs().hasAttribute<FixedLayoutAttr>());
 }
 
 /// Check whether the given type representation will be

lib/AST/DeclContext.cpp

@@ -368,21 +368,17 @@ swift::FragileFunctionKindRequest::evaluate(Evaluator &evaluator,
 
     // Stored property initializer contexts use minimal resilience expansion
     // if the type is formally fixed layout.
-    if (isa<PatternBindingInitializer>(dc)) {
-      if (auto *NTD = dyn_cast<NominalTypeDecl>(dc->getParent())) {
-        auto nominalAccess =
-          NTD->getFormalAccessScope(/*useDC=*/nullptr,
-                                    /*treatUsableFromInlineAsPublic=*/true);
-        if (!nominalAccess.isPublic())
-          return {FragileFunctionKind::None,
-                  /*allowUsableFromInline=*/false};
-
-        if (NTD->isFormallyResilient())
-          return {FragileFunctionKind::None,
-                  /*allowUsableFromInline=*/false};
-
-        return {FragileFunctionKind::PropertyInitializer, true};
+    if (auto *init = dyn_cast <PatternBindingInitializer>(dc)) {
+      auto bindingIndex = init->getBindingIndex();
+      if (auto *varDecl = init->getBinding()->getAnchoringVarDecl(bindingIndex)) {
+        if (varDecl->isInitExposedToClients()) {
+          return {FragileFunctionKind::PropertyInitializer,
+                  /*allowUsableFromInline=*/true};
+        }
       }
+
+      return {FragileFunctionKind::None,
+              /*allowUsableFromInline=*/false};
     }
 
     if (auto *AFD = dyn_cast<AbstractFunctionDecl>(dc)) {
@@ -434,7 +430,8 @@ swift::FragileFunctionKindRequest::evaluate(Evaluator &evaluator,
     }
   }
 
-  return {FragileFunctionKind::None, false};
+  return {FragileFunctionKind::None,
+          /*allowUsableFromInline=*/false};
 }
 
 /// Determine whether the innermost context is generic.

lib/Sema/ResilienceDiagnostics.cpp

@@ -27,48 +27,24 @@
 
 using namespace swift;
 
-bool TypeChecker::diagnoseInlinableDeclRef(SourceLoc loc,
-                                           const ValueDecl *D,
-                                           ExportContext where) {
-  auto fragileKind = where.getFragileFunctionKind();
-  if (fragileKind.kind == FragileFunctionKind::None)
-    return false;
-
-  // Do some important fast-path checks that apply to all cases.
-
-  // Type parameters are OK.
-  if (isa<AbstractTypeParamDecl>(D))
-    return false;
-
-  // Check whether the declaration is accessible.
-  if (diagnoseInlinableDeclRefAccess(loc, D, where))
-    return true;
-
-  // Check whether the declaration comes from a publically-imported module.
-  // Skip this check for accessors because the associated property or subscript
-  // will also be checked, and will provide a better error message.
-  if (!isa<AccessorDecl>(D))
-    if (diagnoseDeclRefExportability(loc, D, where))
-      return true;
-
-  return false;
-}
-
 bool TypeChecker::diagnoseInlinableDeclRefAccess(SourceLoc loc,
                                                  const ValueDecl *D,
                                                  ExportContext where) {
-  auto *DC = where.getDeclContext();
   auto fragileKind = where.getFragileFunctionKind();
-  assert(fragileKind.kind != FragileFunctionKind::None);
+  if (fragileKind.kind == FragileFunctionKind::None)
+    return false;
 
   // Local declarations are OK.
   if (D->getDeclContext()->isLocalContext())
     return false;
 
-  // Public declarations or SPI used from SPI are OK.
+  auto *DC = where.getDeclContext();
+
+  // Public declarations are OK, even if they're SPI or came from an
+  // implementation-only import. We'll diagnose exportability violations
+  // from diagnoseDeclRefExportability().
   if (D->getFormalAccessScope(/*useDC=*/nullptr,
-                              fragileKind.allowUsableFromInline).isPublic() &&
-      !(D->isSPI() && !DC->getInnermostDeclarationDeclContext()->isSPI()))
+                              fragileKind.allowUsableFromInline).isPublic())
     return false;
 
   auto &Context = DC->getASTContext();
@@ -81,14 +57,6 @@ bool TypeChecker::diagnoseInlinableDeclRefAccess(SourceLoc loc,
     return false;
   }
 
-  // Property initializers that are not exposed to clients are OK.
-  if (auto pattern = dyn_cast<PatternBindingInitializer>(DC)) {
-    auto bindingIndex = pattern->getBindingIndex();
-    auto *varDecl = pattern->getBinding()->getAnchoringVarDecl(bindingIndex);
-    if (!varDecl->isInitExposedToClients())
-      return false;
-  }
-
   DowngradeToWarning downgradeToWarning = DowngradeToWarning::No;
 
   // Swift 4.2 did not perform any checks for type aliases.
@@ -145,6 +113,11 @@ bool
 TypeChecker::diagnoseDeclRefExportability(SourceLoc loc,
                                           const ValueDecl *D,
                                           ExportContext where) {
+  // Accessors cannot have exportability that's different than the storage,
+  // so skip them for now.
+  if (isa<AccessorDecl>(D))
+    return false;
+
   if (!where.mustOnlyReferenceExportedDecls())
     return false;
 

lib/Sema/TypeCheckAccess.cpp

@@ -1051,29 +1051,26 @@ class UsableFromInlineChecker : public AccessControlCheckerBase,
   UNINTERESTING(Accessor) // Handled by the Var or Subscript.
   UNINTERESTING(OpaqueType) // Handled by the Var or Subscript.
 
-  /// If \p PBD declared stored instance properties in a fixed-contents struct,
-  /// return said struct.
-  static const StructDecl *
-  getFixedLayoutStructContext(const PatternBindingDecl *PBD) {
-    auto *parentStruct = dyn_cast<StructDecl>(PBD->getDeclContext());
-    if (!parentStruct)
-      return nullptr;
-    if (!(parentStruct->getAttrs().hasAttribute<FrozenAttr>() ||         
-          parentStruct->getAttrs().hasAttribute<FixedLayoutAttr>()) ||
-        PBD->isStatic() || !PBD->hasStorage()) {
-      return nullptr;
-    }
-    // We don't check for "in resilient modules" because there's no reason to
-    // write '@_fixedLayout' on a struct in a non-resilient module.
-    return parentStruct;
+  /// If \p VD's layout is exposed by a @frozen struct or class, return said
+  /// struct or class.
+  ///
+  /// Stored instance properties in @frozen structs and classes must always use
+  /// public/@usableFromInline types. In these cases, check the access against
+  /// the struct instead of the VarDecl, and customize the diagnostics.
+  static const ValueDecl *
+  getFixedLayoutStructContext(const VarDecl *VD) {
+    if (VD->isLayoutExposedToClients())
+      return dyn_cast<NominalTypeDecl>(VD->getDeclContext());
+
+    return nullptr;
   }
 
   /// \see visitPatternBindingDecl
   void checkNamedPattern(const NamedPattern *NP,
-                         const ValueDecl *fixedLayoutStructContext,
                          bool isTypeContext,
                          const llvm::DenseSet<const VarDecl *> &seenVars) {
     const VarDecl *theVar = NP->getDecl();
+    auto *fixedLayoutStructContext = getFixedLayoutStructContext(theVar);
     if (!fixedLayoutStructContext && shouldSkipChecking(theVar))
       return;
     // Only check individual variables if we didn't check an enclosing
@@ -1101,7 +1098,6 @@ class UsableFromInlineChecker : public AccessControlCheckerBase,
 
   /// \see visitPatternBindingDecl
   void checkTypedPattern(const TypedPattern *TP,
-                         const ValueDecl *fixedLayoutStructContext,
                          bool isTypeContext,
                          llvm::DenseSet<const VarDecl *> &seenVars) {
     // FIXME: We need an access level to check against, so we pull one out
@@ -1114,6 +1110,7 @@ class UsableFromInlineChecker : public AccessControlCheckerBase,
     });
     if (!anyVar)
       return;
+    auto *fixedLayoutStructContext = getFixedLayoutStructContext(anyVar);
     if (!fixedLayoutStructContext && shouldSkipChecking(anyVar))
       return;
 
@@ -1154,27 +1151,18 @@ class UsableFromInlineChecker : public AccessControlCheckerBase,
   void visitPatternBindingDecl(PatternBindingDecl *PBD) {
     bool isTypeContext = PBD->getDeclContext()->isTypeContext();
 
-    // Stored instance properties in public/@usableFromInline fixed-contents
-    // structs in resilient modules must always use public/@usableFromInline
-    // types. In these cases, check the access against the struct instead of the
-    // VarDecl, and customize the diagnostics.
-    const ValueDecl *fixedLayoutStructContext =
-        getFixedLayoutStructContext(PBD);
-
     llvm::DenseSet<const VarDecl *> seenVars;
     for (auto idx : range(PBD->getNumPatternEntries())) {
       PBD->getPattern(idx)->forEachNode([&](const Pattern *P) {
         if (auto *NP = dyn_cast<NamedPattern>(P)) {
-          checkNamedPattern(NP, fixedLayoutStructContext, isTypeContext,
-                            seenVars);
+          checkNamedPattern(NP, isTypeContext, seenVars);
           return;
         }
 
         auto *TP = dyn_cast<TypedPattern>(P);
         if (!TP)
           return;
-        checkTypedPattern(TP, fixedLayoutStructContext, isTypeContext,
-                          seenVars);
+        checkTypedPattern(TP, isTypeContext, seenVars);
       });
       seenVars.clear();
     }

lib/Sema/TypeCheckAttr.cpp

@@ -967,14 +967,15 @@ void AttributeChecker::visitSPIAccessControlAttr(SPIAccessControlAttr *attr) {
     }
 
     // Forbid stored properties marked SPI in frozen types.
-    if (auto property = dyn_cast<AbstractStorageDecl>(VD))
-      if (auto DC = dyn_cast<NominalTypeDecl>(D->getDeclContext()))
-        if (property->hasStorage() &&
-            !DC->isFormallyResilient() &&
-            !DC->isSPI())
+    if (auto property = dyn_cast<VarDecl>(VD)) {
+      if (auto NTD = dyn_cast<NominalTypeDecl>(D->getDeclContext())) {
+        if (property->isLayoutExposedToClients() && !NTD->isSPI()) {
           diagnoseAndRemoveAttr(attr,
                                 diag::spi_attribute_on_frozen_stored_properties,
                                 VD->getName());
+        }
+      }
+    }
   }
 }