Strengthen the indexed path guarantees

infra/indexer/frontend/frontend_test.cc

@@ -3118,5 +3118,22 @@ TEST(FrontendTest, ImplicitComparisonInstantiation) {
           RequiredEntityId(index, Entity::Kind::kClass, "", "TestTemplateClass",
                            "<T>", "snippet.cc", 11, 14)));
 }
+
+TEST(FrontendTest, CommandLineMacro) {
+  auto index = IndexSnippet("int MACRO;", {"-DMACRO=expansion"})->Export();
+  EXPECT_HAS_ENTITY(index, Entity::Kind::kVariable, "", "expansion", "",
+                    "snippet.cc", 1, 1);
+  int found = 0;
+  for (const auto& index_entity : index.entities) {
+    if (index_entity.full_name() == "MACRO") {
+      EXPECT_EQ(index_entity.kind(), Entity::Kind::kMacro);
+      // NOTE(kartynnik): Why isn't this `<command line>`?
+      EXPECT_EQ(index.locations[index_entity.location_id()].path(),
+                "<built-in>");
+      ++found;
+    }
+  }
+  EXPECT_EQ(found, 1);
+}
 }  // namespace indexer
 }  // namespace oss_fuzz

infra/indexer/index/file_copier.cc

@@ -51,10 +51,8 @@ FileCopier::FileCopier(absl::string_view base_path,
   }
 }
 
