[PATCH] D51359: Adding HardLink Support to VirtualFileSystem.

UTKARSH SAXENA via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Thu Aug 30 07:52:54 PDT 2018

usaxena95 marked 7 inline comments as done.
usaxena95 added a comment.

> Could we try removing Status from the base class and move it into `InMemoryFile` and `InMemoryDir`?

Moved the Stat and **getStatus** into the subclasses. User of **getStatus** now calls **getStatus** of individual subclasses.

Comment at: lib/Basic/VirtualFileSystem.cpp:526
+  InMemoryNode *ResolvedNode;
+  StringRef Name;
ilya-biryukov wrote:
> Can we get away without storing `Name`? `toString()` can just say `"hard link to "  + ResolveNode->toString()`
Yeah we can do that. It is just for debug purpose only.

Comment at: lib/Basic/VirtualFileSystem.cpp:529
+  InMemoryHardLink(Status Stat, InMemoryNode *ResolvedNode)
+      : InMemoryNode(std::move(Stat), IME_HARD_LINK),
ilya-biryukov wrote:
> ilya-biryukov wrote:
> > Why do we need the Stat?
> > We can get it from `ResolveNode` if we want to store a copy. Any reason why it would be different?
> ResolveNode can't be `null`, right? Maybe use a reference instead?
> Also, maybe make it const?
Changed it to reference. Can't make it const as the there it is involved in methods that are non const (like getResolvedFile returns a pointer, getBuffer is not const)

Comment at: lib/Basic/VirtualFileSystem.cpp:535
+  InMemoryNode *getResolvedNode() { return ResolvedNode; }
+  llvm::MemoryBuffer *getBuffer() {
+    return static_cast<InMemoryFile *>(ResolvedNode)->getBuffer();
ilya-biryukov wrote:
> Could we inline `getBuffer` and `getStatus`? Having the client code that's explicit about resolving the references seems like a good idea.
Removed these methods. Moved the logic to client code.

  rC Clang


More information about the cfe-commits mailing list