[PATCH] D116750: [clang][lex] Keep references to `DirectoryLookup` objects up-to-date

[Alex Hoppen via Phabricator via cfe-commits]

ahoppen added a comment.

I like this a lot better. Some comments inline.



================
Comment at: clang/include/clang/Lex/HeaderSearch.h:179
   /// directory is suppressed.
-  std::vector<DirectoryLookup> SearchDirs;
-  /// Whether the DirectoryLookup at the corresponding index in SearchDirs has
-  /// been successfully used to lookup a file.
-  std::vector<bool> SearchDirsUsage;
+  std::vector<DirectoryLookup *> SearchDirs;
+  /// Set of SearchDirs that have been successfully used to lookup a file.
----------------
I haven’t tried it but is there a particular reason why this can’t be a `const DirectoryLookup *`?


================
Comment at: clang/lib/Lex/HeaderSearch.cpp:323
+
+    if (SearchDirs[Idx]->isFramework()) {
       // Search for or infer a module map for a framework. Here we use
----------------
Nitpick: `SearchDirs[Idx]` can be simplified to `SearchDir->isFramework()`. Similarly below.


================
Comment at: clang/unittests/Lex/HeaderSearchTest.cpp:276
+    std::vector<bool> ExpectedSearchDirUsageAfterM2{false, true, false};
+    EXPECT_EQ(Search.getSearchDirUsage(), ExpectedSearchDirUsageAfterM2);
+    std::vector<bool> ExpectedUserEntryUsageAfterM2{false, true, false};
----------------
Wouldn’t it be cleaner to just check that `UsedSearchDirs` only contains a single element and that it’s name is `/M2`? In that case we could also remove `getSearchDirUsage`.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D116750/new/

https://reviews.llvm.org/D116750