r224055 - [modules] When constructing paths relative to a module, strip out /./ directory
Richard Smith
richard at metafoo.co.uk
Fri Dec 12 14:43:33 PST 2014
Bot failure was fixed in r224145.
On Fri, Dec 12, 2014 at 5:59 AM, Aaron Ballman <aaron at aaronballman.com>
wrote:
> 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
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20141212/1fc21d5c/attachment.html>
More information about the cfe-commits
mailing list