Make FileSpec::Dump use FileSpec::GetPath(), not the other way around.

[chaoren]

Summary

Fix FileSpec::Dump() to output denormalized path.

See D9942 for previous discussions.

[zturner]

looks good, just some style nitpicks.

include/lldb/Host/FileSpec.h

634

I think this should either be a const member function that operates on m_syntax or a file static function in the .cpp file.

source/Host/common/FileSpec.cpp

626

Why curly braces instead of parentheses?

829

I'm not sure what the style guide says (if anything) but I personally find it a little hard to read lots of "if (x) y;" on a single line. very close together.

[chaoren]

include/lldb/Host/FileSpec.h

634

Yeah, I agree it'd be better if it's contained in the .cpp file. Normalize/DeNormalize (and maybe even GetPathSeparator) should probably also be in there.

source/Host/common/FileSpec.cpp

626

Since we're using C++11, I prefer uniform initialization whenever possible.

829

Yeah, I have no strong opinion on this. Will update.

[chaoren]

• Address review comments.

[zturner]

Looks good.

include/lldb/Host/FileSpec.h

public:
     /// Dump this object to a Stream.
     ///
     /// Dump the object to the supplied stream \a s. If the object
     /// contains a valid directory name, it will be displayed followed
     /// by a directory delimiter, and the filename.
     ///
     /// @param[in] s
     ///     The stream to which to dump the object description.
-    ///
-    /// @param[in] trailing_slash
-    ///     If true and the file is a non root directory, then a trailing slash
-    ///     will be added.
     //------------------------------------------------------------------
     void
-    Dump(Stream *s, bool trailing_slash = true) const;
+    Dump(Stream *s) const;
 
     //------------------------------------------------------------------
     /// Existence test.
     ///
     /// @return
     ///     \b true if the file exists on disk, \b false otherwise.
     //------------------------------------------------------------------
     bool
public:
     /// @return
     ///     A shared pointer to the data. This shared pointer can
     ///     contain a NULL DataBuffer pointer, so the contained pointer
     ///     must be checked prior to using it.
     //------------------------------------------------------------------
     lldb::DataBufferSP
     ReadFileContentsAsCString(Error *error_ptr = NULL);
 
+    static bool PathSyntaxIsPosix(PathSyntax syntax);
+
     static void Normalize(llvm::SmallVectorImpl<char> &path, PathSyntax syntax = ePathSyntaxHostNative);
     static void DeNormalize(llvm::SmallVectorImpl<char> &path, PathSyntax syntax = ePathSyntaxHostNative);
 
+    char GetPathSeparator() const;
 
     //------------------------------------------------------------------
     /// Run through the input string, replaying the effect of any ".." and produce
     /// the resultant path.  The input path is not required to be in the host file system
     /// format, but it is required to be normalized to that system.
     ///
     /// @param[in] input
     ///     The input path to analyze.

source/Host/common/FileSpec.cpp

if (this != &rhs)
         m_directory = rhs.m_directory;
         m_filename = rhs.m_filename;
         m_is_resolved = rhs.m_is_resolved;
         m_syntax = rhs.m_syntax;
     }
     return *this;
 }
 
+bool
+FileSpec::PathSyntaxIsPosix(PathSyntax syntax)
+{
+    return (syntax == ePathSyntaxPosix ||
+            (syntax == ePathSyntaxHostNative &&
+             FileSystem::GetNativePathSyntax() == ePathSyntaxPosix));
+}
+
 void FileSpec::Normalize(llvm::SmallVectorImpl<char> &path, PathSyntax syntax)
 {
-    if (syntax == ePathSyntaxPosix ||
-        (syntax == ePathSyntaxHostNative && FileSystem::GetNativePathSyntax() == ePathSyntaxPosix))
-        return;
+    if (PathSyntaxIsPosix(syntax)) return;
 
     std::replace(path.begin(), path.end(), '\\', '/');
     // Windows path can have \\ slashes which can be changed by replace
     // call above to //. Here we remove the duplicate.
     auto iter = std::unique ( path.begin(), path.end(),
                                []( char &c1, char &c2 ){
                                   return (c1 == '/' && c2 == '/');});
     path.erase(iter, path.end());
 }
 
 void FileSpec::DeNormalize(llvm::SmallVectorImpl<char> &path, PathSyntax syntax)
 {
-    if (syntax == ePathSyntaxPosix ||
-        (syntax == ePathSyntaxHostNative && FileSystem::GetNativePathSyntax() == ePathSyntaxPosix))
-        return;
+    if (PathSyntaxIsPosix(syntax)) return;
 
     std::replace(path.begin(), path.end(), '/', '\\');
 }
 
