<div dir="ltr">I believe we need a more principled approach here :)<div><br></div><div>1. The first thing we need is to actually produce nice errors when we hit an addFile() with the same path (normalized or non-normalized)</div><div>I'd propose the following approach: we compare the buffers; if the buffer contents are equal, we declare success; otherwise we return a nice descriptive error ("trying to add the same file with different content").</div><div><br></div><div>2. I think we'll want 2 different modes in InMemoryFileSystem: Normalize should be either on or off; if on, we should normalize everything, and return an error if we hop up over /. if off, we should not normalize anything.</div></div><br><div class="gmail_quote"><div dir="ltr">On Mon, Oct 12, 2015 at 3:32 PM Benjamin Kramer via cfe-commits <<a href="mailto:cfe-commits@lists.llvm.org">cfe-commits@lists.llvm.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Author: d0k<br>
Date: Mon Oct 12 08:30:38 2015<br>
New Revision: 250036<br>
<br>
URL: <a href="http://llvm.org/viewvc/llvm-project?rev=250036&view=rev" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project?rev=250036&view=rev</a><br>
Log:<br>
[VFS] Don't try to be heroic with '.' in paths.<br>
<br>
Actually the only special path we have to handle is ./foo, the rest is<br>
tricky to get right so do the same thing as the existing YAML vfs here.<br>
<br>
Modified:<br>
cfe/trunk/lib/Basic/VirtualFileSystem.cpp<br>
cfe/trunk/unittests/Basic/VirtualFileSystemTest.cpp<br>
<br>
Modified: cfe/trunk/lib/Basic/VirtualFileSystem.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Basic/VirtualFileSystem.cpp?rev=250036&r1=250035&r2=250036&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Basic/VirtualFileSystem.cpp?rev=250036&r1=250035&r2=250036&view=diff</a><br>
==============================================================================<br>
--- cfe/trunk/lib/Basic/VirtualFileSystem.cpp (original)<br>
+++ cfe/trunk/lib/Basic/VirtualFileSystem.cpp Mon Oct 12 08:30:38 2015<br>
@@ -10,7 +10,6 @@<br>
//===----------------------------------------------------------------------===//<br>
<br>
#include "clang/Basic/VirtualFileSystem.h"<br>
-#include "clang/Basic/FileManager.h"<br>
#include "llvm/ADT/DenseMap.h"<br>
#include "llvm/ADT/STLExtras.h"<br>
#include "llvm/ADT/StringExtras.h"<br>
@@ -497,12 +496,14 @@ void InMemoryFileSystem::addFile(const T<br>
assert(!EC);<br>
(void)EC;<br>
<br>
- FileManager::removeDotPaths(Path, /*RemoveDotDot=*/false);<br>
- if (Path.empty())<br>
- return;<br>
-<br>
detail::InMemoryDirectory *Dir = Root.get();<br>
auto I = llvm::sys::path::begin(Path), E = llvm::sys::path::end(Path);<br>
+ if (*I == ".")<br>
+ ++I;<br>
+<br>
+ if (I == E)<br>
+ return;<br>
+<br>
while (true) {<br>
StringRef Name = *I;<br>
detail::InMemoryNode *Node = Dir->getChild(Name);<br>
@@ -556,11 +557,13 @@ lookupInMemoryNode(const InMemoryFileSys<br>
assert(!EC);<br>
(void)EC;<br>
<br>
- FileManager::removeDotPaths(Path, /*RemoveDotDot=*/false);<br>
- if (Path.empty())<br>
+ auto I = llvm::sys::path::begin(Path), E = llvm::sys::path::end(Path);<br>
+ if (*I == ".")<br>
+ ++I;<br>
+<br>
+ if (I == E)<br>
return Dir;<br>
<br>
- auto I = llvm::sys::path::begin(Path), E = llvm::sys::path::end(Path);<br>
while (true) {<br>
detail::InMemoryNode *Node = Dir->getChild(*I);<br>
++I;<br>
<br>
Modified: cfe/trunk/unittests/Basic/VirtualFileSystemTest.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Basic/VirtualFileSystemTest.cpp?rev=250036&r1=250035&r2=250036&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Basic/VirtualFileSystemTest.cpp?rev=250036&r1=250035&r2=250036&view=diff</a><br>
==============================================================================<br>
--- cfe/trunk/unittests/Basic/VirtualFileSystemTest.cpp (original)<br>
+++ cfe/trunk/unittests/Basic/VirtualFileSystemTest.cpp Mon Oct 12 08:30:38 2015<br>
@@ -551,8 +551,6 @@ TEST_F(InMemoryFileSystemTest, OverlayFi<br>
FS.addFile("/a", 0, MemoryBuffer::getMemBuffer("a"));<br>
auto Stat = FS.status("/");<br>
ASSERT_FALSE(Stat.getError()) << Stat.getError() << FS.toString();<br>
- Stat = FS.status("/.");<br>
- ASSERT_FALSE(Stat.getError()) << Stat.getError() << FS.toString();<br>
Stat = FS.status("/a");<br>
ASSERT_FALSE(Stat.getError()) << Stat.getError() << "\n" << FS.toString();<br>
ASSERT_EQ("/a", Stat->getName());<br>
@@ -568,18 +566,18 @@ TEST_F(InMemoryFileSystemTest, OverlayFi<br>
<br>
TEST_F(InMemoryFileSystemTest, OpenFileForRead) {<br>
FS.addFile("/a", 0, MemoryBuffer::getMemBuffer("a"));<br>
- FS.addFile("././c", 0, MemoryBuffer::getMemBuffer("c"));<br>
+ FS.addFile("./c", 0, MemoryBuffer::getMemBuffer("c"));<br>
auto File = FS.openFileForRead("/a");<br>
ASSERT_EQ("a", (*(*File)->getBuffer("ignored"))->getBuffer());<br>
File = FS.openFileForRead("/a"); // Open again.<br>
ASSERT_EQ("a", (*(*File)->getBuffer("ignored"))->getBuffer());<br>
- File = FS.openFileForRead("/././a"); // Open again.<br>
+ File = FS.openFileForRead("./a"); // Open again.<br>
ASSERT_EQ("a", (*(*File)->getBuffer("ignored"))->getBuffer());<br>
File = FS.openFileForRead("/");<br>
ASSERT_EQ(File.getError(), errc::invalid_argument) << FS.toString();<br>
File = FS.openFileForRead("/b");<br>
ASSERT_EQ(File.getError(), errc::no_such_file_or_directory) << FS.toString();<br>
- File = FS.openFileForRead("./c");<br>
+ File = FS.openFileForRead("c");<br>
ASSERT_EQ("c", (*(*File)->getBuffer("ignored"))->getBuffer());<br>
}<br>
<br>
<br>
<br>
_______________________________________________<br>
cfe-commits mailing list<br>
<a href="mailto:cfe-commits@lists.llvm.org" target="_blank">cfe-commits@lists.llvm.org</a><br>
<a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits" rel="noreferrer" target="_blank">http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits</a><br>
</blockquote></div>