This is an archive of the discontinued LLVM Phabricator instance.

[VFS] Cleanups to VFS interfaces.
ClosedPublic

Authored by sammccall on Jul 24 2018, 4:33 AM.

Details

Summary
  • add comments clarifying semantics
  • Status::copyWithNewName(Status, Name) --> instance method
  • Status::copyWithNewName(fs::file_status, Name) --> constructor (it's not a copy)
  • File::getName() -> getRealPath(), reflecting its actual behavior/function and stop returning status().getName() in the base class (callers can do this fallback if they want to, it complicates the contracts).

This is mostly NFC, but the behavior of File::getName() affects FileManager's
FileEntry::tryGetRealPathName(), which now fails in more cases:

  • non-real file cases
  • real-file cases where the underlying vfs::File was opened in a way that doesn't call realpath().

(In these cases we don't know a distinct real name, so in principle it seems OK)

Diff Detail

Repository
rL LLVM

Event Timeline

sammccall created this revision.Jul 24 2018, 4:33 AM
This revision is now accepted and ready to land.Jul 24 2018, 4:43 AM
This revision was automatically updated to reflect the committed changes.

Looks like this caused some test failures (http://lab.llvm.org:8011/builders/clang-cmake-aarch64-quick/builds/14441/steps/ninja%20check%201), e.g.:

[ RUN      ] GoToInclude.All
/home/buildslave/buildslave/clang-cmake-aarch64-quick/llvm/tools/clang/tools/extra/unittests/clangd/XRefsTests.cpp:953: Failure
Value of: *Locations
Expected: has 1 element that is equal to 0:0-0:0@file:///clangd-test/foo.h
  Actual: {}

I'm not familiar with this patch, but reverting it locally seems to get tests passing again, so I'd like to revert this change temporarily.

For the record: this got reverted in rL337850