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