[PATCH] D65907: Introduce FileEntryRef and use it when handling includes to report correct dependencies when the FileManager is reused across invocations

[Bruno Cardoso Lopes via Phabricator via cfe-commits]

bruno added inline comments.


================
Comment at: clang/include/clang/Basic/FileManager.h:110
+/// A reference to a \c FileEntry that includes the name of the file as it was
+/// accessed by the FileManager's client.
+class FileEntryRef {
----------------
How does that work with the VFS? Will it keep the virtual path as the FileName to return with getName() or the external-content? Should it change when in face of `use-external-names` attribute?


================
Comment at: clang/include/clang/Basic/FileManager.h:249-251
+  /// This function is deprecated and will be removed at some point in the
+  /// future, new clients should use
+  ///  \c getFileRef.
----------------
Bigcheese wrote:
> `LLVM_DEPRECATED()`?  (or w/e the name is of our depreciation attribute macro).
Probably can't be done until we move all uses over? There's probably still enough of them that the warning would be very annoying?


================
Comment at: clang/lib/Basic/FileManager.cpp:194
+static llvm::ErrorOr<FileEntryRef>
+promoteFileRef(StringRef name, llvm::ErrorOr<FileEntry &> value) {
+  if (value)
----------------
I know the surrounding functions are not consistent with capitalization, but maybe `name` and `value` here should?


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

https://reviews.llvm.org/D65907