[PATCH] D127663: [clang][lex] NFCI: Use DirectoryEntryRef in HeaderSearch::LookupFile

[Jan Svoboda via Phabricator via cfe-commits]

jansvoboda11 added a comment.

The failing test was a fun one to debug. It didn't test what it intended to. It wanted to check that header referenced from module map is found via a search path, but the command line was constructed in such way that the header was found relative to the includer. The test relied on the `FileManager` hackery that mutates `FileEntry` objects. Now that we call `SourceMgr.getFileEntryRefForID()`, we don't see effects of this hack. I ended up only fixing the test, since removing the `FileManager` hack safely probably requires us to remove all remaining `{File,Directory}Entry::getName()` usages.

LMK if you have more feedback.



================
Comment at: clang/lib/Frontend/FrontendAction.cpp:811
       // Relative searches begin from CWD.
-      const DirectoryEntry *Dir = nullptr;
-      if (auto DirOrErr = CI.getFileManager().getDirectory("."))
-        Dir = *DirOrErr;
-      SmallVector<std::pair<const FileEntry *, const DirectoryEntry *>, 1> CWD;
-      CWD.push_back({nullptr, Dir});
+      auto Dir = CI.getFileManager().getOptionalDirectoryRef(".");
+      SmallVector<std::pair<const FileEntry *, DirectoryEntryRef>, 1> CWD;
----------------
bnbarham wrote:
> Worth adding an assert here? It'd be nice to cleanup the CWD handling in FileManager at some point.
Assert that `Dir` is not empty? Seems overly defensive to me, what would be the benefit of that? Catching if the working directory was yanked from underneath Clang?


================
Comment at: clang/lib/Lex/PPDirectives.cpp:891
   // stack, record the parent #includes.
-  SmallVector<std::pair<const FileEntry *, const DirectoryEntry *>, 16>
-      Includers;
+  SmallVector<std::pair<const FileEntry *, DirectoryEntryRef>, 16> Includers;
   bool BuildSystemModule = false;
----------------
bnbarham wrote:
> Could we change this to a FileEntryRef as well, or would you prefer to do that later? It'll touch basically all the same places again (eg. each parameter above).
I'd prefer to do it separately for smaller diff and easier bisection. LMK if you have strong preference for doing it in the same commit.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127663