Re: r338057 - [VirtualFileSystem] InMemoryFileSystem::status: Return a Status with the requested name
I reverted this in r338084 because it broke clang tests on Windows:
http://lab.llvm.org:8011/builders/clang-x86-windows-msvc2015/builds/12916
On Thu, Jul 26, 2018 at 11:55 AM Simon Marchi via cfe-commits <
cfe-commits@lists.llvm.org> wrote:
> Author: simark
> Date: Thu Jul 26 11:55:02 2018
> New Revision: 338057
>
> URL: http://llvm.org/viewvc/llvm-project?rev=338057&view=rev
> Log:
> [VirtualFileSystem] InMemoryFileSystem::status: Return a Status with the
> requested name
>
> Summary:
>
> InMemoryFileSystem::status behaves differently than
> RealFileSystem::status. The Name contained in the Status returned by
> RealFileSystem::status will be the path as requested by the caller,
> whereas InMemoryFileSystem::status returns the normalized path.
>
> For example, when requested the status for "../src/first.h",
> RealFileSystem returns a Status with "../src/first.h" as the Name.
> InMemoryFileSystem returns "/absolute/path/to/src/first.h".
>
> The reason for this change is that I want to make a unit test in the
> clangd testsuite (where we use an InMemoryFileSystem) to reproduce a
> bug I get with the clangd program (where a RealFileSystem is used).
> This difference in behavior "hides" the bug in the unit test version.
>
> Reviewers: malaperle, ilya-biryukov, bkramer
>
> Subscribers: cfe-commits, ioeric, ilya-biryukov, bkramer, hokein, omtcyfz
>
> Differential Revision: https://reviews.llvm.org/D48903
>
> Modified:
> cfe/trunk/lib/Basic/FileManager.cpp
> cfe/trunk/lib/Basic/VirtualFileSystem.cpp
> cfe/trunk/unittests/Basic/VirtualFileSystemTest.cpp
> cfe/trunk/unittests/Driver/ToolChainTest.cpp
>
> Modified: cfe/trunk/lib/Basic/FileManager.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Basic/FileManager.cpp?rev=338057&r1=338056&r2=338057&view=diff
>
> ==
> --- cfe/trunk/lib/Basic/FileManager.cpp (original)
> +++ cfe/trunk/lib/Basic/FileManager.cpp Thu Jul 26 11:55:02 2018
> @@ -315,9 +315,11 @@ const FileEntry *FileManager::getFile(St
>UFE.InPCH = Data.InPCH;
>UFE.File = std::move(F);
>UFE.IsValid = true;
> - if (UFE.File)
> -if (auto RealPathName = UFE.File->getName())
> - UFE.RealPathName = *RealPathName;
> +
> + SmallString<128> RealPathName;
> + if (!FS->getRealPath(InterndFileName, RealPathName))
> +UFE.RealPathName = RealPathName.str();
> +
>return &UFE;
> }
>
>
> Modified: cfe/trunk/lib/Basic/VirtualFileSystem.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Basic/VirtualFileSystem.cpp?rev=338057&r1=338056&r2=338057&view=diff
>
> ==
> --- cfe/trunk/lib/Basic/VirtualFileSystem.cpp (original)
> +++ cfe/trunk/lib/Basic/VirtualFileSystem.cpp Thu Jul 26 11:55:02 2018
> @@ -474,12 +474,28 @@ class InMemoryNode {
>Status Stat;
>InMemoryNodeKind Kind;
>
> +protected:
> + /// Return Stat. This should only be used for internal/debugging use.
> When
> + /// clients wants the Status of this node, they should use
> + /// \p getStatus(StringRef).
> + const Status &getStatus() const { return Stat; }
> +
> public:
>InMemoryNode(Status Stat, InMemoryNodeKind Kind)
>: Stat(std::move(Stat)), Kind(Kind) {}
>virtual ~InMemoryNode() = default;
>
> - const Status &getStatus() const { return Stat; }
> + /// Return the \p Status for this node. \p RequestedName should be the
> name
> + /// through which the caller referred to this node. It will override
> + /// \p Status::Name in the return value, to mimic the behavior of \p
> RealFile.
> + Status getStatus(StringRef RequestedName) const {
> +return Status::copyWithNewName(Stat, RequestedName);
> + }
> +
> + /// Get the filename of this node (the name without the directory part).
> + StringRef getFileName() const {
> +return llvm::sys::path::filename(Stat.getName());
> + }
>InMemoryNodeKind getKind() const { return Kind; }
>virtual std::string toString(unsigned Indent) const = 0;
> };
> @@ -504,14 +520,21 @@ public:
>}
> };
>
> -/// Adapt a InMemoryFile for VFS' File interface.
> +/// Adapt a InMemoryFile for VFS' File interface. The goal is to make
> +/// \p InMemoryFileAdaptor mimic as much as possible the behavior of
> +/// \p RealFile.
> class InMemoryFileAdaptor : public File {
>InMemoryFile &Node;
> + /// The name to use when returning a Status for this file.
> + std::string RequestedName;
>
> public:
> - explicit InMemoryFileAdaptor(InMemoryFile &Node) : Node(Node) {}
> + explicit InMemoryFileAdaptor(InMemoryFile &Node, std::string
> RequestedName)
> + : Node(Node), RequestedName(std::move(RequestedName)) {}
>
> - llvm::ErrorOr status() override { return Node.getStatus(); }
> + llvm::ErrorOr status() override {
> +return Node.getStatus(RequestedName);
> + }
>
>llvm::ErrorOr>
>getBuffer(const Twine &Name, int64_t FileSize, bool
> RequiresNullTerminator
r338084 - Revert r338057 "[VirtualFileSystem] InMemoryFileSystem::status: Return a Status with the requested name"
Author: rnk
Date: Thu Jul 26 16:21:51 2018
New Revision: 338084
URL: http://llvm.org/viewvc/llvm-project?rev=338084&view=rev
Log:
Revert r338057 "[VirtualFileSystem] InMemoryFileSystem::status: Return a Status
with the requested name"
This broke clang/test/PCH/case-insensitive-include.c on Windows.
Modified:
cfe/trunk/lib/Basic/FileManager.cpp
cfe/trunk/lib/Basic/VirtualFileSystem.cpp
cfe/trunk/unittests/Basic/VirtualFileSystemTest.cpp
cfe/trunk/unittests/Driver/ToolChainTest.cpp
Modified: cfe/trunk/lib/Basic/FileManager.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Basic/FileManager.cpp?rev=338084&r1=338083&r2=338084&view=diff
==
--- cfe/trunk/lib/Basic/FileManager.cpp (original)
+++ cfe/trunk/lib/Basic/FileManager.cpp Thu Jul 26 16:21:51 2018
@@ -315,11 +315,9 @@ const FileEntry *FileManager::getFile(St
UFE.InPCH = Data.InPCH;
UFE.File = std::move(F);
UFE.IsValid = true;
-
- SmallString<128> RealPathName;
- if (!FS->getRealPath(InterndFileName, RealPathName))
-UFE.RealPathName = RealPathName.str();
-
+ if (UFE.File)
+if (auto RealPathName = UFE.File->getName())
+ UFE.RealPathName = *RealPathName;
return &UFE;
}
Modified: cfe/trunk/lib/Basic/VirtualFileSystem.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Basic/VirtualFileSystem.cpp?rev=338084&r1=338083&r2=338084&view=diff
==
--- cfe/trunk/lib/Basic/VirtualFileSystem.cpp (original)
+++ cfe/trunk/lib/Basic/VirtualFileSystem.cpp Thu Jul 26 16:21:51 2018
@@ -474,28 +474,12 @@ class InMemoryNode {
Status Stat;
InMemoryNodeKind Kind;
-protected:
- /// Return Stat. This should only be used for internal/debugging use. When
- /// clients wants the Status of this node, they should use
- /// \p getStatus(StringRef).
- const Status &getStatus() const { return Stat; }
-
public:
InMemoryNode(Status Stat, InMemoryNodeKind Kind)
: Stat(std::move(Stat)), Kind(Kind) {}
virtual ~InMemoryNode() = default;
- /// Return the \p Status for this node. \p RequestedName should be the name
- /// through which the caller referred to this node. It will override
- /// \p Status::Name in the return value, to mimic the behavior of \p
RealFile.
- Status getStatus(StringRef RequestedName) const {
-return Status::copyWithNewName(Stat, RequestedName);
- }
-
- /// Get the filename of this node (the name without the directory part).
- StringRef getFileName() const {
-return llvm::sys::path::filename(Stat.getName());
- }
+ const Status &getStatus() const { return Stat; }
InMemoryNodeKind getKind() const { return Kind; }
virtual std::string toString(unsigned Indent) const = 0;
};
@@ -520,21 +504,14 @@ public:
}
};
-/// Adapt a InMemoryFile for VFS' File interface. The goal is to make
-/// \p InMemoryFileAdaptor mimic as much as possible the behavior of
-/// \p RealFile.
+/// Adapt a InMemoryFile for VFS' File interface.
class InMemoryFileAdaptor : public File {
InMemoryFile &Node;
- /// The name to use when returning a Status for this file.
- std::string RequestedName;
public:
- explicit InMemoryFileAdaptor(InMemoryFile &Node, std::string RequestedName)
- : Node(Node), RequestedName(std::move(RequestedName)) {}
+ explicit InMemoryFileAdaptor(InMemoryFile &Node) : Node(Node) {}
- llvm::ErrorOr status() override {
-return Node.getStatus(RequestedName);
- }
+ llvm::ErrorOr status() override { return Node.getStatus(); }
llvm::ErrorOr>
getBuffer(const Twine &Name, int64_t FileSize, bool RequiresNullTerminator,
@@ -734,7 +711,7 @@ lookupInMemoryNode(const InMemoryFileSys
llvm::ErrorOr InMemoryFileSystem::status(const Twine &Path) {
auto Node = lookupInMemoryNode(*this, Root.get(), Path);
if (Node)
-return (*Node)->getStatus(Path.str());
+return (*Node)->getStatus();
return Node.getError();
}
@@ -747,8 +724,7 @@ InMemoryFileSystem::openFileForRead(cons
// When we have a file provide a heap-allocated wrapper for the memory buffer
// to match the ownership semantics for File.
if (auto *F = dyn_cast(*Node))
-return std::unique_ptr(
-new detail::InMemoryFileAdaptor(*F, Path.str()));
+return std::unique_ptr(new detail::InMemoryFileAdaptor(*F));
// FIXME: errc::not_a_file?
return make_error_code(llvm::errc::invalid_argument);
@@ -760,33 +736,21 @@ namespace {
class InMemoryDirIterator : public clang::vfs::detail::DirIterImpl {
detail::InMemoryDirectory::const_iterator I;
detail::InMemoryDirectory::const_iterator E;
- std::string RequestedDirName;
-
- void setCurrentEntry() {
-if (I != E) {
- SmallString<256> Path(RequestedDirName);
- llvm::sys::path::append(Path, I->second->getFileName());
- Cu
r338057 - [VirtualFileSystem] InMemoryFileSystem::status: Return a Status with the requested name
Author: simark
Date: Thu Jul 26 11:55:02 2018
New Revision: 338057
URL: http://llvm.org/viewvc/llvm-project?rev=338057&view=rev
Log:
[VirtualFileSystem] InMemoryFileSystem::status: Return a Status with the
requested name
Summary:
InMemoryFileSystem::status behaves differently than
RealFileSystem::status. The Name contained in the Status returned by
RealFileSystem::status will be the path as requested by the caller,
whereas InMemoryFileSystem::status returns the normalized path.
For example, when requested the status for "../src/first.h",
RealFileSystem returns a Status with "../src/first.h" as the Name.
InMemoryFileSystem returns "/absolute/path/to/src/first.h".
The reason for this change is that I want to make a unit test in the
clangd testsuite (where we use an InMemoryFileSystem) to reproduce a
bug I get with the clangd program (where a RealFileSystem is used).
This difference in behavior "hides" the bug in the unit test version.
Reviewers: malaperle, ilya-biryukov, bkramer
Subscribers: cfe-commits, ioeric, ilya-biryukov, bkramer, hokein, omtcyfz
Differential Revision: https://reviews.llvm.org/D48903
Modified:
cfe/trunk/lib/Basic/FileManager.cpp
cfe/trunk/lib/Basic/VirtualFileSystem.cpp
cfe/trunk/unittests/Basic/VirtualFileSystemTest.cpp
cfe/trunk/unittests/Driver/ToolChainTest.cpp
Modified: cfe/trunk/lib/Basic/FileManager.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Basic/FileManager.cpp?rev=338057&r1=338056&r2=338057&view=diff
==
--- cfe/trunk/lib/Basic/FileManager.cpp (original)
+++ cfe/trunk/lib/Basic/FileManager.cpp Thu Jul 26 11:55:02 2018
@@ -315,9 +315,11 @@ const FileEntry *FileManager::getFile(St
UFE.InPCH = Data.InPCH;
UFE.File = std::move(F);
UFE.IsValid = true;
- if (UFE.File)
-if (auto RealPathName = UFE.File->getName())
- UFE.RealPathName = *RealPathName;
+
+ SmallString<128> RealPathName;
+ if (!FS->getRealPath(InterndFileName, RealPathName))
+UFE.RealPathName = RealPathName.str();
+
return &UFE;
}
Modified: cfe/trunk/lib/Basic/VirtualFileSystem.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Basic/VirtualFileSystem.cpp?rev=338057&r1=338056&r2=338057&view=diff
==
--- cfe/trunk/lib/Basic/VirtualFileSystem.cpp (original)
+++ cfe/trunk/lib/Basic/VirtualFileSystem.cpp Thu Jul 26 11:55:02 2018
@@ -474,12 +474,28 @@ class InMemoryNode {
Status Stat;
InMemoryNodeKind Kind;
+protected:
+ /// Return Stat. This should only be used for internal/debugging use. When
+ /// clients wants the Status of this node, they should use
+ /// \p getStatus(StringRef).
+ const Status &getStatus() const { return Stat; }
+
public:
InMemoryNode(Status Stat, InMemoryNodeKind Kind)
: Stat(std::move(Stat)), Kind(Kind) {}
virtual ~InMemoryNode() = default;
- const Status &getStatus() const { return Stat; }
+ /// Return the \p Status for this node. \p RequestedName should be the name
+ /// through which the caller referred to this node. It will override
+ /// \p Status::Name in the return value, to mimic the behavior of \p
RealFile.
+ Status getStatus(StringRef RequestedName) const {
+return Status::copyWithNewName(Stat, RequestedName);
+ }
+
+ /// Get the filename of this node (the name without the directory part).
+ StringRef getFileName() const {
+return llvm::sys::path::filename(Stat.getName());
+ }
InMemoryNodeKind getKind() const { return Kind; }
virtual std::string toString(unsigned Indent) const = 0;
};
@@ -504,14 +520,21 @@ public:
}
};
-/// Adapt a InMemoryFile for VFS' File interface.
+/// Adapt a InMemoryFile for VFS' File interface. The goal is to make
+/// \p InMemoryFileAdaptor mimic as much as possible the behavior of
+/// \p RealFile.
class InMemoryFileAdaptor : public File {
InMemoryFile &Node;
+ /// The name to use when returning a Status for this file.
+ std::string RequestedName;
public:
- explicit InMemoryFileAdaptor(InMemoryFile &Node) : Node(Node) {}
+ explicit InMemoryFileAdaptor(InMemoryFile &Node, std::string RequestedName)
+ : Node(Node), RequestedName(std::move(RequestedName)) {}
- llvm::ErrorOr status() override { return Node.getStatus(); }
+ llvm::ErrorOr status() override {
+return Node.getStatus(RequestedName);
+ }
llvm::ErrorOr>
getBuffer(const Twine &Name, int64_t FileSize, bool RequiresNullTerminator,
@@ -711,7 +734,7 @@ lookupInMemoryNode(const InMemoryFileSys
llvm::ErrorOr InMemoryFileSystem::status(const Twine &Path) {
auto Node = lookupInMemoryNode(*this, Root.get(), Path);
if (Node)
-return (*Node)->getStatus();
+return (*Node)->getStatus(Path.str());
return Node.getError();
}
@@ -724,7 +747,8 @@ InMemoryFileSystem::openFileForRead(cons
// When we have a file provide a heap-allocated wrapper for the memory buffer
// to match t
