From: Richard Smith Date: Thu, 11 Dec 2014 20:50:24 +0000 (+0000) Subject: [modules] When constructing paths relative to a module, strip out /./ directory X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=0748a208292bf2db03308ae05402c9b0166282ef;p=clang [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. git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@224055 91177308-0d34-0410-b5e6-96231b3b80d8 --- diff --git a/include/clang/Basic/FileManager.h b/include/clang/Basic/FileManager.h index 132bf37019..bd3f27ab9e 100644 --- a/include/clang/Basic/FileManager.h +++ b/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 &Path); + /// \brief Retrieve the canonical name for a given directory. /// /// This is a very expensive operation, despite its results being cached, diff --git a/lib/Basic/FileManager.cpp b/lib/Basic/FileManager.cpp index af6022fdc9..214e0f35a5 100644 --- a/lib/Basic/FileManager.cpp +++ b/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 &Path) { + using namespace llvm::sys; + + SmallVector 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::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 { diff --git a/lib/Frontend/ModuleDependencyCollector.cpp b/lib/Frontend/ModuleDependencyCollector.cpp index 882bf8ee24..1ac7e36eff 100644 --- a/lib/Frontend/ModuleDependencyCollector.cpp +++ b/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 &Path) { - using namespace llvm::sys; - SmallVector 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(); diff --git a/lib/Lex/HeaderSearch.cpp b/lib/Lex/HeaderSearch.cpp index 02fd87c822..d6b255fb01 100644 --- a/lib/Lex/HeaderSearch.cpp +++ b/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 -.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(); diff --git a/lib/Serialization/ASTWriter.cpp b/lib/Serialization/ASTWriter.cpp index adecf9dab7..02663ad43d 100644 --- a/lib/Serialization/ASTWriter.cpp +++ b/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 &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 &Path) { - bool Changed = false; - - 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); + assert(Context && "should have context when outputting 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(); diff --git a/test/Modules/Inputs/dependency-gen-base.modulemap b/test/Modules/Inputs/dependency-gen-base.modulemap new file mode 100644 index 0000000000..8b30ffa674 --- /dev/null +++ b/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" diff --git a/test/Modules/Inputs/dependency-gen-base2.modulemap b/test/Modules/Inputs/dependency-gen-base2.modulemap new file mode 100644 index 0000000000..7808c8041a --- /dev/null +++ b/test/Modules/Inputs/dependency-gen-base2.modulemap @@ -0,0 +1,4 @@ +module "test-base2" { + export * + textual header "Inputs/dependency-gen-included2.h" +} diff --git a/test/Modules/Inputs/dependency-gen-included.h b/test/Modules/Inputs/dependency-gen-included.h new file mode 100644 index 0000000000..0e1cdfcd1e --- /dev/null +++ b/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 diff --git a/test/Modules/Inputs/dependency-gen-included2.h b/test/Modules/Inputs/dependency-gen-included2.h new file mode 100644 index 0000000000..fcd8f12ba5 --- /dev/null +++ b/test/Modules/Inputs/dependency-gen-included2.h @@ -0,0 +1,7 @@ +#ifndef DEPENDENCY_GEN_INCLUDED2_H +#define DEPENDENCY_GEN_INCLUDED2_H + +void h() { +} + +#endif diff --git a/test/Modules/Inputs/dependency-gen.h b/test/Modules/Inputs/dependency-gen.h new file mode 100644 index 0000000000..2671e262c6 --- /dev/null +++ b/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 diff --git a/test/Modules/dependency-gen.modulemap.cpp b/test/Modules/dependency-gen.modulemap.cpp new file mode 100644 index 0000000000..c49714c14e --- /dev/null +++ b/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 diff --git a/test/Modules/modular_maps.cpp b/test/Modules/modular_maps.cpp index bdb2116340..606c979c4c 100644 --- a/test/Modules/modular_maps.cpp +++ b/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