harlanhaskins marked 2 inline comments as done. harlanhaskins added inline comments.
================ Comment at: clang/include/clang/Basic/FileManager.h:143 /// - llvm::StringMap<DirectoryEntry*, llvm::BumpPtrAllocator> SeenDirEntries; + llvm::StringMap<llvm::ErrorOr<DirectoryEntry*>, llvm::BumpPtrAllocator> + SeenDirEntries; ---------------- jkorous wrote: > Maybe we could replace this with some type that has hard-to-use-incorrectly > interface instead of using `containsValue()`. You're right, these should store `llvm::ErrorOr<DirectoryEntry &>` and use `std::err::no_such_file_or_directory` as a placeholder instead of `nullptr`. ================ Comment at: clang/include/clang/Basic/FileManager.h:217 /// - /// This returns NULL if the file doesn't exist. + /// This returns a \c std::error_code if there was an error loading the file. /// ---------------- jkorous wrote: > Does that mean that it's now safe to assume the value is `!= NULL` in the > absence of errors? That's the intent of these changes, yes, but it should also be documented. 👍 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D65534/new/ https://reviews.llvm.org/D65534 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits