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



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to