Moving FileManager::removeDotPaths to llvm::sys::path::remove_dots

[aizatsky]

Summary

Corresponding LLVM change: http://reviews.llvm.org/D14393

[aizatsky]

• reformat
• fixes

[aizatsky]

• test fix

[kcc]

I would probably leave remove_leading_dotslash intact, ant that should eliminate the need to change the test.

test/Frontend/dependency-gen.c

19 ↗

(On Diff #39432)

This is an intended change, right? Why?

[aizatsky]

I would probably leave remove_leading_dotslash intact, ant that should eliminate the need to change the test.

I disagree. The old function is much more limited - it stripped "./" only at the beginning. Plus it would result in two very similar functions. I think having only one is better.

test/Frontend/dependency-gen.c

19 ↗

(On Diff #39432)

Because new function also removes /./ in the middle. It used to be a/b/./x.h and it is now a/b/x.h

[silvas]

test/Frontend/dependency-gen.c

19 ↗

(On Diff #39432)

To keep this change mechanical, let's just not touch remove_leading_dotslash in this patch.

[aizatsky]

review

[aizatsky]

Please take another look.

[kcc]

LGTM

[silvas]

LGTM

cfe/trunk/include/clang/Basic/FileManager.h

public:
   void GetUniqueIDMapping(
                     SmallVectorImpl<const FileEntry *> &UIDToFiles) const;
 
   /// \brief Modifies the size and modification time of a previously created
   /// FileEntry. Use with caution.
   static void modifyFileEntry(FileEntry *File, off_t Size,
                               time_t ModificationTime);
 
-  /// \brief Remove any './' components from a path.
-  static bool removeDotPaths(SmallVectorImpl<char> &Path, bool RemoveDotDot = false);
-
   /// \brief Retrieve the canonical name for a given directory.
   ///
   /// This is a very expensive operation, despite its results being cached,
   /// and should only be used when the physical layout of the file system is
   /// required, which is (almost) never.
   StringRef getCanonicalName(const DirectoryEntry *Dir);
 
   void PrintStats() const;
 };
 
 }  // end namespace clang
 
 #endif

cfe/trunk/lib/Basic/FileManager.cpp

 }
 
 void FileManager::modifyFileEntry(FileEntry *File,
                                   off_t Size, time_t ModificationTime) {
   File->Size = Size;
   File->ModTime = ModificationTime;
 }
 
