jansvoboda11 added inline comments.
================ Comment at: clang/include/clang/Basic/FileManager.h:316 + void getSeenFileEntries( + SmallVectorImpl<OptionalFileEntryRefDegradesToFileEntryPtr> &Entries) + const; ---------------- rmaz wrote: > jansvoboda11 wrote: > > Since we're already modifying the two only users of this function, maybe we > > could use `Optional<FileEntryRef>` instead of > > `OptionalFileEntryRefDegradesToFileEntryPtr` (which we want to eventually > > remove)? > We could do that with less churn once more or the called methods in the loop > over the files are refactored in D142724, but I'm fine with either approach. Hmm, thinking about this more, it doesn't make sense for this function to return empty `Optional<FileEntryRef>`. Changing the parameter type to `SmallVectorImpl<FileEntryRef> &` and updating the clients to use `.` instead of `->` is the most clear/correct option IMO. I think the function name & documentation should point out that we're only returning files non-vfs-redirected files (and vfs-redirected files that don't use the external name). ================ Comment at: clang/lib/Basic/FileManager.cpp:20 #include "clang/Basic/FileManager.h" +#include "clang/Basic/FileEntry.h" #include "clang/Basic/FileSystemStatCache.h" ---------------- Already included in `FileManager.h`. ================ Comment at: clang/lib/Serialization/ASTWriter.cpp:39 #include "clang/Basic/DiagnosticOptions.h" +#include "clang/Basic/FileEntry.h" #include "clang/Basic/FileManager.h" ---------------- Already provided by `FileManager.h`. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D143414/new/ https://reviews.llvm.org/D143414 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
