[Serialization] Refactor normal conformance serialization

include/swift/AST/Witness.h

@@ -150,9 +150,6 @@ class Witness {
   /// Determines whether there is a witness at all.
   explicit operator bool() const { return !storage.isNull(); }
 
-  /// Determine whether this witness requires any substitutions.
-  bool requiresSubstitution() const { return storage.is<StoredWitness *>(); }
-
   /// Retrieve the substitutions required to use this witness from the
   /// synthetic environment.
   ///
@@ -164,15 +161,17 @@ class Witness {
 
   /// Retrieve the synthetic generic environment.
   GenericEnvironment *getSyntheticEnvironment() const {
-    assert(requiresSubstitution() && "No substitutions required for witness");
-    return storage.get<StoredWitness *>()->syntheticEnvironment;
+    if (auto *storedWitness = storage.dyn_cast<StoredWitness *>())
+      return storedWitness->syntheticEnvironment;
+    return nullptr;
   }
 
   /// Retrieve the substitution map that maps the interface types of the
   /// requirement to the interface types of the synthetic environment.
   SubstitutionList getRequirementToSyntheticSubs() const {
-    assert(requiresSubstitution() && "No substitutions required for witness");
-    return storage.get<StoredWitness *>()->reqToSyntheticEnvSubs;
+    if (auto *storedWitness = storage.dyn_cast<StoredWitness *>())
+      return storedWitness->reqToSyntheticEnvSubs;
+    return {};
   }
 
   LLVM_ATTRIBUTE_DEPRECATED(

include/swift/Serialization/ModuleFormat.h

@@ -55,7 +55,7 @@ const uint16_t VERSION_MAJOR = 0;
 /// describe what change you made. The content of this comment isn't important;
 /// it just ensures a conflict if two people change the module format.
 /// Don't worry about adhering to the 80-column limit for this line.
-const uint16_t VERSION_MINOR = 408; // Last change: begin_access [nontracking]
+const uint16_t VERSION_MINOR = 409; // Last change: standalone requirement subs
 
 using DeclIDField = BCFixed<31>;
 

lib/AST/ASTVerifier.cpp

@@ -2507,13 +2507,18 @@ class Verifier : public ASTWalker {
           // Check the witness substitutions.
           const auto &witness = normal->getWitness(req, nullptr);
 
-          if (witness.requiresSubstitution()) {
-            GenericEnv.push_back({witness.getSyntheticEnvironment()});
-            for (const auto &sub : witness.getSubstitutions()) {
-              verifyChecked(sub.getReplacement());
-            }
+          if (auto *genericEnv = witness.getSyntheticEnvironment())
+            GenericEnv.push_back({genericEnv});
+
+          for (const auto &sub : witness.getRequirementToSyntheticSubs())
+            verifyChecked(sub.getReplacement());
+
+          for (const auto &sub : witness.getSubstitutions())
+            verifyChecked(sub.getReplacement());
+
+          if (auto *genericEnv = witness.getSyntheticEnvironment()) {
             assert(GenericEnv.back().storage.dyn_cast<GenericEnvironment *>()
-                     == witness.getSyntheticEnvironment());
+                     == genericEnv);
             GenericEnv.pop_back();
           }
 

lib/AST/ProtocolConformance.cpp

@@ -41,8 +41,13 @@ using namespace swift;
 Witness::Witness(ValueDecl *decl, SubstitutionList substitutions,
                  GenericEnvironment *syntheticEnv,
                  SubstitutionList reqToSynthesizedEnvSubs) {
-  auto &ctx = decl->getASTContext();
+  if (!syntheticEnv && substitutions.empty() &&
+      reqToSynthesizedEnvSubs.empty()) {
+    storage = decl;
+    return;
+  }
 
+  auto &ctx = decl->getASTContext();
   auto declRef = ConcreteDeclRef(ctx, decl, substitutions);
   auto storedMem = ctx.Allocate(sizeof(StoredWitness), alignof(StoredWitness));
   auto stored = new (storedMem)

lib/Serialization/Deserialization.cpp

@@ -5309,13 +5309,13 @@ void ModuleFile::finishNormalConformance(NormalProtocolConformance *conformance,
     if (auto syntheticSig = getGenericSignature(*rawIDIter++)) {
       // Create the synthetic environment.
       syntheticEnv = syntheticSig->createGenericEnvironment();
+    }
 
-      // Requirement -> synthetic substitutions.
-      if (unsigned numReqSubstitutions = *rawIDIter++) {
-        while (numReqSubstitutions--) {
-          auto sub = maybeReadSubstitution(DeclTypeCursor, nullptr);
-          reqToSyntheticSubs.push_back(*sub);
-        }
+    // Requirement -> synthetic substitutions.
+    if (unsigned numReqSubstitutions = *rawIDIter++) {
+      while (numReqSubstitutions--) {
+        auto sub = maybeReadSubstitution(DeclTypeCursor, nullptr);
+        reqToSyntheticSubs.push_back(*sub);
       }
     }
 
@@ -5334,13 +5334,6 @@ void ModuleFile::finishNormalConformance(NormalProtocolConformance *conformance,
       continue;
     }
 
-    // Handle simple witnesses.
-    if (witnessSubstitutions.empty() && !syntheticSig && !syntheticEnv &&
-        reqToSyntheticSubs.empty()) {
-      trySetWitness(Witness(witness));
-      continue;
-    }
-
     // Set the witness.
     trySetWitness(Witness(witness, witnessSubstitutions,
                           syntheticEnv, reqToSyntheticSubs));

lib/Serialization/Serialization.cpp

@@ -1530,19 +1530,17 @@ void Serializer::writeNormalConformance(
       // If there is no witness, we're done.
       if (!witness.getDecl()) return;
 
-      if (auto genericEnv = witness.requiresSubstitution() 
-                              ? witness.getSyntheticEnvironment()
-                              : nullptr) {
+      if (auto *genericEnv = witness.getSyntheticEnvironment()) {
         // Generic signature.
         auto *genericSig = genericEnv->getGenericSignature();
         data.push_back(addGenericSignatureRef(genericSig));
-
-        auto reqToSyntheticSubs = witness.getRequirementToSyntheticSubs();
-        data.push_back(reqToSyntheticSubs.size());
       } else {
         data.push_back(/*null generic signature*/0);
       }
 
+      auto reqToSyntheticSubs = witness.getRequirementToSyntheticSubs();
+
+      data.push_back(reqToSyntheticSubs.size());
       data.push_back(witness.getSubstitutions().size());
   });
 
@@ -1568,19 +1566,14 @@ void Serializer::writeNormalConformance(
    // Bail out early for simple witnesses.
    if (!witness.getDecl()) return;
 
-   if (witness.requiresSubstitution()) {
-     // Write requirement-to-synthetic substitutions.
-     writeSubstitutions(witness.getRequirementToSyntheticSubs(),
-                        DeclTypeAbbrCodes,
-                        nullptr);
-   }
+   // Write requirement-to-synthetic substitutions.
+   writeSubstitutions(witness.getRequirementToSyntheticSubs(),
+                      DeclTypeAbbrCodes, nullptr);
 
    // Write the witness substitutions.
    writeSubstitutions(witness.getSubstitutions(),
                       DeclTypeAbbrCodes,
-                      witness.requiresSubstitution()
-                        ? witness.getSyntheticEnvironment()
-                        : nullptr);
+                      witness.getSyntheticEnvironment());
   });
 }
 

validation-test/compiler_crashers_2_fixed/0153-rdar36497404.swift

@@ -0,0 +1,44 @@
+// RUN: %target-build-swift -emit-module -o %t %s
+
+public protocol P1 {}
+public protocol P2 {}
+
+public protocol P3 {
+  static func a()
+
+  func b()
+  func b<I: P1>(_: (I) -> Void)
+
+  static func c<I: P1>(_: I)
+  static func d()
+  static func d<I: P1>(_: ([(I, I)]) -> Void)
+  static func d<I: P1>(_: ([I: I]) -> Void)
+  static func d<Q: P1>(_: Q)
+
+  static func e<Q: P1, I: P2>(_: Q, _: (I) -> Void)
+  static func f<Q: P1, I: P2>(_: Q, _: (I) -> Void)
+
+  func g<I: P1>(_: I)
+}
+
+public extension P3 {
+  static func a() {}
+
+  func b() {}
+  func b<I: P1>(_: (I) -> Void) {}
+
+  static func c<I: P1>(_: I) {}
+
+  static func d() {}
+  static func d<I: P1>(_: ([(I, I)]) -> Void) {}
+  static func d<I: P1>(_: ([I: I]) -> Void) {}
+  static func d<Q: P1>(_: Q) {}
+
+  static func e<Q: P1, I: P2>(_: Q, _: (I) -> Void) {}
+  static func f<Q: P1, I: P2>(_: Q, _: (I) -> Void) {}
+
+  func g<I: P1>(_: I) {}
+}
+
+struct S: P3 {
+}