ilya-biryukov added a comment.
Thanks for the change. Having something like this makes total sense.
Mutating existing in-memory nodes looks shaky and requires making `Status`
mutable, which also looks undesirable.
Maybe we could consider adding a new kind of `InMemoryNode` for hard links that
would store the link target? We would have to "resolve" the link on each
access, but that would probably allow to avoid changing `Status` interfaces and
get rid of the `UniqueID -> set of files' map. WDYT?
Also some generic comments wrt to the coding style, naming, etc.
================
Comment at: include/clang/Basic/VirtualFileSystem.h:80
+ void setUniqueID(llvm::sys::fs::UniqueID UID) { this->UID = UID; }
+ void setSize(uint64_t Size) { this->Size = Size; }
----------------
It seems `Status` was designed to be immutable.
Can we copy or reassign the whole status at the points where we need it?
================
Comment at: include/clang/Basic/VirtualFileSystem.h:359
+ /// Add a HardLink from one file to another.
+ /// Makes the UniqueID of To file same as that of From file. If CopyBuffer is
----------------
Maybe use the common spelling: "hard link"? Otherwise it feels like there is a
HardLink class somewhere.
================
Comment at: include/clang/Basic/VirtualFileSystem.h:362
+ /// true then contents of from buffer is copied into the buffer of To file.
+ void addHardlink(const Twine &From, const Twine &To, bool CopyBuffer);
+
----------------
The links seem to only work for files at this point.
Maybe explicitly document what happens with directories?
Also, maybe document what happens with existing hard links and existing files?
================
Comment at: include/clang/Basic/VirtualFileSystem.h:362
+ /// true then contents of from buffer is copied into the buffer of To file.
+ void addHardlink(const Twine &From, const Twine &To, bool CopyBuffer);
+
----------------
ilya-biryukov wrote:
> The links seem to only work for files at this point.
> Maybe explicitly document what happens with directories?
> Also, maybe document what happens with existing hard links and existing files?
Maybe s/addHardlink/addHardLink?
================
Comment at: include/clang/Basic/VirtualFileSystem.h:468
#endif // LLVM_CLANG_BASIC_VIRTUALFILESYSTEM_H
+
----------------
Accidental whitespace change?
================
Comment at: lib/Basic/VirtualFileSystem.cpp:516
+ void setBuffer(std::unique_ptr<llvm::MemoryBuffer> buffer) {
+ this->Buffer = std::move(buffer);
----------------
s/buffer/Buffer
================
Comment at: lib/Basic/VirtualFileSystem.cpp:645
+ // Merging ToIDSet into FromIDSet.
+ for (detail::InMemoryNode *to_node : ToIDSet) {
+ to_node->setUniqueId(FromStat->getUniqueID());
----------------
s/to_node/ToNode
================
Comment at: unittests/Basic/VirtualFileSystemTest.cpp:1608
}
+
----------------
Accidental whitespace change?
Repository:
rC Clang
https://reviews.llvm.org/D51359
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
