QueryFile is now storing symbol indices directly

src/command_line.cc

@@ -124,8 +124,7 @@ std::string Join(const std::vector<std::string>& elements, std::string sep) {
 }
 
 
-optional<QueryableLocation> GetDefinitionSpellingOfUsr(QueryableDatabase* db, const Usr& usr) {
-  SymbolIdx symbol = db->usr_to_symbol[usr];
+optional<QueryableLocation> GetDefinitionSpellingOfSymbol(QueryableDatabase* db, const SymbolIdx& symbol) {
   switch (symbol.kind) {
   case SymbolKind::Type: {
     QueryableTypeDef* def = &db->types[symbol.idx];
@@ -148,6 +147,10 @@ optional<QueryableLocation> GetDefinitionSpellingOfUsr(QueryableDatabase* db, co
   return nullopt;
 }
 
+optional<QueryableLocation> GetDefinitionSpellingOfUsr(QueryableDatabase* db, const Usr& usr) {
+  return GetDefinitionSpellingOfSymbol(db, db->usr_to_symbol[usr]);
+}
+
 optional<QueryableLocation> GetDefinitionExtentOfUsr(QueryableDatabase* db, const Usr& usr) {
   SymbolIdx symbol = db->usr_to_symbol[usr];
   switch (symbol.kind) {
@@ -555,10 +558,10 @@ void QueryDbMainLoop(
       int target_line = msg->params.position.line + 1;
       int target_column = msg->params.position.character + 1;
 
-      for (const UsrRef& ref : file->def.all_symbols) {
+      for (const SymbolRef& ref : file->def.all_symbols) {
         if (ref.loc.range.start.line >= target_line && ref.loc.range.end.line <= target_line &&
             ref.loc.range.start.column <= target_column && ref.loc.range.end.column >= target_column) {
-          optional<QueryableLocation> location = GetDefinitionSpellingOfUsr(db, ref.usr);
+          optional<QueryableLocation> location = GetDefinitionSpellingOfSymbol(db, ref.idx);
           if (location)
             response.result.push_back(GetLsLocation(location.value()));
           break;
@@ -582,13 +585,12 @@ void QueryDbMainLoop(
       }
 
       std::cerr << "File outline size is " << file->def.outline.size() << std::endl;
-      for (UsrRef ref : file->def.outline) {
-        SymbolIdx symbol = db->usr_to_symbol[ref.usr];
+      for (SymbolRef ref : file->def.outline) {
+        SymbolIdx symbol = ref.idx;
 
         lsSymbolInformation info;
         info.location = GetLsLocation(ref.loc);
 
-        // TODO: cleanup namespace/naming so there is only one SymbolKind.
         switch (symbol.kind) {
         case SymbolKind::Type: {
           QueryableTypeDef& def = db->types[symbol.idx];
@@ -645,11 +647,11 @@ void QueryDbMainLoop(
         break;
       }
 
-      for (UsrRef ref : file->def.outline) {
+      for (SymbolRef ref : file->def.outline) {
         // NOTE: We OffsetColumn so that the code lens always show up in a
         // predictable order. Otherwise, the client may randomize it.
 
-        SymbolIdx symbol = db->usr_to_symbol[ref.usr];
+        SymbolIdx symbol = ref.idx;
         switch (symbol.kind) {
         case SymbolKind::Type: {
           QueryableTypeDef& def = db->types[symbol.idx];

src/indexer.h

@@ -30,7 +30,7 @@ struct Id {
   uint64_t id;
 
   Id() : id(0) {}  // Needed for containers. Do not use directly.
-  Id(uint64_t id) : id(id) {}
+  explicit Id(uint64_t id) : id(id) {}
 
   bool operator==(const Id<T>& other) const { return id == other.id; }
 

src/query.cc

@@ -135,46 +135,46 @@ QueryableFile::Def BuildFileDef(const IdMap& id_map, const IndexedFile& indexed)
   QueryableFile::Def def;
   def.usr = indexed.path;
 
-  auto add_outline = [&def, &id_map](Usr usr, Range range) {
-    def.outline.push_back(UsrRef(usr, MapIdToUsr(id_map, range)));
+  auto add_outline = [&def, &id_map](SymbolIdx idx, Range range) {
+    def.outline.push_back(SymbolRef(idx, MapIdToUsr(id_map, range)));
   };
-  auto add_all_symbols = [&def, &id_map](Usr usr, Range range) {
-    def.all_symbols.push_back(UsrRef(usr, MapIdToUsr(id_map, range)));
+  auto add_all_symbols = [&def, &id_map](SymbolIdx idx, Range range) {
+    def.all_symbols.push_back(SymbolRef(idx, MapIdToUsr(id_map, range)));
   };
 
   for (const IndexedTypeDef& def : indexed.types) {
     if (def.def.definition_spelling.has_value())
-      add_all_symbols(def.def.usr, def.def.definition_spelling.value());
+      add_all_symbols(id_map.ToSymbol(def.id), def.def.definition_spelling.value());
     if (def.def.definition_extent.has_value())
-      add_outline(def.def.usr, def.def.definition_extent.value());
+      add_outline(id_map.ToSymbol(def.id), def.def.definition_extent.value());
     for (const Range& use : def.uses)
-      add_all_symbols(def.def.usr, use);
+      add_all_symbols(id_map.ToSymbol(def.id), use);
   }
   for (const IndexedFuncDef& def : indexed.funcs) {
     if (def.def.definition_spelling.has_value())
-      add_all_symbols(def.def.usr, def.def.definition_spelling.value());
+      add_all_symbols(id_map.ToSymbol(def.id), def.def.definition_spelling.value());
     if (def.def.definition_extent.has_value())
-      add_outline(def.def.usr, def.def.definition_extent.value());
+      add_outline(id_map.ToSymbol(def.id), def.def.definition_extent.value());
     for (Range decl : def.declarations) {
-      add_outline(def.def.usr, decl);
-      add_all_symbols(def.def.usr, decl);
+      add_outline(id_map.ToSymbol(def.id), decl);
+      add_all_symbols(id_map.ToSymbol(def.id), decl);
     }
     for (const Range& use : def.uses)
-      add_all_symbols(def.def.usr, use);
+      add_all_symbols(id_map.ToSymbol(def.id), use);
   }
   for (const IndexedVarDef& def : indexed.vars) {
     if (def.def.definition_spelling.has_value())
-      add_all_symbols(def.def.usr, def.def.definition_spelling.value());
+      add_all_symbols(id_map.ToSymbol(def.id), def.def.definition_spelling.value());
     if (def.def.definition_extent.has_value())
-      add_outline(def.def.usr, def.def.definition_extent.value());
+      add_outline(id_map.ToSymbol(def.id), def.def.definition_extent.value());
     for (const Range& use : def.uses)
-      add_all_symbols(def.def.usr, use);
+      add_all_symbols(id_map.ToSymbol(def.id), use);
   }
 
-  std::sort(def.outline.begin(), def.outline.end(), [](const UsrRef& a, const UsrRef& b) {
+  std::sort(def.outline.begin(), def.outline.end(), [](const SymbolRef& a, const SymbolRef& b) {
     return a.loc.range.start < b.loc.range.start;
   });
-  std::sort(def.all_symbols.begin(), def.all_symbols.end(), [](const UsrRef& a, const UsrRef& b) {
+  std::sort(def.all_symbols.begin(), def.all_symbols.end(), [](const SymbolRef& a, const SymbolRef& b) {
     return a.loc.range.start < b.loc.range.start;
   });
 
@@ -352,123 +352,101 @@ QueryFileId GetQueryFileIdFromUsr(QueryableDatabase* query_db, const Usr& usr) {
   auto it = query_db->usr_to_symbol.find(usr);
   if (it != query_db->usr_to_symbol.end()) {
     assert(it->second.kind == SymbolKind::File);
-    return it->second.idx;
+    return QueryFileId(it->second.idx);
   }
 
-  int idx = query_db->files.size();
+  size_t idx = query_db->files.size();
   query_db->usr_to_symbol[usr] = SymbolIdx(SymbolKind::File, idx);
   query_db->files.push_back(QueryableFile(usr));
-  return idx;
+  return QueryFileId(idx);
 }
 
 QueryTypeId GetQueryTypeIdFromUsr(QueryableDatabase* query_db, const Usr& usr) {
   auto it = query_db->usr_to_symbol.find(usr);
   if (it != query_db->usr_to_symbol.end()) {
     assert(it->second.kind == SymbolKind::Type);
-    return it->second.idx;
+    return QueryTypeId(it->second.idx);
   }
 
-  int idx = query_db->types.size();
+  size_t idx = query_db->types.size();
   query_db->usr_to_symbol[usr] = SymbolIdx(SymbolKind::Type, idx);
   query_db->types.push_back(QueryableTypeDef(usr));
-  return idx;
+  return QueryTypeId(idx);
 }
 
 QueryFuncId GetQueryFuncIdFromUsr(QueryableDatabase* query_db, const Usr& usr) {
   auto it = query_db->usr_to_symbol.find(usr);
   if (it != query_db->usr_to_symbol.end()) {
     assert(it->second.kind == SymbolKind::Func);
-    return it->second.idx;
+    return QueryFuncId(it->second.idx);
   }
 
-  int idx = query_db->funcs.size();
+  size_t idx = query_db->funcs.size();
   query_db->usr_to_symbol[usr] = SymbolIdx(SymbolKind::Func, idx);
   query_db->funcs.push_back(QueryableFuncDef(usr));
-  return idx;
+  return QueryFuncId(idx);
 }
 
 QueryVarId GetQueryVarIdFromUsr(QueryableDatabase* query_db, const Usr& usr) {
   auto it = query_db->usr_to_symbol.find(usr);
   if (it != query_db->usr_to_symbol.end()) {
     assert(it->second.kind == SymbolKind::Var);
-    return it->second.idx;
+    return QueryVarId(it->second.idx);
   }
 
-  int idx = query_db->vars.size();
+  size_t idx = query_db->vars.size();
   query_db->usr_to_symbol[usr] = SymbolIdx(SymbolKind::Var, idx);
   query_db->vars.push_back(QueryableVarDef(usr));
-  return idx;
+  return QueryVarId(idx);
 }
 
 
-#if false
-int GetOrAddSymbol(QueryableDatabase* query_db, SymbolKind kind, const Usr& usr) {
-  // TODO: consider having separate lookup maps so they are smaller (maybe
-  // lookups will go faster).
-  auto it = query_db->usr_to_symbol.find(usr);
-
-  // Found; return existing symbol.
-  if (it != query_db->usr_to_symbol.end()) {
-    assert(it->second.kind == kind);
-    return it->second.idx;
-  }
-
-  // Not found; add a new symbol.
-  switch (kind) {
-  case SymbolKind::File: {
-    int idx = query_db->files.size();
-    query_db->usr_to_symbol[usr] = SymbolIdx(kind, idx);
-    query_db->files.push_back(QueryableFile(usr));
-    return idx;
-  }
-  case SymbolKind::Type: {
-    int idx = query_db->types.size();
-    query_db->usr_to_symbol[usr] = SymbolIdx(kind, idx);
-    query_db->types.push_back(QueryableTypeDef(usr));
-    return idx;
-  }
-  case SymbolKind::Func: {
-    int idx = query_db->funcs.size();
-    query_db->usr_to_symbol[usr] = SymbolIdx(kind, idx);
-    query_db->funcs.push_back(QueryableFuncDef(usr));
-    return idx;
-  }
-  case SymbolKind::Var: {
-    int idx = query_db->vars.size();
-    query_db->usr_to_symbol[usr] = SymbolIdx(kind, idx);
-    query_db->vars.push_back(QueryableVarDef(usr));
-    return idx;
-  }
-  case SymbolKind::Invalid: {
-    assert(false);
-    return -1;
-  }
-  }
-
-  assert(false);
-  return -1;
-}
-#endif
-
 IdMap::IdMap(QueryableDatabase* query_db, const IdCache& local_ids)
   : local_ids(local_ids) {
   index_file_id = GetQueryFileIdFromUsr(query_db, local_ids.primary_file);
 
-  cached_type_ids_.reserve(local_ids.type_id_to_usr.size());
+  cached_type_ids_.set_empty_key(-1);
+  cached_type_ids_.resize(local_ids.type_id_to_usr.size());
   for (const auto& entry : local_ids.type_id_to_usr)
-    cached_type_ids_[entry.first] = GetQueryTypeIdFromUsr(query_db, entry.second);
+    cached_type_ids_[entry.first.id] = GetQueryTypeIdFromUsr(query_db, entry.second).id;
 
-  cached_func_ids_.reserve(local_ids.func_id_to_usr.size());
+  cached_func_ids_.set_empty_key(-1);
+  cached_func_ids_.resize(local_ids.func_id_to_usr.size());
   for (const auto& entry : local_ids.func_id_to_usr)
-    cached_func_ids_[entry.first] = GetQueryFuncIdFromUsr(query_db, entry.second);
+    cached_func_ids_[entry.first.id] = GetQueryFuncIdFromUsr(query_db, entry.second).id;
 
-  cached_var_ids_.reserve(local_ids.var_id_to_usr.size());
+  cached_var_ids_.set_empty_key(-1);
+  cached_var_ids_.resize(local_ids.var_id_to_usr.size());
   for (const auto& entry : local_ids.var_id_to_usr)
-    cached_var_ids_[entry.first] = GetQueryVarIdFromUsr(query_db, entry.second);
+    cached_var_ids_[entry.first.id] = GetQueryVarIdFromUsr(query_db, entry.second).id;
+}
+
+QueryTypeId IdMap::ToQuery(IndexTypeId id) const {
+  assert(cached_type_ids_.find(id.id) != cached_type_ids_.end());
+  return QueryTypeId(cached_type_ids_.find(id.id)->second);
 }
 
+QueryFuncId IdMap::ToQuery(IndexFuncId id) const {
+  assert(cached_func_ids_.find(id.id) != cached_func_ids_.end());
+  return QueryFuncId(cached_func_ids_.find(id.id)->second);
+}
 
+QueryVarId IdMap::ToQuery(IndexVarId id) const {
+  assert(cached_var_ids_.find(id.id) != cached_var_ids_.end());
+  return QueryVarId(cached_var_ids_.find(id.id)->second);
+}
 
+SymbolIdx IdMap::ToSymbol(IndexTypeId id) const {
+  return SymbolIdx(SymbolKind::Type, ToQuery(id).id);
+}
+
+SymbolIdx IdMap::ToSymbol(IndexFuncId id) const {
+  return SymbolIdx(SymbolKind::Func, ToQuery(id).id);
+}
+
+SymbolIdx IdMap::ToSymbol(IndexVarId id) const {
+  return SymbolIdx(SymbolKind::Var, ToQuery(id).id);
+}
 
 
 
@@ -633,7 +611,7 @@ void IndexUpdate::Merge(const IndexUpdate& update) {
 
 
 
-void UpdateQualifiedName(QueryableDatabase* db, int* qualified_name_index, SymbolKind kind, int symbol_index, const std::string& name) {
+void UpdateQualifiedName(QueryableDatabase* db, size_t* qualified_name_index, SymbolKind kind, size_t symbol_index, const std::string& name) {
   if (*qualified_name_index == -1) {
     db->qualified_names.push_back(name);
     db->symbols.push_back(SymbolIdx(kind, symbol_index));

src/query.h

@@ -56,6 +56,38 @@ struct QueryableLocation {
   }
 };
 
+enum class SymbolKind { Invalid, File, Type, Func, Var };
+struct SymbolIdx {
+  SymbolKind kind;
+  size_t idx;
+
+  SymbolIdx() : kind(SymbolKind::Invalid), idx(-1) {} // Default ctor needed by stdlib. Do not use.
+  SymbolIdx(SymbolKind kind, uint64_t idx) : kind(kind), idx(idx) {}
+
+  bool operator==(const SymbolIdx& that) const {
+    return kind == that.kind && idx == that.idx;
+  }
+  bool operator!=(const SymbolIdx& that) const { return !(*this == that); }
+  bool operator<(const SymbolIdx& that) const {
+    return kind < that.kind || idx < that.idx;
+  }
+};
+
+struct SymbolRef {
+  SymbolIdx idx;
+  QueryableLocation loc;
+
+  SymbolRef(SymbolIdx idx, QueryableLocation loc) : idx(idx), loc(loc) {}
+
+  bool operator==(const SymbolRef& that) const {
+    return idx == that.idx && loc == that.loc;
+  }
+  bool operator!=(const SymbolRef& that) const { return !(*this == that); }
+  bool operator<(const SymbolRef& that) const {
+    return idx < that.idx && loc.range.start < that.loc.range.start;
+  }
+};
+
 struct UsrRef {
   Usr usr;
   QueryableLocation loc;
@@ -110,15 +142,15 @@ struct QueryableFile {
   struct Def {
     Usr usr;
     // Outline of the file (ie, for code lens).
-    std::vector<UsrRef> outline;
+    std::vector<SymbolRef> outline;
     // Every symbol found in the file (ie, for goto definition)
-    std::vector<UsrRef> all_symbols;
+    std::vector<SymbolRef> all_symbols;
   };
 
   using DefUpdate = Def;
 
   DefUpdate def;
-  int qualified_name_idx = -1;
+  size_t qualified_name_idx = -1;
 
   QueryableFile(const Usr& usr) { def.usr = usr; }
   QueryableFile(const Def& def) : def(def) {}
@@ -135,7 +167,7 @@ struct QueryableTypeDef {
   std::vector<Usr> derived;
   std::vector<Usr> instantiations;
   std::vector<QueryableLocation> uses;
-  int qualified_name_idx = -1;
+  size_t qualified_name_idx = -1;
 
   QueryableTypeDef(const Usr& usr) : def(usr) {}
   QueryableTypeDef(const DefUpdate& def) : def(def) {}
@@ -154,7 +186,7 @@ struct QueryableFuncDef {
   std::vector<Usr> derived;
   std::vector<UsrRef> callers;
   std::vector<QueryableLocation> uses;
-  int qualified_name_idx = -1;
+  size_t qualified_name_idx = -1;
 
   QueryableFuncDef(const Usr& usr) : def(usr) {}
   QueryableFuncDef(const DefUpdate& def) : def(def) {}
@@ -167,23 +199,13 @@ struct QueryableVarDef {
 
   DefUpdate def;
   std::vector<QueryableLocation> uses;
-  int qualified_name_idx = -1;
+  size_t qualified_name_idx = -1;
 
   QueryableVarDef(const Usr& usr) : def(usr) {}
   QueryableVarDef(const DefUpdate& def) : def(def) {}
   QueryableVarDef(const IdMap& id_map, const IndexedVarDef& indexed);
 };
 
-enum class SymbolKind { Invalid, File, Type, Func, Var };
-struct SymbolIdx {
-  SymbolKind kind;
-  uint64_t idx;
-
-  SymbolIdx() : kind(SymbolKind::Invalid), idx(-1) {} // Default ctor needed by stdlib. Do not use.
-  SymbolIdx(SymbolKind kind, uint64_t idx) : kind(kind), idx(idx) {}
-};
-
-
 struct IndexUpdate {
   // Creates a new IndexUpdate based on the delta from previous to current. If
   // no delta computation should be done just pass null for previous.
@@ -272,10 +294,18 @@ struct IdMap {
 
   IdMap(QueryableDatabase* query_db, const IdCache& local_ids);
 
+  QueryTypeId ToQuery(IndexTypeId id) const;
+  QueryFuncId ToQuery(IndexFuncId id) const;
+  QueryVarId ToQuery(IndexVarId id) const;
+  SymbolIdx ToSymbol(IndexTypeId id) const;
+  SymbolIdx ToSymbol(IndexFuncId id) const;
+  SymbolIdx ToSymbol(IndexVarId id) const;
+
   // TODO
 private:
   QueryFileId index_file_id;
-  std::unordered_map<IndexTypeId, QueryTypeId> cached_type_ids_;
-  std::unordered_map<IndexFuncId, QueryFuncId> cached_func_ids_;
-  std::unordered_map<IndexVarId, QueryVarId> cached_var_ids_;
+  // TODO: make these type safe
+  google::dense_hash_map<size_t, size_t> cached_type_ids_; // IndexTypeId -> QueryTypeId
+  google::dense_hash_map<size_t, size_t> cached_func_ids_; // IndexFuncId -> QueryFuncId
+  google::dense_hash_map<size_t, size_t> cached_var_ids_;  // IndexVarId  -> QueryVarId
 };

src/test.cc

@@ -56,7 +56,7 @@ void DiffDocuments(std::string path, rapidjson::Document& expected, rapidjson::D
 
   int max_diff = 5;
 
-  int len = std::min(actual_output.size(), expected_output.size());
+  size_t len = std::min(actual_output.size(), expected_output.size());
   for (int i = 0; i < len; ++i) {
     if (actual_output[i] != expected_output[i]) {
       if (--max_diff < 0) {