r243718 - [modules] Fix issue where building a module from a relative path when -working-directory option is set, results in error.

[Richard Smith]

On Thu, Jul 30, 2015 at 5:58 PM, Argyrios Kyrtzidis <akyrtzi at gmail.com>
wrote:

> Author: akirtzidis
> Date: Thu Jul 30 19:58:32 2015
> New Revision: 243718
>
> URL: http://llvm.org/viewvc/llvm-project?rev=243718&view=rev
> Log:
> [modules] Fix issue where building a module from a relative path when
> -working-directory option is set, results in error.
>
> The error was "module '<name>' was built in directory '<path>' but now
> resides in directory '<path>'
> rdar://21330027
>
> Added:
>     cfe/trunk/test/Modules/Inputs/working-dir-test/
>     cfe/trunk/test/Modules/Inputs/working-dir-test/Test.framework/
>     cfe/trunk/test/Modules/Inputs/working-dir-test/Test.framework/Headers/
>
> cfe/trunk/test/Modules/Inputs/working-dir-test/Test.framework/Headers/Test.h
>     cfe/trunk/test/Modules/Inputs/working-dir-test/Test.framework/Modules/
>
> cfe/trunk/test/Modules/Inputs/working-dir-test/Test.framework/Modules/module.modulemap
>     cfe/trunk/test/Modules/working-dir-flag.m
> Modified:
>     cfe/trunk/include/clang/Basic/FileManager.h
>     cfe/trunk/lib/Basic/FileManager.cpp
>     cfe/trunk/lib/Serialization/ASTWriter.cpp
>
> Modified: cfe/trunk/include/clang/Basic/FileManager.h
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/FileManager.h?rev=243718&r1=243717&r2=243718&view=diff
>
> ==============================================================================
> --- cfe/trunk/include/clang/Basic/FileManager.h (original)
> +++ cfe/trunk/include/clang/Basic/FileManager.h Thu Jul 30 19:58:32 2015
> @@ -254,7 +254,13 @@ public:
>    /// \brief If path is not absolute and FileSystemOptions set the working
>    /// directory, the path is modified to be relative to the given
>    /// working directory.
> -  void FixupRelativePath(SmallVectorImpl<char> &path) const;
> +  /// \returns true if \c path changed.
> +  bool FixupRelativePath(SmallVectorImpl<char> &path) const;
> +
> +  /// Makes \c Path absolute taking into account FileSystemOptions and the
> +  /// working directory option.
> +  /// \returns true if \c Path changed to absolute.
> +  bool makeAbsolutePath(SmallVectorImpl<char> &Path) const;
>
>    /// \brief Produce an array mapping from the unique IDs assigned to each
>    /// file to the corresponding FileEntry pointer.
>
> Modified: cfe/trunk/lib/Basic/FileManager.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Basic/FileManager.cpp?rev=243718&r1=243717&r2=243718&view=diff
>
> ==============================================================================
> --- cfe/trunk/lib/Basic/FileManager.cpp (original)
> +++ cfe/trunk/lib/Basic/FileManager.cpp Thu Jul 30 19:58:32 2015
> @@ -389,16 +389,28 @@ FileManager::getVirtualFile(StringRef Fi
>    return UFE;
>  }
>
> -void FileManager::FixupRelativePath(SmallVectorImpl<char> &path) const {
> +bool FileManager::FixupRelativePath(SmallVectorImpl<char> &path) const {
>    StringRef pathRef(path.data(), path.size());
>
>    if (FileSystemOpts.WorkingDir.empty()
>        || llvm::sys::path::is_absolute(pathRef))
> -    return;
> +    return false;
>
>    SmallString<128> NewPath(FileSystemOpts.WorkingDir);
>    llvm::sys::path::append(NewPath, pathRef);
>    path = NewPath;
> +  return true;
> +}
> +
> +bool FileManager::makeAbsolutePath(SmallVectorImpl<char> &Path) const {
> +  bool Changed = FixupRelativePath(Path);
> +
> +  if (!llvm::sys::path::is_absolute(StringRef(Path.data(), Path.size())))
> {
> +    llvm::sys::fs::make_absolute(Path);
> +    Changed = true;
> +  }
> +
> +  return Changed;
>  }
>
>  llvm::ErrorOr<std::unique_ptr<llvm::MemoryBuffer>>
>
> Modified: cfe/trunk/lib/Serialization/ASTWriter.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Serialization/ASTWriter.cpp?rev=243718&r1=243717&r2=243718&view=diff
>
> ==============================================================================
> --- cfe/trunk/lib/Serialization/ASTWriter.cpp (original)
> +++ cfe/trunk/lib/Serialization/ASTWriter.cpp Thu Jul 30 19:58:32 2015
> @@ -1074,14 +1074,7 @@ void ASTWriter::WriteBlockInfoBlock() {
>  /// \return \c true if the path was changed.
>  static 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);
> +  return FileMgr.makeAbsolutePath(Path) | FileMgr.removeDotPaths(Path);
>

These two operations are unsequenced, and I suspect the order in which
they're performed matters...


>  }
>
>  /// \brief Adjusts the given filename to only write out the portion of the
> @@ -1429,7 +1422,7 @@ void ASTWriter::WriteControlBlock(Prepro
>
>      SmallString<128> OutputPath(OutputFile);
>
> -    llvm::sys::fs::make_absolute(OutputPath);
> +    SM.getFileManager().makeAbsolutePath(OutputPath);
>      StringRef origDir = llvm::sys::path::parent_path(OutputPath);
>
>      RecordData Record;
>
> Added:
> cfe/trunk/test/Modules/Inputs/working-dir-test/Test.framework/Headers/Test.h
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/working-dir-test/Test.framework/Headers/Test.h?rev=243718&view=auto
>
> ==============================================================================
> ---
> cfe/trunk/test/Modules/Inputs/working-dir-test/Test.framework/Headers/Test.h
> (added)
> +++
> cfe/trunk/test/Modules/Inputs/working-dir-test/Test.framework/Headers/Test.h
> Thu Jul 30 19:58:32 2015
> @@ -0,0 +1 @@
> +void test_me_call(void);
>
> Added:
> cfe/trunk/test/Modules/Inputs/working-dir-test/Test.framework/Modules/module.modulemap
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/working-dir-test/Test.framework/Modules/module.modulemap?rev=243718&view=auto
>
> ==============================================================================
> ---
> cfe/trunk/test/Modules/Inputs/working-dir-test/Test.framework/Modules/module.modulemap
> (added)
> +++
> cfe/trunk/test/Modules/Inputs/working-dir-test/Test.framework/Modules/module.modulemap
> Thu Jul 30 19:58:32 2015
> @@ -0,0 +1,6 @@
> +framework module Test {
> +  umbrella header "Test.h"
> +
> +  export *
> +  module * { export * }
> +}
>
> Added: cfe/trunk/test/Modules/working-dir-flag.m
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/working-dir-flag.m?rev=243718&view=auto
>
> ==============================================================================
> --- cfe/trunk/test/Modules/working-dir-flag.m (added)
> +++ cfe/trunk/test/Modules/working-dir-flag.m Thu Jul 30 19:58:32 2015
> @@ -0,0 +1,9 @@
> +// RUN: rm -rf %t.mcp
> +// RUN: %clang_cc1 -fmodules-cache-path=%t.mcp -fmodules
> -fimplicit-module-maps -F . -working-directory=%S/Inputs/working-dir-test
> %s -verify
> +// expected-no-diagnostics
> +
> + at import Test;
> +
> +void foo() {
> +  test_me_call();
> +}
>
>
> _______________________________________________
> 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/20150730/70522a9e/attachment.html>