[PATCH] D51359: Adding HardLink Support to VirtualFileSystem.

[UTKARSH SAXENA via Phabricator via cfe-commits]

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
+public:
+  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.


Repository:
  rC Clang

https://reviews.llvm.org/D51359