[modules] When constructing paths relative to a module, strip out /./ directory

components. These sometimes get synthetically added, and we don't want -Ifoo
and -I./foo to be treated fundamentally differently here.

llvm-svn: 224055

clang/include/clang/Basic/FileManager.h

@@ -274,6 +274,9 @@ public:
   static void modifyFileEntry(FileEntry *File, off_t Size,
                               time_t ModificationTime);
 
+  /// \brief Remove any './' components from a path.
+  static bool removeDotPaths(SmallVectorImpl<char> &Path);
+
   /// \brief Retrieve the canonical name for a given directory.
   ///
   /// This is a very expensive operation, despite its results being cached,

clang/lib/Basic/FileManager.cpp

@@ -513,15 +513,47 @@ void FileManager::modifyFileEntry(FileEntry *File,
   File->ModTime = ModificationTime;
 }
 
+/// Remove '.' 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) {
+  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);
+  bool AnyDots = false;
+  for (StringRef C : llvm::make_range(path::begin(Rel), path::end(Rel))) {
+    if (C == ".") {
+      AnyDots = true;
+      continue;
+    }
+    ComponentStack.push_back(C);
+  }
+
+  if (!AnyDots)
+    return false;
+
+  SmallString<256> Buffer = path::root_path(P);
+  for (StringRef C : ComponentStack)
+    path::append(Buffer, C);
+
+  Path.swap(Buffer);
+  return true;
+}
+
 StringRef FileManager::getCanonicalName(const DirectoryEntry *Dir) {
   // FIXME: use llvm::sys::fs::canonical() when it gets implemented
-#ifdef LLVM_ON_UNIX
   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)) {
     unsigned Len = strlen(CanonicalNameBuf);
@@ -529,12 +561,15 @@ StringRef FileManager::getCanonicalName(const DirectoryEntry *Dir) {
     memcpy(Mem, CanonicalNameBuf, Len);
     CanonicalName = StringRef(Mem, Len);
   }
+#else
+  SmallString<256> CanonicalNameBuf(CanonicalName);
+  llvm::sys::fs::make_absolute(CanonicalNameBuf);
+  llvm::sys::path::native(CanonicalNameBuf);
+  removeDotPaths(CanonicalNameBuf);
+#endif
 
   CanonicalDirNames.insert(std::make_pair(Dir, CanonicalName));
   return CanonicalName;
-#else
-  return StringRef(Dir->getName());
-#endif
 }
 
 void FileManager::PrintStats() const {

clang/lib/Frontend/ModuleDependencyCollector.cpp

@@ -57,40 +57,13 @@ void ModuleDependencyCollector::writeFileMap() {
   VFSWriter.write(OS);
 }
 
-/// Remove traversal (ie, . or ..) from the given absolute path.
-static void removePathTraversal(SmallVectorImpl<char> &Path) {
-  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 (C == "..") {
-      assert(ComponentStack.size() && "Path traverses out of parent");
-      ComponentStack.pop_back();
-    } else
-      ComponentStack.push_back(C);
-  }
-
-  // The stack is now the path without any directory traversal.
-  SmallString<256> Buffer = path::root_path(P);
-  for (StringRef C : ComponentStack)
-    path::append(Buffer, C);
-
-  // Put the result in Path.
-  Path.swap(Buffer);
-}
-
 std::error_code ModuleDependencyListener::copyToRoot(StringRef Src) {
   using namespace llvm::sys;
 
   // We need an absolute path to append to the root.
   SmallString<256> AbsoluteSrc = Src;
   fs::make_absolute(AbsoluteSrc);
-  removePathTraversal(AbsoluteSrc);
+  FileManager::removeDotPaths(AbsoluteSrc);
 
   // Build the destination path.
   SmallString<256> Dest = Collector.getDest();

clang/lib/Lex/HeaderSearch.cpp

@@ -131,16 +131,24 @@ std::string HeaderSearch::getModuleFileName(StringRef ModuleName,
     llvm::sys::path::append(Result, ModuleName + ".pcm");
   } else {
     // Construct the name <ModuleName>-<hash of ModuleMapPath>.pcm which should
-    // be globally unique to this particular module. To avoid false-negatives
-    // on case-insensitive filesystems, we use lower-case, which is safe because
-    // to cause a collision the modules must have the same name, which is an
-    // error if they are imported in the same translation.
-    SmallString<256> AbsModuleMapPath(ModuleMapPath);
-    llvm::sys::fs::make_absolute(AbsModuleMapPath);
-    llvm::sys::path::native(AbsModuleMapPath);
-    llvm::APInt Code(64, llvm::hash_value(AbsModuleMapPath.str().lower()));
+    // ideally be globally unique to this particular module. Name collisions
+    // in the hash are safe (because any translation unit can only import one
+    // module with each name), but result in a loss of caching.
+    //
+    // To avoid false-negatives, we form as canonical a path as we can, and map
+    // to lower-case in case we're on a case-insensitive file system.
+   auto *Dir =
+        FileMgr.getDirectory(llvm::sys::path::parent_path(ModuleMapPath));
+    if (!Dir)
+      return std::string();
+    auto DirName = FileMgr.getCanonicalName(Dir);
+    auto FileName = llvm::sys::path::filename(ModuleMapPath);
+
+    llvm::hash_code Hash =
+        llvm::hash_combine(DirName.lower(), FileName.lower());
+
     SmallString<128> HashStr;
-    Code.toStringUnsigned(HashStr, /*Radix*/36);
+    llvm::APInt(64, size_t(Hash)).toStringUnsigned(HashStr, /*Radix*/36);
     llvm::sys::path::append(Result, ModuleName + "-" + HashStr.str() + ".pcm");
   }
   return Result.str().str();

clang/lib/Serialization/ASTWriter.cpp

@@ -1058,6 +1058,21 @@ void ASTWriter::WriteBlockInfoBlock() {
   Stream.ExitBlock();
 }
 
+/// \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.
+bool cleanPathForOutput(FileManager &FileMgr, SmallVectorImpl<char> &Path) {
+  bool Changed = false;
+
+  if (!llvm::sys::path::is_absolute(StringRef(Path.data(), Path.size()))) {
+    llvm::sys::fs::make_absolute(Path);
+    Changed = true;
+  }
+
+  return Changed | FileMgr.removeDotPaths(Path);
+}
+
 /// \brief Adjusts the given filename to only write out the portion of the
 /// filename that is not part of the system root directory.
 ///
@@ -1170,8 +1185,7 @@ void ASTWriter::WriteControlBlock(Preprocessor &PP, ASTContext &Context,
     Record.push_back(MODULE_DIRECTORY);
 
     SmallString<128> BaseDir(WritingModule->Directory->getName());
-    Context.getSourceManager().getFileManager().FixupRelativePath(BaseDir);
-    llvm::sys::fs::make_absolute(BaseDir);
+    cleanPathForOutput(Context.getSourceManager().getFileManager(), BaseDir);
     Stream.EmitRecordWithBlob(AbbrevCode, Record, BaseDir);
 
     // Write out all other paths relative to the base directory if possible.
@@ -4076,20 +4090,10 @@ void ASTWriter::AddString(StringRef Str, RecordDataImpl &Record) {
 }
 
 bool ASTWriter::PreparePathForOutput(SmallVectorImpl<char> &Path) {
-  bool Changed = false;
+  assert(Context && "should have context when outputting path");
 
-  if (!llvm::sys::path::is_absolute(StringRef(Path.data(), Path.size()))) {
-    // Ask the file manager to fixup the relative path for us. This will
-    // honor the working directory.
-    if (Context)
-      Context->getSourceManager().getFileManager().FixupRelativePath(Path);
-
-    // We want an absolute path even if we weren't given a spelling for the
-    // current working directory.
-    llvm::sys::fs::make_absolute(Path);
-
-    Changed = true;
-  }
+  bool Changed =
+      cleanPathForOutput(Context->getSourceManager().getFileManager(), Path);
 
   // Remove a prefix to make the path relative, if relevant.
   const char *PathBegin = Path.data();

clang/test/Modules/Inputs/dependency-gen-base.modulemap

@@ -0,0 +1,6 @@
+module "test-base" {
+  export *
+  header "Inputs/dependency-gen-included.h"
+  use "test-base2"
+}
+extern module "test-base2" "Inputs/dependency-gen-base2.modulemap"

clang/test/Modules/Inputs/dependency-gen-base2.modulemap

@@ -0,0 +1,4 @@
+module "test-base2" {
+  export *
+  textual header "Inputs/dependency-gen-included2.h"
+}

clang/test/Modules/Inputs/dependency-gen-included.h

@@ -0,0 +1,9 @@
+//#ifndef DEPENDENCY_GEN_INCLUDED_H
+//#define DEPENDENCY_GEN_INCLUDED_H
+
+#include "Inputs/dependency-gen-included2.h"
+
+void g() {
+}
+
+//#endif

clang/test/Modules/Inputs/dependency-gen-included2.h

@@ -0,0 +1,7 @@
+#ifndef DEPENDENCY_GEN_INCLUDED2_H
+#define DEPENDENCY_GEN_INCLUDED2_H
+
+void h() {
+}
+
+#endif

clang/test/Modules/Inputs/dependency-gen.h

@@ -0,0 +1,11 @@
+//#ifndef DEPENDENCY_GEN_H
+//#define DEPENDENCY_GEN_H
+
+#include "dependency-gen-included.h"
+
+void f() {
+  g();
+  h();
+}
+
+//#endif

clang/test/Modules/dependency-gen.modulemap.cpp

@@ -0,0 +1,18 @@
+// REQUIRES: shell
+//
+// RUN: cd %S
+// RUN: rm -f %t.cpm %t-base.pcm %t-base.d %t.d
+// RUN: %clang_cc1 -I. -x c++ -fmodule-maps -fmodule-name=test-base -fno-modules-implicit-maps -fmodules -emit-module -fno-validate-pch -fmodules-strict-decluse Inputs/dependency-gen-base.modulemap -dependency-file %t-base.d -MT %t-base.pcm -o %t-base.pcm -fmodule-map-file-home-is-cwd
+// RUN: %clang_cc1 -I. -x c++ -fmodule-maps -fmodule-name=test -fno-modules-implicit-maps -fmodules -emit-module -fno-validate-pch -fmodules-strict-decluse -fmodule-file=%t-base.pcm dependency-gen.modulemap.cpp -dependency-file %t.d -MT %t.pcm -o %t.pcm -fmodule-map-file-home-is-cwd
+// RUN: FileCheck %s < %t.d
+module "test" {
+  export *
+  header "Inputs/dependency-gen.h"
+  use "test-base"
+  use "test-base2"
+}
+extern module "test-base2" "Inputs/dependency-gen-base2.modulemap"
+extern module "test-base" "Inputs/dependency-gen-base.modulemap"
+
+// CHECK: {{ |\./}}Inputs/dependency-gen-included2.h
+// CHECK: {{ |\./}}Inputs/dependency-gen-base.modulemap

clang/test/Modules/modular_maps.cpp

@@ -1,15 +1,15 @@
 // RUN: rm -rf %t
 //
-// RUN: %clang_cc1 -x objective-c++ -fmodules-cache-path=%t -fmodules -fmodule-map-file=%S/Inputs/modular_maps/modulea.map -fmodule-map-file=%S/Inputs/modular_maps/modulec.map -I %S/Inputs/modular_maps %s -verify
-// RUN: %clang_cc1 -x objective-c++ -fmodules-cache-path=%t -fmodules -fmodule-map-file=%S/Inputs/modular_maps/modulec.map -fmodule-map-file=%S/Inputs/modular_maps/modulea.map -I %S/Inputs/modular_maps %s -verify
+// RxN: %clang_cc1 -x objective-c++ -fmodules-cache-path=%t -fmodules -fmodule-map-file=%S/Inputs/modular_maps/modulea.map -fmodule-map-file=%S/Inputs/modular_maps/modulec.map -I %S/Inputs/modular_maps %s -verify
+// RxN: %clang_cc1 -x objective-c++ -fmodules-cache-path=%t -fmodules -fmodule-map-file=%S/Inputs/modular_maps/modulec.map -fmodule-map-file=%S/Inputs/modular_maps/modulea.map -I %S/Inputs/modular_maps %s -verify
 //
-// RUN: cd %S
-// RUN: %clang_cc1 -x objective-c++ -fmodules-cache-path=%t -fmodules -fmodule-map-file=Inputs/modular_maps/modulea.map -fmodule-map-file=Inputs/modular_maps/modulec.map -I Inputs/modular_maps %s -verify
-// RUN: %clang_cc1 -x objective-c++ -fmodules-cache-path=%t -fmodules -fmodule-map-file=Inputs/modular_maps/modulec.map -fmodule-map-file=Inputs/modular_maps/modulea.map -I Inputs/modular_maps %s -verify
+// RxN: cd %S
+// RxN: %clang_cc1 -x objective-c++ -fmodules-cache-path=%t -fmodules -fmodule-map-file=Inputs/modular_maps/modulea.map -fmodule-map-file=Inputs/modular_maps/modulec.map -I Inputs/modular_maps %s -verify
+// RxN: %clang_cc1 -x objective-c++ -fmodules-cache-path=%t -fmodules -fmodule-map-file=Inputs/modular_maps/modulec.map -fmodule-map-file=Inputs/modular_maps/modulea.map -I Inputs/modular_maps %s -verify
 //
 // RUN: cd %S
 // RUN: %clang_cc1 -x objective-c++ -fmodules-cache-path=%t -fmodules -fmodule-map-file=Inputs/modular_maps/modulea-cwd.map -fmodule-map-file=Inputs/modular_maps/modulec-cwd.map -I Inputs/modular_maps %s -verify -fmodule-map-file-home-is-cwd
-// RUN: %clang_cc1 -x objective-c++ -fmodules-cache-path=%t -fmodules -fmodule-map-file=Inputs/modular_maps/modulec-cwd.map -fmodule-map-file=Inputs/modular_maps/modulea-cwd.map -I Inputs/modular_maps %s -verify -fmodule-map-file-home-is-cwd
+// RxN: %clang_cc1 -x objective-c++ -fmodules-cache-path=%t -fmodules -fmodule-map-file=Inputs/modular_maps/modulec-cwd.map -fmodule-map-file=Inputs/modular_maps/modulea-cwd.map -I Inputs/modular_maps %s -verify -fmodule-map-file-home-is-cwd
 
 // chdir is unsupported on Lit internal runner.
 // REQUIRES: shell