+char
+FileSpec::GetPathSeparator() const
+{
+    return PathSyntaxIsPosix(m_syntax) ? '/' : '\\';
+}
+
 //------------------------------------------------------------------
 // Update the contents of this object with a new path. The path will
 // be split up into a directory and filename and stored as uniqued
 // string values for quick comparison and efficient memory usage.
 //------------------------------------------------------------------
 void
 FileSpec::SetFile (const char *pathname, bool resolve, PathSyntax syntax)
 {

 }
 
 //------------------------------------------------------------------
 // Dump the object to the supplied stream. If the object contains
 // a valid directory name, it will be displayed followed by a
 // directory delimiter, and the filename.
 //------------------------------------------------------------------
 void
-FileSpec::Dump(Stream *s, bool trailing_slash) const
+FileSpec::Dump(Stream *s) const
 {
     if (s)
     {
-        m_directory.Dump(s);
-        if ((m_filename || trailing_slash) && m_directory &&
-                !m_directory.GetStringRef().endswith("/"))
-            s->PutChar('/');
-        m_filename.Dump(s);
+        std::string path{GetPath(true)};
+        s->PutCString(path.c_str());
+        if (!m_filename && !path.empty() && path.back() != GetPathSeparator())
+            s->PutChar(GetPathSeparator());
     }
 }
 
 //------------------------------------------------------------------
 // Returns true if the file exists.
 //------------------------------------------------------------------
 bool
 FileSpec::Exists () const
FileSpec::GetPath(bool denormalize) const
     llvm::SmallString<64> result;
     GetPath(result, denormalize);
     return std::string(result.begin(), result.end());
 }
 
 void
 FileSpec::GetPath(llvm::SmallVectorImpl<char> &path, bool denormalize) const
 {
-    StreamString stream;
-    Dump(&stream, false);
-    path.append(stream.GetString().begin(), stream.GetString().end());
+    path.append(m_directory.GetStringRef().begin(), m_directory.GetStringRef().end());
+    if (m_directory) path.insert(path.end(), '/');
+    path.append(m_filename.GetStringRef().begin(), m_filename.GetStringRef().end());
     Normalize(path, m_syntax);
+    if (path.size() > 1 && path.back() == '/') path.pop_back();
     if (denormalize && !path.empty())
         DeNormalize(path, m_syntax);
 }
 
 ConstString
 FileSpec::GetFileNameExtension () const
 {
     if (m_filename)

 bool
 FileSpec::IsRelativeToCurrentWorkingDirectory () const
 {
     const char *dir = m_directory.GetCString();
     llvm::StringRef directory(dir ? dir : "");
 
     if (directory.size() > 0)
     {
-        if (m_syntax == ePathSyntaxWindows)
-        {
-            if (directory.size() >= 2 && directory[1] == ':')
-                return false;
-            if (directory[0] == '/')
-                return false;
-            return true;
-        }
-        else
+        if (PathSyntaxIsPosix(m_syntax))
         {
             // If the path doesn't start with '/' or '~', return true
             switch (directory[0])
             {
                 case '/':
                 case '~':
                     return false;
                 default:
                     return true;
             }
         }
+        else
+        {
+            if (directory.size() >= 2 && directory[1] == ':')
+                return false;
+            if (directory[0] == '/')
+                return false;
+            return true;
+        }
     }
     else if (m_filename)
     {
         // No directory, just a basename, return true
         return true;
     }
     return false;
 }