[PATCH] D63295: [clang][HeaderSearch] Shorten paths for includes in mainfile's directory

Haojian Wu via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Tue Jul 2 02:28:34 PDT 2019


hokein added a comment.

looks mostly good, a few nits.



================
Comment at: clang/include/clang/Lex/HeaderSearch.h:724
+  /// slashes as separators. MainFile is the absolute path of the file that we
+  /// are generating the diagnostics for. It can be empty.
   ///
----------------
I think we should have some documentation about this new behavior in the comment here.


================
Comment at: clang/lib/Sema/SemaLookup.cpp:5245
+  llvm::StringRef IncludingFile =
+      SourceMgr.getFileEntryForID(SourceMgr.getFileID(UseLoc))
+          ->tryGetRealPathName();
----------------
it seems not safe, getFileEntryForID may return a nullptr...


================
Comment at: clang/unittests/Lex/HeaderSearchTest.cpp:124
+                                                   /*WorkingDir=*/"",
+                                                   "/y/a.cc"),
+            "z/t.h");
----------------
nit: add `/*MainFile=*/` comment annotation to improve the code readability.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D63295





More information about the cfe-commits mailing list