simark marked 6 inline comments as done.
simark added inline comments.
================
Comment at: lib/Basic/VirtualFileSystem.cpp:516
+ explicit InMemoryFileAdaptor(InMemoryFile &Node, std::string RequestedName)
+ : Node(Node), RequestedName (std::move (RequestedName))
+ {}
----------------
ilya-biryukov wrote:
> NIT: The formatting is broken here.
Hmm this is what git-clang-format does (unless this comment refers to a
previous version where I had not run clang-format).
================
Comment at: lib/Basic/VirtualFileSystem.cpp:520
+ llvm::ErrorOr<Status> status() override {
+ Status St = Node.getStatus();
+ return Status::copyWithNewName(St, RequestedName);
----------------
ilya-biryukov wrote:
> Maybe add a `RequestedName` parameter to the `InMemoryNode` instead to make
> sure it's not misused?
> It looks like all the clients calling it have to change the name and some are
> not doing it now, e.g. the directory iterator will use statuses with full
> paths.
Ok, I tried to do something like that.
================
Comment at: lib/Basic/VirtualFileSystem.cpp:521
+ Status St = Node.getStatus();
+ return Status::copyWithNewName(St, RequestedName);
+ }
----------------
ilya-biryukov wrote:
> Maybe add a comment that this matches the real file system behavior?
I added a comment to `InMemoryNode::getSatus`, since that's where the
`copyWithNewName` is done now.
================
Comment at: lib/Basic/VirtualFileSystem.cpp:724
+ Status St = (*Node)->getStatus();
+ return Status::copyWithNewName(St, Path.str());
+ }
----------------
ilya-biryukov wrote:
> NIT: we don't need `str()` here
Otherwise I'm getting this:
```
/home/emaisin/src/llvm/tools/clang/lib/Basic/VirtualFileSystem.cpp:1673:9:
error: no matching function for call to 'copyWithNewName'
S = Status::copyWithNewName(S, Path);
^~~~~~~~~~~~~~~~~~~~~~~
/home/emaisin/src/llvm/tools/clang/lib/Basic/VirtualFileSystem.cpp:76:16: note:
candidate function not viable: no known conversion from 'const llvm::Twine' to
'llvm::StringRef' for 2nd argument
Status Status::copyWithNewName(const Status &In, StringRef NewName) {
^
/home/emaisin/src/llvm/tools/clang/lib/Basic/VirtualFileSystem.cpp:82:16: note:
candidate function not viable: no known conversion from 'clang::vfs::Status' to
'const llvm::sys::fs::file_status' for 1st argument
Status Status::copyWithNewName(const file_status &In, StringRef NewName) {
^
```
Repository:
rC Clang
https://reviews.llvm.org/D48903
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
