<div dir="rtl"><div dir="ltr">Hi Takumi,</div><div dir="ltr"><br></div><div dir="ltr">I wish DOS would have used '/' but now it's far too late. Even today it sometimes causes troubles building on Windows, problems with configure, make,...</div>

<div dir="ltr"><br></div><div dir="ltr">The current state in incosistent, where most of LLVM uses sys::path and thus '\\' (on WIndows) but some places use hardcodes '/'. </div><div dir="ltr"><br></div><div dir="ltr">

This patch does not make the '\\' or '/' decision but rather centralizes the decision to llvm::sys::path, so if someday we would like to switch all to '/' as you like, only path.cpp would be modified.<br>

</div><div dir="ltr"><br></div><div dir="ltr">Yaron</div><div dir="ltr"><br></div></div><div class="gmail_extra"><br><br><div class="gmail_quote"><div dir="ltr">2014-05-09 14:39 GMT+03:00 NAKAMURA Takumi <span dir="ltr"><<a href="mailto:geek4civic@gmail.com" target="_blank">geek4civic@gmail.com</a>></span>:</div>

<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">I prefer '/' could be used everywhere, unless it were illegal. I think<br>
'\\' makes the world worse.<br>
<br>
2014-05-09 19:53 GMT+09:00 Yaron Keren <<a href="mailto:yaron.keren@gmail.com">yaron.keren@gmail.com</a>>:<br>
<div class="HOEnZb"><div class="h5">> In the discussion linked, Reid mentioned that when cross-compiling aren't<br>
> the paths are valid on the host only, so shouldn't the host path seperator<br>
> be used? the target may not even have a file system.<br>
><br>
> The alternative is to have all path seperators as '/', but this disagrees<br>
> with path.cpp functionality and all its many clients.<br>
><br>
> Alp, is that whan you meant?<br>
><br>
><br>
> 2014-05-09 13:21 GMT+03:00 Alp Toker <<a href="mailto:alp@nuanti.com">alp@nuanti.com</a>>:<br>
>><br>
>><br>
>> On 09/05/2014 10:32, Yaron Keren wrote:<br>
>>><br>
>>> Hi echristo, rnk, chapuni,<br>
>>><br>
>>> Earlier duscussion:<br>
>>>   <a href="https://www.mail-archive.com/cfe-commits@cs.uiuc.edu/msg92891.html" target="_blank">https://www.mail-archive.com/cfe-commits@cs.uiuc.edu/msg92891.html</a><br>
>>><br>
>>> Most of LLVM code uses the llvm::sys::path::append function to<br>
>>> concatenate paths and will get the correct native slash. In some code<br>
>>> locations it seems that direct string concatenation was used, possibly<br>
>>> because the code writer knows there are no extra path sepeartors to deal<br>
>>> with, so string concat was more efficient than append.<br>
>>><br>
>>> In the locations below, the path seperator is hardcoded as '/'. Fixed by<br>
>>> exposing the native seperator from llvm::sys:;path and using it.<br>
>>><br>
>>> As an alternative, the append function could be used but it's more code<br>
>>> and less efficient.<br>
>>><br>
>>> <a href="http://reviews.llvm.org/D3687" target="_blank">http://reviews.llvm.org/D3687</a><br>
>>><br>
>>> Files:<br>
>>>    include/llvm/Support/Path.h<br>
>>>    lib/MC/MCDwarf.cpp<br>
>>>    lib/Support/Path.cpp<br>
>>>    lib/Support/SourceMgr.cpp<br>
>>><br>
>>> Index: include/llvm/Support/Path.h<br>
>>> ===================================================================<br>
>>> --- include/llvm/Support/Path.h<br>
>>> +++ include/llvm/Support/Path.h<br>
>>> @@ -295,6 +295,11 @@<br>
>>>   /// @result true if \a value is a path separator character on the host<br>
>>> OS<br>
>>>   bool is_separator(char value);<br>
>>>   +/// @brief Return the preferred separator for this platform.<br>
>>> +///<br>
>>> +/// @result const char* to the preferred separator, null-terminated.<br>
>>> +const char *get_separator();<br>
>>> +<br>
>>>   /// @brief Get the typical temporary directory for the system, e.g.,<br>
>>>   /// "/var/tmp" or "C:/TEMP"<br>
>>>   ///<br>
>>> Index: lib/Support/Path.cpp<br>
>>> ===================================================================<br>
>>> --- lib/Support/Path.cpp<br>
>>> +++ lib/Support/Path.cpp<br>
>>> @@ -569,6 +569,12 @@<br>
>>>     }<br>
>>>   }<br>
>>>   +static const char preferred_separator_string[] = {<br>
>>> preferred_separator, '\0' };<br>
>>> +<br>
>>> +const char *get_separator() {<br>
>>> +  return preferred_separator_string;<br>
>>> +}<br>
>>> +<br>
>>>   void system_temp_directory(bool erasedOnReboot, SmallVectorImpl<char><br>
>>> &result) {<br>
>>>     result.clear();<br>
>>>   Index: lib/MC/MCDwarf.cpp<br>
>>> ===================================================================<br>
>>> --- lib/MC/MCDwarf.cpp<br>
>>> +++ lib/MC/MCDwarf.cpp<br>
>>> @@ -689,7 +689,7 @@<br>
>>>     const SmallVectorImpl<std::string> &MCDwarfDirs =<br>
>>> context.getMCDwarfDirs();<br>
>>>     if (MCDwarfDirs.size() > 0) {<br>
>>>       MCOS->EmitBytes(MCDwarfDirs[0]);<br>
>>> -    MCOS->EmitBytes("/");<br>
>>> +    MCOS->EmitBytes(sys::path::get_separator());<br>
>>>     }<br>
>>>     const SmallVectorImpl<MCDwarfFile> &MCDwarfFiles =<br>
>>>       MCOS->getContext().getMCDwarfFiles();<br>
>><br>
>><br>
>> I know there isn't much clarity yet on the issue but it's worth mentioning<br>
>> (in the commit message or a comment) the current state of affairs regarding<br>
>> paths in cross-compiled debug info.<br>
>><br>
>> Alp.<br>
>><br>
>><br>
>><br>
>>> Index: lib/Support/SourceMgr.cpp<br>
>>> ===================================================================<br>
>>> --- lib/Support/SourceMgr.cpp<br>
>>> +++ lib/Support/SourceMgr.cpp<br>
>>> @@ -18,6 +18,7 @@<br>
>>>   #include "llvm/ADT/Twine.h"<br>
>>>   #include "llvm/Support/Locale.h"<br>
>>>   #include "llvm/Support/MemoryBuffer.h"<br>
>>> +#include "llvm/Support/Path.h"<br>
>>>   #include "llvm/Support/raw_ostream.h"<br>
>>>   #include "llvm/Support/system_error.h"<br>
>>>   using namespace llvm;<br>
>>> @@ -60,7 +61,7 @@<br>
>>>       // If the file didn't exist directly, see if it's in an include<br>
>>> path.<br>
>>>     for (unsigned i = 0, e = IncludeDirectories.size(); i != e &&<br>
>>> !NewBuf; ++i) {<br>
>>> -    IncludedFile = IncludeDirectories[i] + "/" + Filename;<br>
>>> +    IncludedFile = IncludeDirectories[i] + sys::path::get_separator() +<br>
>>> Filename;<br>
>>>       MemoryBuffer::getFile(IncludedFile.c_str(), NewBuf);<br>
>>>     }<br>
>>><br>
>>><br>
>>> _______________________________________________<br>
>>> llvm-commits mailing list<br>
>>> <a href="mailto:llvm-commits@cs.uiuc.edu">llvm-commits@cs.uiuc.edu</a><br>
>>> <a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits</a><br>
>><br>
>><br>
>> --<br>
>> <a href="http://www.nuanti.com" target="_blank">http://www.nuanti.com</a><br>
>> the browser experts<br>
>><br>
><br>
</div></div></blockquote></div><br></div>