Fix crash when loading old cache when file format changed.

Also eliminate an unnecessary copy after finishing deserialization.

src/cache.cc

@@ -30,11 +30,7 @@ std::unique_ptr<IndexFile> LoadCachedIndex(Config* config,
   if (!file_content)
     return nullptr;
 
-  optional<IndexFile> indexed = Deserialize(filename, *file_content);
+  return Deserialize(filename, *file_content, IndexFile::kCurrentVersion);
-  if (indexed && indexed->version == IndexFile::kCurrentVersion)
-    return MakeUnique<IndexFile>(indexed.value());
-
-  return nullptr;
 }
 
 optional<std::string> LoadCachedFileContents(Config* config,

src/serializer.cc

@@ -179,29 +179,39 @@ std::string Serialize(IndexFile& file) {
   return output.GetString();
 }
 
-optional<IndexFile> Deserialize(std::string path, std::string serialized) {
+std::unique_ptr<IndexFile> Deserialize(std::string path, std::string serialized, optional<int> expected_version) {
   rapidjson::Document reader;
   reader.Parse(serialized.c_str());
   if (reader.HasParseError())
-    return nullopt;
+    return nullptr;
 
-  IndexFile file(path);
+  // Do not deserialize a document with a bad version. Doing so could cause a
-  Reflect(reader, file);
+  // crash because the file format may have changed.
+  if (expected_version) {
+    auto actual_version = reader.FindMember("version");
+    if (actual_version == reader.MemberEnd() ||
+        actual_version->value.GetInt() != expected_version) {
+      return nullptr;
+    }
+  }
+
+  auto file = MakeUnique<IndexFile>(path);
+  Reflect(reader, *file);
 
   // Restore non-serialized state.
-  file.path = path;
+  file->path = path;
-  file.id_cache.primary_file = file.path;
+  file->id_cache.primary_file = file->path;
-  for (const auto& type : file.types) {
+  for (const auto& type : file->types) {
-    file.id_cache.type_id_to_usr[type.id] = type.def.usr;
+    file->id_cache.type_id_to_usr[type.id] = type.def.usr;
-    file.id_cache.usr_to_type_id[type.def.usr] = type.id;
+    file->id_cache.usr_to_type_id[type.def.usr] = type.id;
   }
-  for (const auto& func : file.funcs) {
+  for (const auto& func : file->funcs) {
-    file.id_cache.func_id_to_usr[func.id] = func.def.usr;
+    file->id_cache.func_id_to_usr[func.id] = func.def.usr;
-    file.id_cache.usr_to_func_id[func.def.usr] = func.id;
+    file->id_cache.usr_to_func_id[func.def.usr] = func.id;
   }
-  for (const auto& var : file.vars) {
+  for (const auto& var : file->vars) {
-    file.id_cache.var_id_to_usr[var.id] = var.def.usr;
+    file->id_cache.var_id_to_usr[var.id] = var.def.usr;
-    file.id_cache.usr_to_var_id[var.def.usr] = var.id;
+    file->id_cache.usr_to_var_id[var.def.usr] = var.id;
   }
 
   return file;

src/serializer.h

@@ -5,8 +5,9 @@
 #include <rapidjson/document.h>
 #include <rapidjson/prettywriter.h>
 
-#include <vector>
+#include <memory>
 #include <string>
+#include <vector>
 
 
 using std::experimental::optional;
@@ -222,6 +223,6 @@ void ReflectMember(Reader& visitor, const char* name, T& value) {
 }
 
 std::string Serialize(IndexFile& file);
-optional<IndexFile> Deserialize(std::string path, std::string serialized);
+std::unique_ptr<IndexFile> Deserialize(std::string path, std::string serialized, optional<int> expected_version);
 
 void SetTestOutputMode();

src/test.cc

@@ -66,7 +66,8 @@ void DiffDocuments(std::string path, std::string path_section, rapidjson::Docume
 
 void VerifySerializeToFrom(IndexFile* file) {
   std::string expected = file->ToString();
-  std::string actual = Deserialize("--.cc", Serialize(*file)).value().ToString();
+  std::unique_ptr<IndexFile> result = Deserialize("--.cc", Serialize(*file), nullopt /*expected_version*/);
+  std::string actual = result->ToString();
   if (expected != actual) {
     std::cerr << "Serialization failure" << std::endl;;
     assert(false);