[PATCH] D48903: [VirtualFileSystem] InMemoryFileSystem::status: Return a Status with the requested name

Ilya Biryukov via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Tue Jul 10 01:03:44 PDT 2018


ilya-biryukov added inline comments.


================
Comment at: lib/Basic/FileManager.cpp:320
+  SmallString<128> RealPathName;
+  if (FS->getRealPath(InterndFileName, RealPathName) == std::error_code())
+    UFE.RealPathName = RealPathName.str();
----------------
NIT: replace replace equality with negative test, i.e. `if (!FS->getRealPath(…))`

I'm not a big fan of bash-like error codes, but that seems to be the idiomatic way to use them.


================
Comment at: lib/Basic/VirtualFileSystem.cpp:488
+  }
+  StringRef getName() const { return Stat.getName(); }
   InMemoryNodeKind getKind() const { return Kind; }
----------------
simark wrote:
> ilya-biryukov wrote:
> > Given that this method is inconsistent with `getStatus()` and seems to be only used in `toString` methods, maybe we could make it `protected`? Otherwise it's really easy to write code that gets the wrong path.
> I now use it in `InMemoryDirIterator::setCurrentEntry`.  I will write a comment to the `getName` method to clarify this.
`getFileName` as a public method and its usage in setCurrentEntry LG , thanks!


================
Comment at: lib/Basic/VirtualFileSystem.cpp:477
+protected:
+  Status Stat;
+
----------------
The inheritors should not be able to modify this field.
Can we get away with a private field and a protected getter that returns a const reference instead?


Repository:
  rC Clang

https://reviews.llvm.org/D48903





More information about the cfe-commits mailing list