[PATCH] D67026: Introduce a DirectoryEntryRef that stores both a reference and an accessed name to the directory entry

[Duncan P. N. Exon Smith via Phabricator via cfe-commits]

dexonsmith accepted this revision.
dexonsmith added a comment.
This revision is now accepted and ready to land.

A few minor comments.  Assuming the `FIXME` I point out was intentionally left for later, this LGTM.



================
Comment at: clang/include/clang/Basic/FileManager.h:59
+public:
+  const DirectoryEntry *getDirEntry() const { return &(*Entry->getValue()); }
+
----------------
Should this return `const Directory&` because it's guaranteed to be valid.


================
Comment at: clang/include/clang/Lex/DirectoryLookup.h:68-69
 public:
   /// DirectoryLookup ctor - Note that this ctor *does not take ownership* of
   /// 'dir'.
+  DirectoryLookup(DirectoryEntryRef Dir, SrcMgr::CharacteristicKind DT,
----------------
Since you're modifying this constructor it might be nice to cleanup the comment style (drop the `Directory ctor -` prefix).  Up to you.


================
Comment at: clang/lib/Lex/HeaderSearch.cpp:299
 StringRef DirectoryLookup::getName() const {
+  // FIXME: Use the name from \c DirectoryEntryRef.
   if (isNormalDir())
----------------
Did you mean to leave this behind for a future commit?  Or did you miss fixing this?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67026