-std::string FileCopier::ToIndexPath(absl::string_view path) const {
-  if (!path.starts_with('/')) {
-    return std::string(path);
-  }
+std::string FileCopier::AbsoluteToIndexPath(absl::string_view path) const {
+  CHECK(path.starts_with("/")) << "Absolute path expected";
 
   std::string result = std::string(path);
   if (!base_path_.empty() && absl::StartsWith(path, base_path_)) {
@@ -72,15 +70,8 @@ std::string FileCopier::ToIndexPath(absl::string_view path) const {
 }
 
 void FileCopier::RegisterIndexedFile(absl::string_view index_path) {
-  if (index_path.empty() || index_path.starts_with('<')) {
-    // Built-in header or a location lacking the filename.
-    return;
-  }
-
-  {
-    absl::MutexLock lock(&mutex_);
-    indexed_files_.emplace(index_path);
-  }
+  absl::MutexLock lock(&mutex_);
+  indexed_files_.emplace(index_path);
 }
 
 void FileCopier::CopyIndexedFiles() {

infra/indexer/index/file_copier.h

@@ -36,9 +36,10 @@ class FileCopier {
              const std::vector<std::string>& extra_paths);
   FileCopier(const FileCopier&) = delete;
 
-  // Rewrites a path into the representation it will have in the index
-  // (relative if within the source tree and absolute otherwise).
-  std::string ToIndexPath(absl::string_view path) const;
+  // Takes an absolute path. Rewrites this path into the representation it will
+  // have in the index (relative if within the source tree and absolute
+  // otherwise).
+  std::string AbsoluteToIndexPath(absl::string_view path) const;
 
   // `index_path` is expected to be produced by `ToIndexPath`.
   void RegisterIndexedFile(absl::string_view index_path);

infra/indexer/index/file_copier_unittest.cc

@@ -43,25 +43,27 @@ std::optional<std::string> GetFileContents(const std::filesystem::path& path) {
 }
 }  // namespace
 
-TEST(FileCopierTest, ToIndexPath) {
+TEST(FileCopierTest, AbsoluteToIndexPath) {
   auto tmp_dir_path = std::filesystem::path(::testing::TempDir());
   auto base_path = tmp_dir_path / "src";
   auto index_path = tmp_dir_path / "idx";
   FileCopier file_copier(base_path.string(), index_path.string(), {"/"});
 
-  EXPECT_EQ(file_copier.ToIndexPath("/a/b/c/d.cc"), "/a/b/c/d.cc");
-  EXPECT_EQ(file_copier.ToIndexPath((base_path / "a/b/c/d.cc").string()),
-            "a/b/c/d.cc");
-  EXPECT_EQ(file_copier.ToIndexPath("a/b/c/d.cc"), "a/b/c/d.cc");
+  EXPECT_EQ(file_copier.AbsoluteToIndexPath("/a/b/c/d.cc"), "/a/b/c/d.cc");
+  EXPECT_EQ(
+      file_copier.AbsoluteToIndexPath((base_path / "a/b/c/d.cc").string()),
+      "a/b/c/d.cc");
+  EXPECT_DEATH(file_copier.AbsoluteToIndexPath("a/b/c/d.cc"),
+               "Absolute path expected");
 }
 
-TEST(FileCopierTest, ToIndexPathOutside) {
+TEST(FileCopierTest, AbsoluteToIndexPathOutside) {
   auto tmp_dir_path = std::filesystem::path(::testing::TempDir());
   auto base_path = tmp_dir_path / "src";
   auto index_path = tmp_dir_path / "idx";
   FileCopier file_copier(base_path.string(), index_path.string(), {"/sysroot"});
 
-  EXPECT_DEATH(file_copier.ToIndexPath("/a/b/c/d.cc"), "/a/b/c/d.cc");
+  EXPECT_DEATH(file_copier.AbsoluteToIndexPath("/a/b/c/d.cc"), "/a/b/c/d.cc");
 }
 
 TEST(FileCopierTest, FileCopying) {
@@ -85,9 +87,12 @@ TEST(FileCopierTest, FileCopying) {
   auto file_c_copy = index_path / "absolute" /
                      sysroot_path.lexically_relative("/") / "y/z/c.cc";
 
-  file_copier.RegisterIndexedFile(file_copier.ToIndexPath(file_a.string()));
-  file_copier.RegisterIndexedFile(file_copier.ToIndexPath(file_b.string()));
-  file_copier.RegisterIndexedFile(file_copier.ToIndexPath(file_c.string()));
+  file_copier.RegisterIndexedFile(
+      file_copier.AbsoluteToIndexPath(file_a.string()));
+  file_copier.RegisterIndexedFile(
+      file_copier.AbsoluteToIndexPath(file_b.string()));
+  file_copier.RegisterIndexedFile(
+      file_copier.AbsoluteToIndexPath(file_c.string()));
 
   file_copier.CopyIndexedFiles();
 

infra/indexer/index/in_memory_index.cc

@@ -196,7 +196,7 @@ void InMemoryIndex::Merge(const InMemoryIndex& other) {
   std::vector<LocationId> new_location_ids(other.locations_.size(),
                                            kInvalidLocationId);
   for (const auto& [location, id] : other.locations_) {
-    new_location_ids[id] = GetLocationId(location);
+    new_location_ids[id] = GetIdForLocationWithIndexPath(location);
   }
 
   // We need to update the location_id for entities, and the entity_id and
@@ -276,15 +276,22 @@ void InMemoryIndex::Expand(size_t locations_count, size_t entities_count,
   references_.reserve(references_.size() + references_count);
 }
 
-LocationId InMemoryIndex::GetLocationId(const Location& location) {
-  // Adjust paths within the base_path to be relative paths.
-  Location new_location = location;
-  new_location.path_ = file_copier_.ToIndexPath(location.path());
+LocationId InMemoryIndex::GetLocationId(Location location) {
+  if (location.is_real()) {
+    // Adjust paths within the base_path to be relative paths.
+    location.path_ = file_copier_.AbsoluteToIndexPath(location.path());
+  }
+  return GetIdForLocationWithIndexPath(location);
+}
 
-  auto [iter, inserted] = locations_.insert({new_location, next_location_id_});
+LocationId InMemoryIndex::GetIdForLocationWithIndexPath(
+    const Location& location) {
+  auto [iter, inserted] = locations_.insert({location, next_location_id_});
   if (inserted) {
     next_location_id_++;
-    file_copier_.RegisterIndexedFile(new_location.path());
+    if (location.is_real()) {
+      file_copier_.RegisterIndexedFile(location.path());
+    }
   }
 
   return iter->second;

infra/indexer/index/in_memory_index.h

@@ -47,7 +47,8 @@ class InMemoryIndex {
   // The `GetXxxId` functions return the id of an existing, matching object if
   // there is already one in the index, or allocate a new id if there is not an
   // identical object in the index.
-  LocationId GetLocationId(const Location& location);
+  // `GetLocationId` expects a location with an absolute path if not built-in.
+  LocationId GetLocationId(Location location);
   EntityId GetEntityId(const Entity& entity);
   ReferenceId GetReferenceId(const Reference& reference);
 
@@ -57,6 +58,9 @@ class InMemoryIndex {
  private:
   FileCopier& file_copier_;
 
+  // Like `GetLocationId`, but requires the path to be already index-adjusted.
+  LocationId GetIdForLocationWithIndexPath(const Location& location);
+
   // Although we could sort location_lookup_ in advance, the performance impact
   // on indexing if we use a btree_map is significant, and it's much faster
   // to sort the index at the end.

infra/indexer/index/types.h

@@ -38,7 +38,8 @@ constexpr LocationId kInvalidLocationId = 0xffffffffffffffffull;
 constexpr EntityId kInvalidEntityId = 0xffffffffffffffffull;
 
 inline bool IsRealPath(absl::string_view path) {
-  return !path.empty() && !path.starts_with("<");
+  // Examples of built-in paths: `<built-in>` and `<command-line>`.
+  return !path.empty() && !path.starts_with('<');
 }
 
 // Represents a source-file location.