-/// Remove '.' and '..' path components from the given absolute path.
-/// \return \c true if any changes were made.
-// FIXME: Move this to llvm::sys::path.
-bool FileManager::removeDotPaths(SmallVectorImpl<char> &Path, bool RemoveDotDot) {
-  using namespace llvm::sys;
-
-  SmallVector<StringRef, 16> ComponentStack;
-  StringRef P(Path.data(), Path.size());
-
-  // Skip the root path, then look for traversal in the components.
-  StringRef Rel = path::relative_path(P);
-  for (StringRef C : llvm::make_range(path::begin(Rel), path::end(Rel))) {
-    if (C == ".")
-      continue;
-    if (RemoveDotDot) {
-      if (C == "..") {
-        if (!ComponentStack.empty())
-          ComponentStack.pop_back();
-        continue;
-      }
-    }
-    ComponentStack.push_back(C);
-  }
-
-  SmallString<256> Buffer = path::root_path(P);
-  for (StringRef C : ComponentStack)
-    path::append(Buffer, C);
-
-  bool Changed = (Path != Buffer);
-  Path.swap(Buffer);
-  return Changed;
-}
-
 StringRef FileManager::getCanonicalName(const DirectoryEntry *Dir) {
   // FIXME: use llvm::sys::fs::canonical() when it gets implemented
   llvm::DenseMap<const DirectoryEntry *, llvm::StringRef>::iterator Known
     = CanonicalDirNames.find(Dir);
   if (Known != CanonicalDirNames.end())
     return Known->second;
 
   StringRef CanonicalName(Dir->getName());
 
 #ifdef LLVM_ON_UNIX
   char CanonicalNameBuf[PATH_MAX];
   if (realpath(Dir->getName(), CanonicalNameBuf))
     CanonicalName = StringRef(CanonicalNameBuf).copy(CanonicalNameStorage);
 #else
   SmallString<256> CanonicalNameBuf(CanonicalName);
   llvm::sys::fs::make_absolute(CanonicalNameBuf);
   llvm::sys::path::native(CanonicalNameBuf);
   // We've run into needing to remove '..' here in the wild though, so
   // remove it.
   // On Windows, symlinks are significantly less prevalent, so removing
   // '..' is pretty safe.
   // Ideally we'd have an equivalent of `realpath` and could implement
   // sys::fs::canonical across all the platforms.
-  removeDotPaths(CanonicalNameBuf, /*RemoveDotDot*/true);
+  llvm::sys::path::remove_dots(CanonicalNameBuf, /* remove_dot_dot */ true);
   CanonicalName = StringRef(CanonicalNameBuf).copy(CanonicalNameStorage);
 #endif
 
   CanonicalDirNames.insert(std::make_pair(Dir, CanonicalName));
   return CanonicalName;
 }
 
 void FileManager::PrintStats() const {

cfe/trunk/lib/Basic/VirtualFileSystem.cpp

bool InMemoryFileSystem::addFile(const Twine &P, time_t ModificationTime,
   P.toVector(Path);
 
   // Fix up relative paths. This just prepends the current working directory.
   std::error_code EC = makeAbsolute(Path);
   assert(!EC);
   (void)EC;
 
   if (useNormalizedPaths())
-    FileManager::removeDotPaths(Path, /*RemoveDotDot=*/true);
+    llvm::sys::path::remove_dots(Path, /*remove_dot_dot=*/true);
 
   if (Path.empty())
     return false;
 
   detail::InMemoryDirectory *Dir = Root.get();
   auto I = llvm::sys::path::begin(Path), E = llvm::sys::path::end(Path);
   while (true) {
     StringRef Name = *I;
lookupInMemoryNode(const InMemoryFileSystem &FS, detail::InMemoryDirectory *Dir,
   P.toVector(Path);
 
   // Fix up relative paths. This just prepends the current working directory.
   std::error_code EC = FS.makeAbsolute(Path);
   assert(!EC);
   (void)EC;
 
   if (FS.useNormalizedPaths())
-    FileManager::removeDotPaths(Path, /*RemoveDotDot=*/true);
+    llvm::sys::path::remove_dots(Path, /*remove_dot_dot=*/true);
 
   if (Path.empty())
     return Dir;
 
   auto I = llvm::sys::path::begin(Path), E = llvm::sys::path::end(Path);
   while (true) {
     detail::InMemoryNode *Node = Dir->getChild(*I);
     ++I;

cfe/trunk/lib/Frontend/ModuleDependencyCollector.cpp

std::error_code ModuleDependencyListener::copyToRoot(StringRef Src) {
 
   // We need an absolute path to append to the root.
   SmallString<256> AbsoluteSrc = Src;
   fs::make_absolute(AbsoluteSrc);
   // Canonicalize to a native path to avoid mixed separator styles.
   path::native(AbsoluteSrc);
   // TODO: We probably need to handle .. as well as . in order to have valid
   // input to the YAMLVFSWriter.
-  FileManager::removeDotPaths(AbsoluteSrc);
+  path::remove_dots(AbsoluteSrc);
 
   // Build the destination path.
   SmallString<256> Dest = Collector.getDest();
   path::append(Dest, path::relative_path(AbsoluteSrc));
 
   // Copy the file into place.
   if (std::error_code EC = fs::create_directories(path::parent_path(Dest),
                                                    /*IgnoreExisting=*/true))

cfe/trunk/lib/Serialization/ASTWriter.cpp

 
 /// \brief Prepares a path for being written to an AST file by converting it
 /// to an absolute path and removing nested './'s.
 ///
 /// \return \c true if the path was changed.
 static bool cleanPathForOutput(FileManager &FileMgr,
                                SmallVectorImpl<char> &Path) {
   bool Changed = FileMgr.makeAbsolutePath(Path);
-  return Changed | FileMgr.removeDotPaths(Path);
+  return Changed | llvm::sys::path::remove_dots(Path);
 }
 
 /// \brief Adjusts the given filename to only write out the portion of the
 /// filename that is not part of the system root directory.
 ///
 /// \param Filename the file name to adjust.
 ///
 /// \param BaseDir When non-NULL, the PCH file is a relocatable AST file and