Merge pull request #85179 from xymus/exportability-nle-structs

Sema: Opt-in check for structs references to hidden dependencies in non-library-evolution mode

Author: xymus

include/swift/AST/Decl.h

@@ -6727,7 +6727,10 @@ class VarDecl : public AbstractStorageDecl {
   ///
   /// 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;
+  ///
+  /// If \p applyImplicit, consider implicitly exposed layouts as well.
+  /// This applies to non-resilient modules.
+  bool isLayoutExposedToClients(bool applyImplicit = false) const;
 
   /// Is this a special debugger variable?
   bool isDebuggerVar() const { return Bits.VarDecl.IsDebuggerVar; }

include/swift/AST/DeclAttr.def

@@ -474,7 +474,7 @@ SIMPLE_DECL_ATTR(_alwaysEmitIntoClient, AlwaysEmitIntoClient,
   83)
 
 SIMPLE_DECL_ATTR(_implementationOnly, ImplementationOnly,
-  OnImport | OnFunc | OnConstructor | OnVar | OnSubscript,
+  OnImport | OnFunc | OnConstructor | OnVar | OnSubscript | OnStruct,
   UserInaccessible | ABIStableToAdd | ABIStableToRemove | APIStableToAdd | APIStableToRemove | UnreachableInABIAttr,
   84)
 

include/swift/AST/DiagnosticsSema.def

@@ -3847,7 +3847,8 @@ ERROR(decl_from_hidden_module,none,
       "%2 was not imported by this file|"
       "C++ types from imported module %2 do not support library evolution|"
       "it was imported via the internal bridging header|"
-      "%2 was not imported publicly}3",
+      "%2 was not imported publicly|"
+      "it is a struct marked '@_implementationOnly'}3",
       (const Decl *, unsigned, Identifier, unsigned))
 ERROR(typealias_desugars_to_type_from_hidden_module,none,
       "%0 aliases '%1.%2' and cannot be used %select{here|"
@@ -3865,7 +3866,8 @@ ERROR(typealias_desugars_to_type_from_hidden_module,none,
       "%4 was not imported by this file|"
       "C++ types from imported module %4 do not support library evolution|"
       "it was imported via the internal bridging header|"
-      "%4 was not imported publicly}5",
+      "%4 was not imported publicly|"
+      "it is a struct marked '@_implementationOnly'}5",
       (const TypeAliasDecl *, StringRef, StringRef, unsigned, Identifier, unsigned))
 ERROR(conformance_from_implementation_only_module,none,
       "cannot use conformance of %0 to %1 %select{here|as property wrapper here|"
@@ -3881,7 +3883,8 @@ ERROR(conformance_from_implementation_only_module,none,
       "%3 was not imported by this file|"
       "C++ types from imported module %3 do not support library evolution|"
       "it was imported via the internal bridging header|"
-      "%3 was not imported publicly}4",
+      "%3 was not imported publicly|"
+      "it is a struct marked '@_implementationOnly'}4",
       (Type, Identifier, unsigned, Identifier, unsigned))
 NOTE(assoc_conformance_from_implementation_only_module,none,
      "in associated type %0 (inferred as %1)", (Type, Type))
@@ -3946,6 +3949,9 @@ ERROR(implementation_only_override_import_without_attr,none,
       "override of %kindonly0 imported as implementation-only must be declared "
       "'@_implementationOnly'",
       (const ValueDecl *))
+ERROR(implementation_only_on_structs_feature,none,
+      "'@_implementationOnly' on structs requires "
+      "'-enable-experimental-feature CheckImplementationOnly'", ())
 
 ERROR(import_attr_conflict,none,
       "%0 inconsistently imported with %1",
@@ -4128,7 +4134,7 @@ WARNING(attr_has_no_effect_on_decl_with_access_level,none,
        (DeclAttribute, AccessLevel))
 ERROR(attr_not_on_decl_with_invalid_access_level,none,
       "'%0' may not be used on "
-      "%select{private|fileprivate|internal|package|%error|%error}1 declarations",
+      "%select{private|fileprivate|internal|package|public|%error}1 declarations",
       (DeclAttribute, AccessLevel))
 
 ERROR(attr_has_no_effect_decl_not_available_before,none,
@@ -7350,7 +7356,8 @@ ERROR(inlinable_decl_ref_from_hidden_module,
       "%2 was not imported by this file|"
       "C++ APIs from imported module %2 do not support library evolution|"
       "it was imported via the internal bridging header|"
-      "%2 was not imported publicly}3",
+      "%2 was not imported publicly|"
+      "it is a struct marked '@_implementationOnly'}3",
       (const ValueDecl *, unsigned, Identifier, unsigned))
 
 ERROR(inlinable_typealias_desugars_to_type_from_hidden_module,
@@ -7362,7 +7369,8 @@ ERROR(inlinable_typealias_desugars_to_type_from_hidden_module,
       "%4 was not imported by this file|"
       "C++ types from imported module %4 do not support library evolution|"
       "it was imported via the internal bridging header|"
-      "%4 was not imported publicly}5",
+      "%4 was not imported publicly|"
+      "it is a struct marked '@_implementationOnly'}5",
       (const TypeAliasDecl *, StringRef, StringRef, unsigned, Identifier, unsigned))
 
 NOTE(missing_import_inserted,

include/swift/Basic/Features.def

@@ -558,6 +558,9 @@ EXPERIMENTAL_FEATURE(EmbeddedExistentials, false)
 /// Allow use of the 'anyAppleOS' availability domain.
 EXPERIMENTAL_FEATURE(AnyAppleOSAvailability, true)
 
+/// Check @_implementationOnly imports in non-library-evolution mode.
+EXPERIMENTAL_FEATURE(CheckImplementationOnly, true)
+
 #undef EXPERIMENTAL_FEATURE_EXCLUDED_FROM_MODULE_INTERFACE
 #undef EXPERIMENTAL_FEATURE
 #undef UPCOMING_FEATURE

lib/AST/Availability.cpp

@@ -1006,7 +1006,7 @@ bool swift::isExported(const ValueDecl *VD) {
 
   // Is this a stored property in a @frozen struct or class?
   if (auto *property = dyn_cast<VarDecl>(VD))
-    if (property->isLayoutExposedToClients())
+    if (property->isLayoutExposedToClients(/*applyImplicit=*/true))
       return true;
 
   return false;

lib/AST/Decl.cpp

@@ -2753,20 +2753,33 @@ bool VarDecl::isInitExposedToClients() const {
   return hasInitialValue() && isLayoutExposedToClients();
 }
 
-bool VarDecl::isLayoutExposedToClients() const {
+bool VarDecl::isLayoutExposedToClients(bool applyImplicit) const {
   auto parent = dyn_cast<NominalTypeDecl>(getDeclContext());
   if (!parent) return false;
   if (isStatic()) return false;
 
-
+  auto M = getDeclContext()->getParentModule();
   auto nominalAccess =
     parent->getFormalAccessScope(/*useDC=*/nullptr,
                                  /*treatUsableFromInlineAsPublic=*/true);
-  if (!nominalAccess.isPublic()) return false;
 
-  if (!parent->getAttrs().hasAttribute<FrozenAttr>() &&
-      !parent->getAttrs().hasAttribute<FixedLayoutAttr>())
+  // Resilient modules and classes hide layouts by default.
+  bool layoutIsHiddenByDefault = !applyImplicit ||
+    !getASTContext().LangOpts.hasFeature(Feature::CheckImplementationOnly) ||
+    M->getResilienceStrategy() == ResilienceStrategy::Resilient;
+  if (layoutIsHiddenByDefault) {
+    if (!nominalAccess.isPublic())
+      return false;
+
+    if (!parent->getAttrs().hasAttribute<FrozenAttr>() &&
+        !parent->getAttrs().hasAttribute<FixedLayoutAttr>())
     return false;
+  } else {
+    // Non-resilient module: layouts are exposed by default unless marked
+    // otherwise.
+    if (parent->getAttrs().hasAttribute<ImplementationOnlyAttr>())
+      return false;
+  }
 
   if (!hasStorage() &&
       !getAttrs().hasAttribute<LazyAttr>() &&

lib/AST/FeatureSet.cpp

@@ -144,6 +144,7 @@ UNINTERESTING_FEATURE(ExtractConstantsFromMembers)
 UNINTERESTING_FEATURE(GroupActorErrors)
 UNINTERESTING_FEATURE(SameElementRequirements)
 UNINTERESTING_FEATURE(SendingArgsAndResults)
+UNINTERESTING_FEATURE(CheckImplementationOnly)
 
 static bool findUnderscoredLifetimeAttr(Decl *decl) {
   auto hasUnderscoredLifetimeAttr = [](Decl *decl) {

lib/Sema/ResilienceDiagnostics.cpp

@@ -338,6 +338,7 @@ static bool diagnoseValueDeclRefExportability(SourceLoc loc, const ValueDecl *D,
 
   case DisallowedOriginKind::ImplementationOnly:
   case DisallowedOriginKind::FragileCxxAPI:
+  case DisallowedOriginKind::ImplementationOnlyMemoryLayout:
     break;
   }
 

lib/Sema/TypeCheckAccess.cpp

@@ -2163,6 +2163,14 @@ swift::getDisallowedOriginKind(const Decl *decl,
           Feature::AssumeResilientCxxTypes))
     return DisallowedOriginKind::FragileCxxAPI;
 
+  if (isa<StructDecl>(decl) || isa<EnumDecl>(decl)) {
+    if (decl->getASTContext().LangOpts.hasFeature(
+            Feature::CheckImplementationOnly) &&
+        decl->getAttrs().hasAttribute<ImplementationOnlyAttr>()) {
+      return DisallowedOriginKind::ImplementationOnlyMemoryLayout;
+    }
+  }
+
   // Report non-public import last as it can be ignored by the caller.
   // See \c diagnoseValueDeclRefExportability.
   auto importSource = decl->getImportAccessFrom(where.getDeclContext());

lib/Sema/TypeCheckAccess.h

@@ -32,7 +32,7 @@ class SourceFile;
 /// itself. Related checks may also be performed.
 void checkAccessControl(Decl *D);
 
-/// Problematic origin of an exported type.
+/// Problematic origin of a decl that may restrict its exportability.
 ///
 /// This enum must be kept in sync with a number of diagnostics:
 ///   diag::inlinable_decl_ref_from_hidden_module
@@ -52,6 +52,7 @@ enum class DisallowedOriginKind : uint8_t {
   InternalBridgingHeaderImport,
 
   NonPublicImport,
+  ImplementationOnlyMemoryLayout,
   None
 };