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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
