Address Review Comments

Author: CodaFi

include/swift/Basic/Fingerprint.h

@@ -28,7 +28,7 @@ class IO;
 
 namespace swift {
 
-/// A \c Fingerprint represents a stable point of identity for a piece of data
+/// A \c Fingerprint represents a stable summary of a given piece of data
 /// in the compiler.
 ///
 /// A \c Fingerprint value is subject to the following invariants:
@@ -109,6 +109,15 @@ class Fingerprint final {
     return llvm::hash_value(fp.Core);
   }
 
+public:
+  /// The fingerprint value consisting of 32 bytes of zeroes.
+  ///
+  /// This fingerprint is a perfectly fine value for an MD5 hash, but it is
+  /// completely arbitrary.
+  static Fingerprint ZERO() {
+    return Fingerprint("00000000000000000000000000000000");
+  }
+
 private:
   /// llvm::yaml would like us to be default constructible, but \c Fingerprint
   /// would prefer to enforce its internal invariants.

lib/AST/DeclContext.cpp

@@ -1012,9 +1012,9 @@ IterableDeclContext::castDeclToIterableDeclContext(const Decl *D) {
 }
 
 Optional<Fingerprint> IterableDeclContext::getBodyFingerprint() const {
-  auto &ctx = getASTContext();
   auto mutableThis = const_cast<IterableDeclContext *>(this);
-  return evaluateOrDefault(ctx.evaluator, ParseMembersRequest{mutableThis},
+  return evaluateOrDefault(getASTContext().evaluator,
+                           ParseMembersRequest{mutableThis},
                            FingerprintAndMembers())
       .fingerprint;
 }

lib/AST/FrontendSourceFileDepGraphFactory.cpp

@@ -506,11 +506,18 @@ void FrontendSourceFileDepGraphFactory::addAllUsedDecls() {
 ModuleDepGraphFactory::ModuleDepGraphFactory(const ModuleDecl *Mod,
                                              bool emitDot)
     : AbstractSourceFileDepGraphFactory(Mod->getASTContext().hadError(),
-                                        Mod->getNameStr(),
-                                        Fingerprint("00000000000000000000000000000000"),
-                                        emitDot,
-                                        Mod->getASTContext().Diags),
-      Mod(Mod) {}
+                                        Mod->getNameStr(), Fingerprint::ZERO(),
+                                        emitDot, Mod->getASTContext().Diags),
+
+      Mod(Mod) {
+  // Since a fingerprint only summarizes the state of the module but not
+  // the state of its fingerprinted sub-declarations, and since a module
+  // contains no state other than sub-declarations, its fingerprint does not
+  // matter and can just be some arbitrary value. Should it be the case that a
+  // change in a declaration that does not have a fingerprint must cause
+  // a rebuild of a file outside of the module, this assumption will need
+  // to be revisited.
+}
 
 void ModuleDepGraphFactory::addAllDefinedDecls() {
   // TODO: express the multiple provides and depends streams with variadic

lib/Driver/Driver.cpp

@@ -547,7 +547,10 @@ populateOutOfDateMap(InputInfoMap &map, llvm::sys::TimePoint<> &LastBuildTime,
       // -swift-version argument is handled in the argsHashStr check that
       // follows.
       CompilationRecordSwiftVersion = value->getValue(scratch);
-      versionValid = CompilationRecordSwiftVersion.equals(version::getSwiftFullVersion(version::Version::getCurrentLanguageVersion()));
+      versionValid = (CompilationRecordSwiftVersion
+                      == version::getSwiftFullVersion(
+                        version::Version::getCurrentLanguageVersion()));
+
     } else if (keyStr == compilation_record::getName(TopLevelKey::Options)) {
       auto *value = dyn_cast<yaml::ScalarNode>(i->getValue());
       if (!value)

lib/Serialization/ModuleFileCoreTableInfo.h

@@ -566,6 +566,8 @@ class ModuleFileSharedCore::DeclUSRTableInfo {
   }
 };
 
+/// Serialization for the table mapping module-level declIDs for serialized
+/// iterable decl contexts to their corresponding \c Fingerprint values.
 class ModuleFileSharedCore::DeclFingerprintsTableInfo {
 public:
   using internal_key_type = uint32_t;

lib/Serialization/Serialization.cpp

@@ -435,6 +435,8 @@ namespace {
     }
   };
 
+  // Side table information for serializing the table keyed under
+  // \c DeclFingerprintsLayout.
   class DeclFingerprintsTableInfo {
   public:
     using key_type = DeclID;

lib/Serialization/Serialization.h

@@ -279,7 +279,7 @@ class Serializer : public SerializerBase {
 
   // In-memory representation of what will eventually be an on-disk
   // hash table of the fingerprint associated with a serialized
-  // iterable decl context.
+  // iterable decl context. It is keyed by that context's decl ID.
   using DeclFingerprintsTable = llvm::MapVector<uint32_t, Fingerprint>;
 
 private: