diff --git a/infra/indexer/frontend/frontend_test.cc b/infra/indexer/frontend/frontend_test.cc index 415945859606..c7ebb7d38167 100644 --- a/infra/indexer/frontend/frontend_test.cc +++ b/infra/indexer/frontend/frontend_test.cc @@ -3118,5 +3118,22 @@ TEST(FrontendTest, ImplicitComparisonInstantiation) { RequiredEntityId(index, Entity::Kind::kClass, "", "TestTemplateClass", "", "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 ``? + EXPECT_EQ(index.locations[index_entity.location_id()].path(), + ""); + ++found; + } + } + EXPECT_EQ(found, 1); +} } // namespace indexer } // namespace oss_fuzz diff --git a/infra/indexer/index/file_copier.cc b/infra/indexer/index/file_copier.cc index 09f3cf926345..9a4673d6ed24 100644 --- a/infra/indexer/index/file_copier.cc +++ b/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() { diff --git a/infra/indexer/index/file_copier.h b/infra/indexer/index/file_copier.h index 0f51eb05cca8..c010512803c5 100644 --- a/infra/indexer/index/file_copier.h +++ b/infra/indexer/index/file_copier.h @@ -36,9 +36,10 @@ class FileCopier { const std::vector& 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); diff --git a/infra/indexer/index/file_copier_unittest.cc b/infra/indexer/index/file_copier_unittest.cc index bc9d35abd9ff..034bcdea100e 100644 --- a/infra/indexer/index/file_copier_unittest.cc +++ b/infra/indexer/index/file_copier_unittest.cc @@ -43,25 +43,27 @@ std::optional 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(); diff --git a/infra/indexer/index/in_memory_index.cc b/infra/indexer/index/in_memory_index.cc index b148f5434701..fbe3b52d095e 100644 --- a/infra/indexer/index/in_memory_index.cc +++ b/infra/indexer/index/in_memory_index.cc @@ -196,7 +196,7 @@ void InMemoryIndex::Merge(const InMemoryIndex& other) { std::vector 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; diff --git a/infra/indexer/index/in_memory_index.h b/infra/indexer/index/in_memory_index.h index b01fb3b9f071..1ff5bd05dace 100644 --- a/infra/indexer/index/in_memory_index.h +++ b/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. diff --git a/infra/indexer/index/types.h b/infra/indexer/index/types.h index 81da32daacc0..065da80e95ee 100644 --- a/infra/indexer/index/types.h +++ b/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: `` and ``. + return !path.empty() && !path.starts_with('<'); } // Represents a source-file location.