[PATCH] D51359: Adding HardLink Support to VirtualFileSystem.

[UTKARSH SAXENA via Phabricator via cfe-commits]

usaxena95 added a comment.

Made the suggested changes.



================
Comment at: include/clang/Basic/VirtualFileSystem.h:359
+  public:
+  /// Add a HardLink to a File.
+  /// The To path must be an existing file or a hardlink. The From file must not
----------------
ilya-biryukov wrote:
> Maybe document that it does nothing for directories?
> Also, any reason to not support directories at this point? The limitation seem artificial at this point.
Links to directories cannot be  added with the current logic. Resolving paths and iterating directories becomes non trivial. Current implementation supports the use case of "file with multiple names" which is mostly sufficient to mimic the compilation.


================
Comment at: lib/Basic/VirtualFileSystem.cpp:529
+public:
+  InMemoryHardLink(Status Stat, InMemoryNode *ResolvedNode)
+      : InMemoryNode(std::move(Stat), IME_HARD_LINK),
----------------
ilya-biryukov wrote:
> usaxena95 wrote:
> > 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)
> Both methods could be made const, though, right?
> It looks like the buffers are always copied when returned from the file system (because the VFS interface returns a `unique_ptr<MemoryBuffer>`).
> `getResolvedFile` can also return a const node.
Since a raw pointer to the resolved file is returned by getResolvedNode (used by for lookup) , I have also changed some other raw pointer type to const pointer tpye.


Repository:
  rC Clang

https://reviews.llvm.org/D51359