Fix hardcoded path seperator in two code locations

[yaron.keren]

Summary

Earlier duscussion:
https://www.mail-archive.com/[e-mail]/msg92891.html

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.

In the locations below, the path seperator is hardcoded as '/'. Fixed by exposing the native seperator from llvm::sys:;path and using it.

As an alternative, the append function could be used but it's more code and less efficient.

[rnk]

LGTM

Let's be consistent with what append does. If users want to do cross-compiling, we'll need to thread a flag down to fs::append() so it also does the right thing.

include/llvm/Support/Path.h

301

This should return a StringRef.

[yaron.keren]

With StringRef.

[rnk]

lib/Support/Path.cpp

573

We don't need this global data. It will probably require a dynamic initializer, which is lame. Clang is smart enough to optimize out strlen if you just return preferred_separator directly:

$ cat t.cpp
extern "C" int strlen(const char *s);
struct StringRef {
  const char *p;
  size_t len;
  StringRef(const char *s) : p(s), len(strlen(s)) {}
};
static const char preferred_sep[] = { '/', '\0' };
StringRef getPrefSep() {
  return StringRef(preferred_sep);
}

$ clang -c t.cpp -O2 -S -o - --target=x86_64-linux -fPIC | grep '^\s[^\.]'
        leaq    _ZL13preferred_sep(%rip), %rax
        movl    $1, %edx
        retq

[yaron.keren]

Static StringRef removed.

[rnk]

Feel free to commit, those where just nits.

[yaron.keren]

Committed revision 208980.

include/llvm/Support/Path.h

 /// @result true if \a value is a path separator character on the host OS
 bool is_separator(char value);
 
+/// @brief Return the preferred separator for this platform.
+///
+/// @result StringRef of the preferred separator, null-terminated.
+const StringRef get_separator();
+
 /// @brief Get the typical temporary directory for the system, e.g., 
 /// "/var/tmp" or "C:/TEMP"
 ///

lib/MC/MCDwarf.cpp

   const SmallVectorImpl<std::string> &MCDwarfDirs = context.getMCDwarfDirs();
   if (MCDwarfDirs.size() > 0) {
     MCOS->EmitBytes(MCDwarfDirs[0]);
-    MCOS->EmitBytes("/");
+    MCOS->EmitBytes(sys::path::get_separator());
   }
   const SmallVectorImpl<MCDwarfFile> &MCDwarfFiles =
     MCOS->getContext().getMCDwarfFiles();

lib/Support/Path.cpp

   }
 }
 
+static const char preferred_separator_string[] = { preferred_separator, '\0' };
+
+const StringRef get_separator() {
+  return preferred_separator_string;
+}
+
 void system_temp_directory(bool erasedOnReboot, SmallVectorImpl<char> &result) {
   result.clear();
 

lib/Support/SourceMgr.cpp

 #include "llvm/ADT/Twine.h"
 #include "llvm/Support/Locale.h"
 #include "llvm/Support/MemoryBuffer.h"
+#include "llvm/Support/Path.h"
 #include "llvm/Support/raw_ostream.h"
 #include "llvm/Support/system_error.h"
 using namespace llvm;

 
   // If the file didn't exist directly, see if it's in an include path.
   for (unsigned i = 0, e = IncludeDirectories.size(); i != e && !NewBuf; ++i) {
-    IncludedFile = IncludeDirectories[i] + "/" + Filename;
+    IncludedFile = IncludeDirectories[i] + sys::path::get_separator().data() + Filename;
     MemoryBuffer::getFile(IncludedFile.c_str(), NewBuf);
   }