<div dir="rtl"><div dir="ltr">In the discussion linked, Reid mentioned that when cross-compiling aren't the paths are valid on the host only, so shouldn't the host path seperator be used? the target may not even have a file system.</div>

<div dir="ltr"><br></div><div dir="ltr">The alternative is to have all path seperators as '/', but this disagrees with path.cpp functionality and all its many clients.</div><div dir="ltr"><br></div><div dir="ltr">

Alp, is that whan you meant?</div><div dir="ltr"><br></div><div dir="ltr"><br></div><div class="gmail_extra" dir="ltr"><div class="gmail_quote"><div>2014-05-09 13:21 GMT+03:00 Alp Toker <span dir="ltr"><<a href="mailto:alp@nuanti.com" target="_blank">alp@nuanti.com</a>></span>:</div>

<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div class="HOEnZb"><div class="h5"><br>
On 09/05/2014 10:32, Yaron Keren wrote:<br>
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
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/<u></u>cfe-commits@cs.uiuc.edu/<u></u>msg92891.html</a><br>
<br>
Most of LLVM code uses the llvm::sys::path::append function to concatenate paths and will get the correct native slash. In some code locations it seems that direct string concatenation was used, possibly because the code writer knows there are no extra path sepeartors to deal with, so string concat was more efficient than append.<br>


<br>
In the locations below, the path seperator is hardcoded as '/'. Fixed by 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 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>
==============================<u></u>==============================<u></u>=======<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 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>
==============================<u></u>==============================<u></u>=======<br>
--- lib/Support/Path.cpp<br>
+++ lib/Support/Path.cpp<br>
@@ -569,6 +569,12 @@<br>
    }<br>
  }<br>
  +static const char preferred_separator_string[] = { 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> &result) {<br>
    result.clear();<br>
  Index: lib/MC/MCDwarf.cpp<br>
==============================<u></u>==============================<u></u>=======<br>
--- lib/MC/MCDwarf.cpp<br>
+++ lib/MC/MCDwarf.cpp<br>
@@ -689,7 +689,7 @@<br>
    const SmallVectorImpl<std::string> &MCDwarfDirs = context.getMCDwarfDirs();<br>
    if (MCDwarfDirs.size() > 0) {<br>
      MCOS->EmitBytes(MCDwarfDirs[0]<u></u>);<br>
-    MCOS->EmitBytes("/");<br>
+    MCOS->EmitBytes(sys::path::<u></u>get_separator());<br>
    }<br>
    const SmallVectorImpl<MCDwarfFile> &MCDwarfFiles =<br>
      MCOS->getContext().<u></u>getMCDwarfFiles();<br>
</blockquote>
<br></div></div>
I know there isn't much clarity yet on the issue but it's worth mentioning (in the commit message or a comment) the current state of affairs regarding paths in cross-compiled debug info.<br>
<br>
Alp.<br>
<br>
<br>
<br>
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div class="">
Index: lib/Support/SourceMgr.cpp<br>
==============================<u></u>==============================<u></u>=======<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 path.<br>
    for (unsigned i = 0, e = IncludeDirectories.size(); i != e && !NewBuf; ++i) {<br>
-    IncludedFile = IncludeDirectories[i] + "/" + Filename;<br>
+    IncludedFile = IncludeDirectories[i] + sys::path::get_separator() + Filename;<br>
      MemoryBuffer::getFile(<u></u>IncludedFile.c_str(), NewBuf);<br>
    }<br>
<br>
<br></div>
______________________________<u></u>_________________<br>
llvm-commits mailing list<br>
<a href="mailto:llvm-commits@cs.uiuc.edu" target="_blank">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/<u></u>mailman/listinfo/llvm-commits</a><span class="HOEnZb"><font color="#888888"><br>
</font></span></blockquote><span class="HOEnZb"><font color="#888888">
<br>
-- <br>
<a href="http://www.nuanti.com" target="_blank">http://www.nuanti.com</a><br>
the browser experts<br>
<br>
</font></span></blockquote></div><br></div></div>