Serialization: Use a DeclVisitor to implement deserialization safety.

Refactor deserialization safety to use a `DeclVisitor` CRTP instead of ad-hoc
casts. This ensures every kind of decl is handled explicitly.

Resolves rdar://115456536

Author: tshortli

lib/Serialization/Serialization.cpp

@@ -1,4 +1,4 @@
-  //===--- Serialization.cpp - Read and write Swift modules -----------------===//
+//===--- Serialization.cpp - Read and write Swift modules -----------------===//
 //
 // This source file is part of the Swift.org open source project
 //
@@ -3276,23 +3276,39 @@ class Serializer::DeclSerializer : public DeclVisitor<DeclSerializer> {
     return count;
   }
 
-public:
-  /// Determine if \p decl is safe to deserialize when it's public
-  /// or otherwise needed by the client in normal builds, this should usually
-  /// correspond to logic in type-checking ensuring these safe decls don't
-  /// refer to implementation details. We have to be careful not to mark
-  /// anything needed by a client as unsafe as the client will reject reading
-  /// it, but at the same time keep the safety checks precise to avoid
-  /// XRef errors and such.
-  static bool isDeserializationSafe(const Decl *decl) {
-    if (auto ext = dyn_cast<ExtensionDecl>(decl)) {
-      // Consider extensions as safe as their extended type.
+  class ExternallyAccessibleDeclVisitor
+      : public DeclVisitor<ExternallyAccessibleDeclVisitor, bool> {
+  public:
+    ExternallyAccessibleDeclVisitor(){};
+
+    bool visit(const Decl *D) {
+      if (auto value = dyn_cast<ValueDecl>(D)) {
+        // A decl is externally accessible if it has a public access level.
+        auto accessScope =
+            value->getFormalAccessScope(/*useDC=*/nullptr,
+                                        /*treatUsableFromInlineAsPublic=*/true);
+        if (accessScope.isPublic() || accessScope.isPackage())
+          return true;
+      }
+
+      return DeclVisitor<ExternallyAccessibleDeclVisitor, bool>::visit(
+          const_cast<Decl *>(D));
+    }
+
+    // Force all decl kinds to be handled explicitly.
+    bool visitDecl(const Decl *D) = delete;
+    bool visitValueDecl(const ValueDecl *valueDecl) = delete;
+
+    bool visitExtensionDecl(const ExtensionDecl *ext) {
+      // Extensions must extend externally accessible types to be externally
+      // accessible.
       auto nominalType = ext->getExtendedNominal();
       if (!nominalType ||
           !isDeserializationSafe(nominalType))
         return false;
 
-      // We can mark the extension unsafe only if it has no public members.
+      // If the extension has any externally accessible members, then it is
+      // externally accessible.
       auto members = ext->getMembers();
       auto hasSafeMembers = std::any_of(members.begin(), members.end(),
         [](const Decl *D) -> bool {
@@ -3303,26 +3319,28 @@ class Serializer::DeclSerializer : public DeclVisitor<DeclSerializer> {
       if (hasSafeMembers)
         return true;
 
-      // We can mark the extension unsafe only if it has no public
-      // conformances.
+      // If the extension has any externally accessible conformances, then it is
+      // externally accessible.
       auto protocols = ext->getLocalProtocols(ConformanceLookupKind::All);
       bool hasSafeConformances = std::any_of(
-          protocols.begin(), protocols.end(), [](ProtocolDecl *protocol) {
-            return isDeserializationSafe(protocol);
+          protocols.begin(), protocols.end(), [this](ProtocolDecl *protocol) {
+            return visit(protocol);
           });
 
       if (hasSafeConformances)
         return true;
 
-      // Truly empty extensions are safe, it may happen in swiftinterfaces.
+      // Truly empty extensions are externally accessible. This can occur in
+      // swiftinterfaces, for example.
       if (members.empty() && protocols.size() == 0)
         return true;
 
       return false;
     }
 
-    if (auto pbd = dyn_cast<PatternBindingDecl>(decl)) {
-      // Pattern bindings are safe if any of their var decls are safe.
+    bool visitPatternBindingDecl(const PatternBindingDecl *pbd) {
+      // Pattern bindings are externally accessible if any of their var decls
+      // are externally accessible.
       for (auto i : range(pbd->getNumPatternEntries())) {
         if (auto *varDecl = pbd->getAnchoringVarDecl(i)) {
           if (isDeserializationSafe(varDecl))
@@ -3333,44 +3351,86 @@ class Serializer::DeclSerializer : public DeclVisitor<DeclSerializer> {
       return false;
     }
 
-    return isDeserializationSafe(cast<ValueDecl>(decl));
-  }
-
-  static bool isDeserializationSafe(const ValueDecl *value) {
-    // A decl is safe if formally accessible publicly.
-    auto accessScope =
-        value->getFormalAccessScope(/*useDC=*/nullptr,
-                                    /*treatUsableFromInlineAsPublic=*/true);
-    if (accessScope.isPublic() || accessScope.isPackage())
-      return true;
-
-    if (auto accessor = dyn_cast<AccessorDecl>(value))
-      // Accessors are as safe as their storage.
-      if (isDeserializationSafe(accessor->getStorage()))
-        return true;
-
-    // Frozen fields are always safe.
-    if (auto var = dyn_cast<VarDecl>(value)) {
+    bool visitVarDecl(const VarDecl *var) {
       if (var->isLayoutExposedToClients())
         return true;
 
-      // Consider all lazy var storage as "safe".
+      // Consider all lazy var storage as externally accessible.
       // FIXME: We should keep track of what lazy var is associated to the
-      //        storage for them to preserve the same safeness.
+      //        storage for them to preserve the same accessibility.
       if (var->isLazyStorageProperty())
         return true;
 
-      // Property wrappers storage is as safe as the wrapped property.
+      // Property wrapper storage is as externally accessible as the wrapped
+      // property.
       if (VarDecl *wrapped = var->getOriginalWrappedProperty())
-        if (isDeserializationSafe(wrapped))
+        if (visit(wrapped))
           return true;
+
+      return false;
     }
 
-    // Paramters don't have meaningful access control.
-    if (isa<ParamDecl>(value) || isa<GenericTypeParamDecl>(value))
-      return true;
+    bool visitAccessorDecl(const AccessorDecl *accessor) {
+      // Accessors are as externally accessible as the storage.
+      return visit(accessor->getStorage());
+    }
+
+    // ValueDecls with effectively public access are considered externally
+    // accessible and are handled in visit(Decl *) above. Some specific kinds of
+    // ValueDecls with additional, unique rules are handled individually above.
+    // ValueDecls that are not effectively public and do not have unique rules
+    // are by default externally inaccessable.
+#define DEFAULT_TO_ACCESS_LEVEL(KIND)                                          \
+  bool visit##KIND##Decl(const KIND##Decl *D) {                                \
+    static_assert(std::is_convertible<KIND##Decl *, ValueDecl *>::value,       \
+                  "##KIND##Decl must be a ValueDecl");                         \
+    return false;                                                              \
+  }
+    DEFAULT_TO_ACCESS_LEVEL(NominalType);
+    DEFAULT_TO_ACCESS_LEVEL(OpaqueType);
+    DEFAULT_TO_ACCESS_LEVEL(TypeAlias);
+    DEFAULT_TO_ACCESS_LEVEL(AssociatedType);
+    DEFAULT_TO_ACCESS_LEVEL(AbstractStorage);
+    DEFAULT_TO_ACCESS_LEVEL(AbstractFunction);
+    DEFAULT_TO_ACCESS_LEVEL(Macro);
+    DEFAULT_TO_ACCESS_LEVEL(EnumElement);
+
+#undef DEFAULT_TO_ACCESS_LEVEL
+
+    // There are several kinds of decls which we never expect to encounter in
+    // external accessibility queries.
+#define UNREACHABLE(KIND)                                                      \
+  bool visit##KIND##Decl(const KIND##Decl *D) {                                \
+    llvm_unreachable("unexpected decl kind");                                  \
+    return true;                                                               \
+  }
+    UNREACHABLE(Module);
+    UNREACHABLE(TopLevelCode);
+    UNREACHABLE(Import);
+    UNREACHABLE(PoundDiagnostic);
+    UNREACHABLE(Missing);
+    UNREACHABLE(MissingMember);
+    UNREACHABLE(MacroExpansion);
+    UNREACHABLE(GenericTypeParam);
+    UNREACHABLE(Param);
+    UNREACHABLE(IfConfig);
+    UNREACHABLE(PrecedenceGroup);
+    UNREACHABLE(Operator);
+    UNREACHABLE(EnumCase);
+
+#undef UNREACHABLE
+  };
 
-    return false;
+public:
+  /// Determine if \p decl is safe to deserialize when it's public
+  /// or otherwise needed by the client in normal builds, this should usually
+  /// correspond to logic in type-checking ensuring these safe decls don't
+  /// refer to implementation details. We have to be careful not to mark
+  /// anything needed by a client as unsafe as the client will reject reading
+  /// it, but at the same time keep the safety checks precise to avoid
+  /// XRef errors and such.
+  static bool isDeserializationSafe(const Decl *decl) {
+    return ExternallyAccessibleDeclVisitor().visit(decl);
   }
 
 private: