r224055 - [modules] When constructing paths relative to a module, strip out /./ directory
Aaron Ballman
aaron at aaronballman.com
Fri Dec 12 05:59:45 PST 2014
This appears to have broken some bots:
http://bb.pgr.jp/builders/ninja-clang-i686-msc17-R/builds/12027
~Aaron
On Thu, Dec 11, 2014 at 3:50 PM, Richard Smith
<richard-llvm at metafoo.co.uk> wrote:
> Author: rsmith
> Date: Thu Dec 11 14:50:24 2014
> New Revision: 224055
>
> URL: http://llvm.org/viewvc/llvm-project?rev=224055&view=rev
> Log:
> [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.
>
> Added:
> cfe/trunk/test/Modules/Inputs/dependency-gen-base.modulemap
> cfe/trunk/test/Modules/Inputs/dependency-gen-base2.modulemap
> cfe/trunk/test/Modules/Inputs/dependency-gen-included.h
> cfe/trunk/test/Modules/Inputs/dependency-gen-included2.h
> cfe/trunk/test/Modules/Inputs/dependency-gen.h
> cfe/trunk/test/Modules/dependency-gen.modulemap.cpp
> Modified:
> cfe/trunk/include/clang/Basic/FileManager.h
> cfe/trunk/lib/Basic/FileManager.cpp
> cfe/trunk/lib/Frontend/ModuleDependencyCollector.cpp
> cfe/trunk/lib/Lex/HeaderSearch.cpp
> cfe/trunk/lib/Serialization/ASTWriter.cpp
> cfe/trunk/test/Modules/modular_maps.cpp
>
> Modified: cfe/trunk/include/clang/Basic/FileManager.h
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/FileManager.h?rev=224055&r1=224054&r2=224055&view=diff
> ==============================================================================
> --- cfe/trunk/include/clang/Basic/FileManager.h (original)
> +++ cfe/trunk/include/clang/Basic/FileManager.h Thu Dec 11 14:50:24 2014
> @@ -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,
>
> Modified: cfe/trunk/lib/Basic/FileManager.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Basic/FileManager.cpp?rev=224055&r1=224054&r2=224055&view=diff
> ==============================================================================
> --- cfe/trunk/lib/Basic/FileManager.cpp (original)
> +++ cfe/trunk/lib/Basic/FileManager.cpp Thu Dec 11 14:50:24 2014
> @@ -513,15 +513,47 @@ void FileManager::modifyFileEntry(FileEn
> 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(
> 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 {
>
> Modified: cfe/trunk/lib/Frontend/ModuleDependencyCollector.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Frontend/ModuleDependencyCollector.cpp?rev=224055&r1=224054&r2=224055&view=diff
> ==============================================================================
> --- cfe/trunk/lib/Frontend/ModuleDependencyCollector.cpp (original)
> +++ cfe/trunk/lib/Frontend/ModuleDependencyCollector.cpp Thu Dec 11 14:50:24 2014
> @@ -57,40 +57,13 @@ void ModuleDependencyCollector::writeFil
> 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();
>
> Modified: cfe/trunk/lib/Lex/HeaderSearch.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Lex/HeaderSearch.cpp?rev=224055&r1=224054&r2=224055&view=diff
> ==============================================================================
> --- cfe/trunk/lib/Lex/HeaderSearch.cpp (original)
> +++ cfe/trunk/lib/Lex/HeaderSearch.cpp Thu Dec 11 14:50:24 2014
> @@ -131,16 +131,24 @@ std::string HeaderSearch::getModuleFileN
> 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();
>
> Modified: cfe/trunk/lib/Serialization/ASTWriter.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Serialization/ASTWriter.cpp?rev=224055&r1=224054&r2=224055&view=diff
> ==============================================================================
> --- cfe/trunk/lib/Serialization/ASTWriter.cpp (original)
> +++ cfe/trunk/lib/Serialization/ASTWriter.cpp Thu Dec 11 14:50:24 2014
> @@ -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(Prepro
> 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,
> }
>
> bool ASTWriter::PreparePathForOutput(SmallVectorImpl<char> &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();
>
> Added: cfe/trunk/test/Modules/Inputs/dependency-gen-base.modulemap
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/dependency-gen-base.modulemap?rev=224055&view=auto
> ==============================================================================
> --- cfe/trunk/test/Modules/Inputs/dependency-gen-base.modulemap (added)
> +++ cfe/trunk/test/Modules/Inputs/dependency-gen-base.modulemap Thu Dec 11 14:50:24 2014
> @@ -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"
>
> Added: cfe/trunk/test/Modules/Inputs/dependency-gen-base2.modulemap
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/dependency-gen-base2.modulemap?rev=224055&view=auto
> ==============================================================================
> --- cfe/trunk/test/Modules/Inputs/dependency-gen-base2.modulemap (added)
> +++ cfe/trunk/test/Modules/Inputs/dependency-gen-base2.modulemap Thu Dec 11 14:50:24 2014
> @@ -0,0 +1,4 @@
> +module "test-base2" {
> + export *
> + textual header "Inputs/dependency-gen-included2.h"
> +}
>
> Added: cfe/trunk/test/Modules/Inputs/dependency-gen-included.h
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/dependency-gen-included.h?rev=224055&view=auto
> ==============================================================================
> --- cfe/trunk/test/Modules/Inputs/dependency-gen-included.h (added)
> +++ cfe/trunk/test/Modules/Inputs/dependency-gen-included.h Thu Dec 11 14:50:24 2014
> @@ -0,0 +1,9 @@
> +//#ifndef DEPENDENCY_GEN_INCLUDED_H
> +//#define DEPENDENCY_GEN_INCLUDED_H
> +
> +#include "Inputs/dependency-gen-included2.h"
> +
> +void g() {
> +}
> +
> +//#endif
>
> Added: cfe/trunk/test/Modules/Inputs/dependency-gen-included2.h
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/dependency-gen-included2.h?rev=224055&view=auto
> ==============================================================================
> --- cfe/trunk/test/Modules/Inputs/dependency-gen-included2.h (added)
> +++ cfe/trunk/test/Modules/Inputs/dependency-gen-included2.h Thu Dec 11 14:50:24 2014
> @@ -0,0 +1,7 @@
> +#ifndef DEPENDENCY_GEN_INCLUDED2_H
> +#define DEPENDENCY_GEN_INCLUDED2_H
> +
> +void h() {
> +}
> +
> +#endif
>
> Added: cfe/trunk/test/Modules/Inputs/dependency-gen.h
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/dependency-gen.h?rev=224055&view=auto
> ==============================================================================
> --- cfe/trunk/test/Modules/Inputs/dependency-gen.h (added)
> +++ cfe/trunk/test/Modules/Inputs/dependency-gen.h Thu Dec 11 14:50:24 2014
> @@ -0,0 +1,11 @@
> +//#ifndef DEPENDENCY_GEN_H
> +//#define DEPENDENCY_GEN_H
> +
> +#include "dependency-gen-included.h"
> +
> +void f() {
> + g();
> + h();
> +}
> +
> +//#endif
>
> Added: cfe/trunk/test/Modules/dependency-gen.modulemap.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/dependency-gen.modulemap.cpp?rev=224055&view=auto
> ==============================================================================
> --- cfe/trunk/test/Modules/dependency-gen.modulemap.cpp (added)
> +++ cfe/trunk/test/Modules/dependency-gen.modulemap.cpp Thu Dec 11 14:50:24 2014
> @@ -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
>
> Modified: cfe/trunk/test/Modules/modular_maps.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/modular_maps.cpp?rev=224055&r1=224054&r2=224055&view=diff
> ==============================================================================
> --- cfe/trunk/test/Modules/modular_maps.cpp (original)
> +++ cfe/trunk/test/Modules/modular_maps.cpp Thu Dec 11 14:50:24 2014
> @@ -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
>
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
More information about the cfe-commits
mailing list