[prev in list] [next in list] [prev in thread] [next in thread]
List: cfe-commits
Subject: Re: r243718 - [modules] Fix issue where building a module from a relative path when -working-directo
From: Richard Smith <richard () metafoo ! co ! uk>
Date: 2015-07-31 1:04:41
Message-ID: CAOfiQqkn=iP-zty86+LHdjHVs60L8AaTegS4nsT+RV2kP=VpyA () mail ! gmail ! com
[Download RAW message or body]
[Attachment #2 (multipart/alternative)]
On Thu, Jul 30, 2015 at 5:58 PM, Argyrios Kyrtzidis <akyrtzi@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
> +
> +@import Test;
> +
> +void foo() {
> + test_me_call();
> +}
>
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits@cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>
[Attachment #5 (text/html)]
<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Thu, Jul 30, 2015 \
at 5:58 PM, Argyrios Kyrtzidis <span dir="ltr"><<a href="mailto:akyrtzi@gmail.com" \
target="_blank">akyrtzi@gmail.com</a>></span> wrote:<br><blockquote \
class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc \
solid;padding-left:1ex">Author: akirtzidis<br>
Date: Thu Jul 30 19:58:32 2015<br>
New Revision: 243718<br>
<br>
URL: <a href="https://urldefense.proofpoint.com/v2/url?u=http-3A__llvm.org_viewvc_llvm \
-2Dproject-3Frev-3D243718-26view-3Drev&d=AwMFaQ&c=8hUWFZcy2Z-Za5rBPlktOQ&r=BSqEv9KvKMW \
_Ob8SyngJ70KdZISM_ASROnREeq0cCxk&m=8prpeY8zE-FlbXUPr4nwuIQDcr6evtVSB_NNxVOGQIw&s=5iEHQVEszDOP6Xtb_Xc1xUwiODzls8xe43UIAGTZgkE&e=" \
rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project?rev=243718&view=rev</a><br>
Log:<br>
[modules] Fix issue where building a module from a relative path when \
-working-directory option is set, results in error.<br> <br>
The error was "module '<name>' was built in directory \
'<path>' but now resides in directory '<path>'<br> \
rdar://21330027<br> <br>
Added:<br>
cfe/trunk/test/Modules/Inputs/working-dir-test/<br>
cfe/trunk/test/Modules/Inputs/working-dir-test/Test.framework/<br>
cfe/trunk/test/Modules/Inputs/working-dir-test/Test.framework/Headers/<br>
cfe/trunk/test/Modules/Inputs/working-dir-test/Test.framework/Headers/Test.h<br>
cfe/trunk/test/Modules/Inputs/working-dir-test/Test.framework/Modules/<br>
cfe/trunk/test/Modules/Inputs/working-dir-test/Test.framework/Modules/module.modulemap<br>
cfe/trunk/test/Modules/working-dir-flag.m<br>
Modified:<br>
cfe/trunk/include/clang/Basic/FileManager.h<br>
cfe/trunk/lib/Basic/FileManager.cpp<br>
cfe/trunk/lib/Serialization/ASTWriter.cpp<br>
<br>
Modified: cfe/trunk/include/clang/Basic/FileManager.h<br>
URL: <a href="https://urldefense.proofpoint.com/v2/url?u=http-3A__llvm.org_viewvc_llvm \
-2Dproject_cfe_trunk_include_clang_Basic_FileManager.h-3Frev-3D243718-26r1-3D243717-26 \
r2-3D243718-26view-3Ddiff&d=AwMFaQ&c=8hUWFZcy2Z-Za5rBPlktOQ&r=BSqEv9KvKMW_Ob8SyngJ70Kd \
ZISM_ASROnREeq0cCxk&m=8prpeY8zE-FlbXUPr4nwuIQDcr6evtVSB_NNxVOGQIw&s=LybD8jX08w9nhrgPl1_StxKF0HlSh6t05RUE3OFBvX0&e=" \
rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/include \
/clang/Basic/FileManager.h?rev=243718&r1=243717&r2=243718&view=diff</a><br>
==============================================================================<br>
--- cfe/trunk/include/clang/Basic/FileManager.h (original)<br>
+++ cfe/trunk/include/clang/Basic/FileManager.h Thu Jul 30 19:58:32 2015<br>
@@ -254,7 +254,13 @@ public:<br>
/// \brief If path is not absolute and FileSystemOptions set the working<br>
/// directory, the path is modified to be relative to the given<br>
/// working directory.<br>
- void FixupRelativePath(SmallVectorImpl<char> &path) const;<br>
+ /// \returns true if \c path changed.<br>
+ bool FixupRelativePath(SmallVectorImpl<char> &path) const;<br>
+<br>
+ /// Makes \c Path absolute taking into account FileSystemOptions and the<br>
+ /// working directory option.<br>
+ /// \returns true if \c Path changed to absolute.<br>
+ bool makeAbsolutePath(SmallVectorImpl<char> &Path) const;<br>
<br>
/// \brief Produce an array mapping from the unique IDs assigned to each<br>
/// file to the corresponding FileEntry pointer.<br>
<br>
Modified: cfe/trunk/lib/Basic/FileManager.cpp<br>
URL: <a href="https://urldefense.proofpoint.com/v2/url?u=http-3A__llvm.org_viewvc_llvm \
-2Dproject_cfe_trunk_lib_Basic_FileManager.cpp-3Frev-3D243718-26r1-3D243717-26r2-3D243 \
718-26view-3Ddiff&d=AwMFaQ&c=8hUWFZcy2Z-Za5rBPlktOQ&r=BSqEv9KvKMW_Ob8SyngJ70KdZISM_ASR \
OnREeq0cCxk&m=8prpeY8zE-FlbXUPr4nwuIQDcr6evtVSB_NNxVOGQIw&s=Odbf202qdUcQK3bIIKSAIzZ2-xnlYfDpKUrmoGgp0Js&e=" \
rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Basic/FileManager.cpp?rev=243718&r1=243717&r2=243718&view=diff</a><br>
==============================================================================<br>
--- cfe/trunk/lib/Basic/FileManager.cpp (original)<br>
+++ cfe/trunk/lib/Basic/FileManager.cpp Thu Jul 30 19:58:32 2015<br>
@@ -389,16 +389,28 @@ FileManager::getVirtualFile(StringRef Fi<br>
return UFE;<br>
}<br>
<br>
-void FileManager::FixupRelativePath(SmallVectorImpl<char> &path) const \
{<br> +bool FileManager::FixupRelativePath(SmallVectorImpl<char> &path) \
const {<br> StringRef pathRef(path.data(), path.size());<br>
<br>
if (FileSystemOpts.WorkingDir.empty()<br>
|| llvm::sys::path::is_absolute(pathRef))<br>
- return;<br>
+ return false;<br>
<br>
SmallString<128> NewPath(FileSystemOpts.WorkingDir);<br>
llvm::sys::path::append(NewPath, pathRef);<br>
path = NewPath;<br>
+ return true;<br>
+}<br>
+<br>
+bool FileManager::makeAbsolutePath(SmallVectorImpl<char> &Path) const \
{<br> + bool Changed = FixupRelativePath(Path);<br>
+<br>
+ if (!llvm::sys::path::is_absolute(StringRef(Path.data(), Path.size()))) {<br>
+ llvm::sys::fs::make_absolute(Path);<br>
+ Changed = true;<br>
+ }<br>
+<br>
+ return Changed;<br>
}<br>
<br>
llvm::ErrorOr<std::unique_ptr<llvm::MemoryBuffer>><br>
<br>
Modified: cfe/trunk/lib/Serialization/ASTWriter.cpp<br>
URL: <a href="https://urldefense.proofpoint.com/v2/url?u=http-3A__llvm.org_viewvc_llvm \
-2Dproject_cfe_trunk_lib_Serialization_ASTWriter.cpp-3Frev-3D243718-26r1-3D243717-26r2 \
-3D243718-26view-3Ddiff&d=AwMFaQ&c=8hUWFZcy2Z-Za5rBPlktOQ&r=BSqEv9KvKMW_Ob8SyngJ70KdZI \
SM_ASROnREeq0cCxk&m=8prpeY8zE-FlbXUPr4nwuIQDcr6evtVSB_NNxVOGQIw&s=veoePDCGGJLdADwp8OEzG9MSpqmMuoDzqryVT2RrlP8&e=" \
rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Ser \
ialization/ASTWriter.cpp?rev=243718&r1=243717&r2=243718&view=diff</a><br> \
==============================================================================<br>
--- cfe/trunk/lib/Serialization/ASTWriter.cpp (original)<br>
+++ cfe/trunk/lib/Serialization/ASTWriter.cpp Thu Jul 30 19:58:32 2015<br>
@@ -1074,14 +1074,7 @@ void ASTWriter::WriteBlockInfoBlock() {<br>
/// \return \c true if the path was changed.<br>
static bool cleanPathForOutput(FileManager &FileMgr,<br>
SmallVectorImpl<char> \
&Path) {<br>
- bool Changed = false;<br>
-<br>
- if (!llvm::sys::path::is_absolute(StringRef(Path.data(), Path.size()))) {<br>
- llvm::sys::fs::make_absolute(Path);<br>
- Changed = true;<br>
- }<br>
-<br>
- return Changed | FileMgr.removeDotPaths(Path);<br>
+ return FileMgr.makeAbsolutePath(Path) | \
FileMgr.removeDotPaths(Path);<br></blockquote><div><br></div><div>These two \
operations are unsequenced, and I suspect the order in which they're performed \
matters...</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 \
.8ex;border-left:1px #ccc solid;padding-left:1ex"> }<br>
<br>
/// \brief Adjusts the given filename to only write out the portion of the<br>
@@ -1429,7 +1422,7 @@ void ASTWriter::WriteControlBlock(Prepro<br>
<br>
SmallString<128> OutputPath(OutputFile);<br>
<br>
- llvm::sys::fs::make_absolute(OutputPath);<br>
+ SM.getFileManager().makeAbsolutePath(OutputPath);<br>
StringRef origDir = llvm::sys::path::parent_path(OutputPath);<br>
<br>
RecordData Record;<br>
<br>
Added: cfe/trunk/test/Modules/Inputs/working-dir-test/Test.framework/Headers/Test.h<br>
URL: <a href="https://urldefense.proofpoint.com/v2/url?u=http-3A__llvm.org_viewvc_llvm \
-2Dproject_cfe_trunk_test_Modules_Inputs_working-2Ddir-2Dtest_Test.framework_Headers_T \
est.h-3Frev-3D243718-26view-3Dauto&d=AwMFaQ&c=8hUWFZcy2Z-Za5rBPlktOQ&r=BSqEv9KvKMW_Ob8 \
SyngJ70KdZISM_ASROnREeq0cCxk&m=8prpeY8zE-FlbXUPr4nwuIQDcr6evtVSB_NNxVOGQIw&s=Umb92o_ULXEn0v6kv5zMu9n2d_OpIjVb2HatTUGUhqA&e=" \
rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Mo \
dules/Inputs/working-dir-test/Test.framework/Headers/Test.h?rev=243718&view=auto</a><br>
==============================================================================<br>
--- cfe/trunk/test/Modules/Inputs/working-dir-test/Test.framework/Headers/Test.h \
(added)<br>
+++ cfe/trunk/test/Modules/Inputs/working-dir-test/Test.framework/Headers/Test.h Thu \
Jul 30 19:58:32 2015<br> @@ -0,0 +1 @@<br>
+void test_me_call(void);<br>
<br>
Added: cfe/trunk/test/Modules/Inputs/working-dir-test/Test.framework/Modules/module.modulemap<br>
URL: <a href="https://urldefense.proofpoint.com/v2/url?u=http-3A__llvm.org_viewvc_llvm \
-2Dproject_cfe_trunk_test_Modules_Inputs_working-2Ddir-2Dtest_Test.framework_Modules_m \
odule.modulemap-3Frev-3D243718-26view-3Dauto&d=AwMFaQ&c=8hUWFZcy2Z-Za5rBPlktOQ&r=BSqEv \
9KvKMW_Ob8SyngJ70KdZISM_ASROnREeq0cCxk&m=8prpeY8zE-FlbXUPr4nwuIQDcr6evtVSB_NNxVOGQIw&s=6pAQfHUxT6OnIRwt6Jd9k6EhTKW6U0rvy1F2vR5l9ck&e=" \
rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Mo \
dules/Inputs/working-dir-test/Test.framework/Modules/module.modulemap?rev=243718&view=auto</a><br>
==============================================================================<br>
--- cfe/trunk/test/Modules/Inputs/working-dir-test/Test.framework/Modules/module.modulemap \
(added)<br>
+++ cfe/trunk/test/Modules/Inputs/working-dir-test/Test.framework/Modules/module.modulemap \
Thu Jul 30 19:58:32 2015<br> @@ -0,0 +1,6 @@<br>
+framework module Test {<br>
+ umbrella header "Test.h"<br>
+<br>
+ export *<br>
+ module * { export * }<br>
+}<br>
<br>
Added: cfe/trunk/test/Modules/working-dir-flag.m<br>
URL: <a href="https://urldefense.proofpoint.com/v2/url?u=http-3A__llvm.org_viewvc_llvm \
-2Dproject_cfe_trunk_test_Modules_working-2Ddir-2Dflag.m-3Frev-3D243718-26view-3Dauto& \
d=AwMFaQ&c=8hUWFZcy2Z-Za5rBPlktOQ&r=BSqEv9KvKMW_Ob8SyngJ70KdZISM_ASROnREeq0cCxk&m=8prp \
eY8zE-FlbXUPr4nwuIQDcr6evtVSB_NNxVOGQIw&s=MxzwAzM1mIcJmsLrTzuar6C4Yn4ruuH3CBPwQmcaq94&e=" \
rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/working-dir-flag.m?rev=243718&view=auto</a><br>
==============================================================================<br>
--- cfe/trunk/test/Modules/working-dir-flag.m (added)<br>
+++ cfe/trunk/test/Modules/working-dir-flag.m Thu Jul 30 19:58:32 2015<br>
@@ -0,0 +1,9 @@<br>
+// RUN: rm -rf %t.mcp<br>
+// RUN: %clang_cc1 -fmodules-cache-path=%t.mcp -fmodules -fimplicit-module-maps -F . \
-working-directory=%S/Inputs/working-dir-test %s -verify<br> +// \
expected-no-diagnostics<br> +<br>
+@import Test;<br>
+<br>
+void foo() {<br>
+ test_me_call();<br>
+}<br>
<br>
<br>
_______________________________________________<br>
cfe-commits mailing list<br>
<a href="mailto:cfe-commits@cs.uiuc.edu">cfe-commits@cs.uiuc.edu</a><br>
<a href="http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits" rel="noreferrer" \
target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits</a><br> \
</blockquote></div><br></div></div>
_______________________________________________
cfe-commits mailing list
cfe-commits@cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic