Delegate path operations to FileSpec.

[chaoren]

Summary

• Added PrependPathComponent utility functions to FileSpec.
• Delegate path operations in ParseCompileUnit to FileSpec.
• Delegate path operations in ParseSupportFiles to FileSpec.

[chaoren]

• Added PrependPathComponent utility functions to FileSpec.
• Delegate path operations in ParseCompileUnit to FileSpec.
• Delegate path operations in ParseSupportFiles to FileSpec.

[ovyalov]

Please see my comments.

source/Host/common/FileSpec.cpp

1365 ↗

(On Diff #27156)

Could you add a check for !new_path and return if true?

source/Plugins/SymbolFile/DWARF/DWARFDebugLine.cpp

526 ↗

(On Diff #27156)

const FileSpec& ?

source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp

984 ↗

(On Diff #27156)

s/NULL/nullptr

[chaoren]

• Addressed review comments.

[ovyalov]

LGTM

lldb/trunk/include/lldb/Host/FileSpec.h

public:
     static void
     Resolve (llvm::SmallVectorImpl<char> &path);
 
     FileSpec
     CopyByAppendingPathComponent (const char *new_path) const;
     
     FileSpec
     CopyByRemovingLastPathComponent () const;
-    
     void
-    AppendPathComponent (const char *new_path);
+    PrependPathComponent(const char *new_path);
+
+    void
+    PrependPathComponent(const std::string &new_path);
+
+    void
+    PrependPathComponent(const FileSpec &new_path);
+
+    void
+    AppendPathComponent(const char *new_path);
 
     void
     AppendPathComponent(const std::string &new_path);
 
     void
+    AppendPathComponent(const FileSpec &new_path);
+
+    void
     RemoveLastPathComponent ();
     
     ConstString
     GetLastPathComponent () const;
     
     //------------------------------------------------------------------
     /// Resolves the user name at the beginning of \a src_path, and writes the output
     /// to \a dst_path.  Note, \a src_path can contain other path components after the

lldb/trunk/source/Host/common/FileSpec.cpp

if (m_directory)
         }
         ConstString result(penultimate_slash_ptr+1,last_slash_ptr-penultimate_slash_ptr);
         return result;
     }
     return ConstString();
 }
 
 void
-FileSpec::AppendPathComponent (const char *new_path)
+FileSpec::PrependPathComponent(const char *new_path)
+{
+    if (!new_path) return;
+    const bool resolve = false;
+    if (m_filename.IsEmpty() && m_directory.IsEmpty())
+    {
+        SetFile(new_path, resolve);
+        return;
+    }
+    StreamString stream;
+    if (m_filename.IsEmpty())
+        stream.Printf("%s/%s", new_path, m_directory.GetCString());
+    else if (m_directory.IsEmpty())
+        stream.Printf("%s/%s", new_path, m_filename.GetCString());
+    else
+        stream.Printf("%s/%s/%s", new_path, m_directory.GetCString(), m_filename.GetCString());
+    SetFile(stream.GetData(), resolve);
+}
+
+void
+FileSpec::PrependPathComponent(const std::string &new_path)
+{
+    return PrependPathComponent(new_path.c_str());
+}
+
+void
+FileSpec::PrependPathComponent(const FileSpec &new_path)
+{
+    return PrependPathComponent(new_path.GetPath(false));
+}
+
+void
+FileSpec::AppendPathComponent(const char *new_path)
 {
+    if (!new_path) return;
     const bool resolve = false;
     if (m_filename.IsEmpty() && m_directory.IsEmpty())
     {
-        SetFile(new_path,resolve);
+        SetFile(new_path, resolve);
         return;
     }
     StreamString stream;
     if (m_filename.IsEmpty())
-        stream.Printf("%s/%s",m_directory.GetCString(),new_path);
+        stream.Printf("%s/%s", m_directory.GetCString(), new_path);
     else if (m_directory.IsEmpty())
-        stream.Printf("%s/%s",m_filename.GetCString(),new_path);
+        stream.Printf("%s/%s", m_filename.GetCString(), new_path);
     else
-        stream.Printf("%s/%s/%s",m_directory.GetCString(), m_filename.GetCString(),new_path);
+        stream.Printf("%s/%s/%s", m_directory.GetCString(), m_filename.GetCString(), new_path);
     SetFile(stream.GetData(), resolve);
 }
 
 void
 FileSpec::AppendPathComponent(const std::string &new_path)
 {
     return AppendPathComponent(new_path.c_str());
 }
 
 void
+FileSpec::AppendPathComponent(const FileSpec &new_path)
+{
+    return AppendPathComponent(new_path.GetPath(false));
+}
+
+void
 FileSpec::RemoveLastPathComponent ()
 {
     const bool resolve = false;
     if (m_filename.IsEmpty() && m_directory.IsEmpty())
     {
         SetFile("",resolve);
         return;
     }

lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFDebugLine.cpp

DWARFDebugLine::ParseSupportFiles (const lldb::ModuleSP &module_sp,
                                    const DWARFDataExtractor& debug_line_data,
                                    const char *cu_comp_dir,
                                    dw_offset_t stmt_list,
                                    FileSpecList &support_files)
 {
     lldb::offset_t offset = stmt_list;
     // Skip the total length
     (void)debug_line_data.GetDWARFInitialLength(&offset);
-    const char * s;
     uint32_t version = debug_line_data.GetU16(&offset);
     if (version < 2 || version > 4)
       return false;
 
     const dw_offset_t end_prologue_offset = debug_line_data.GetDWARFOffset(&offset) + offset;
     // Skip instruction length, default is stmt, line base, line range
     offset += 4;
     // For DWARF4, skip maximum operations per instruction
     if (version >= 4)
         offset += 1;
     // Skip opcode base, and all opcode lengths
     const uint8_t opcode_base = debug_line_data.GetU8(&offset);
     offset += opcode_base - 1;
-    std::vector<std::string> include_directories;
-    include_directories.push_back("");  // Directory at index zero doesn't exist
+    std::vector<FileSpec> include_directories{{}}; // Directory at index zero doesn't exist
     while (offset < end_prologue_offset)
     {
-        s = debug_line_data.GetCStr(&offset);
-        if (s && s[0])
-            include_directories.push_back(s);
+        FileSpec dir{debug_line_data.GetCStr(&offset), false};
+        if (dir)
+            include_directories.emplace_back(std::move(dir));
         else
             break;
     }
-    std::string fullpath;
-    std::string remapped_fullpath;
     while (offset < end_prologue_offset)
     {
-        const char* path = debug_line_data.GetCStr( &offset );
-        if (path && path[0])
+        FileSpec file_spec{debug_line_data.GetCStr(&offset), false};
+        if (file_spec)
         {
-            uint32_t dir_idx    = debug_line_data.GetULEB128( &offset );
+            uint32_t dir_idx = debug_line_data.GetULEB128(&offset);
             debug_line_data.Skip_LEB128(&offset); // Skip mod_time
             debug_line_data.Skip_LEB128(&offset); // Skip length
 
-            if (path[0] == '/')
-            {
-                // The path starts with a directory delimiter, so we are done.
-                if (module_sp->RemapSourceFile (path, fullpath))
-                    support_files.Append(FileSpec (fullpath.c_str(), false));
-                else
-                    support_files.Append(FileSpec (path, false));
-            }
-            else
-            {
-                if (dir_idx > 0 && dir_idx < include_directories.size())
-                {
-                    if (cu_comp_dir && include_directories[dir_idx][0] != '/')
-                    {
-                        fullpath = cu_comp_dir;
-
-                        if (*fullpath.rbegin() != '/')
-                            fullpath += '/';
-                        fullpath += include_directories[dir_idx];
-
-                    }
-                    else
-                        fullpath = include_directories[dir_idx];
-                }
-                else if (cu_comp_dir && cu_comp_dir[0])
-                {
-                    fullpath = cu_comp_dir;
-                }
-
-                if (!fullpath.empty())
+            if (file_spec.IsRelativeToCurrentWorkingDirectory())
+                if (0 < dir_idx && dir_idx < include_directories.size())
                 {
-                   if (*fullpath.rbegin() != '/')
-                        fullpath += '/';
+                    const FileSpec &dir = include_directories[dir_idx];
+                    file_spec.PrependPathComponent(dir);
                 }
-                fullpath += path;
-                if (module_sp->RemapSourceFile (fullpath.c_str(), remapped_fullpath))
-                    support_files.Append(FileSpec (remapped_fullpath.c_str(), false));
-                else
-                    support_files.Append(FileSpec (fullpath.c_str(), false));
+                if (file_spec.IsRelativeToCurrentWorkingDirectory())
+                    file_spec.PrependPathComponent(cu_comp_dir);
             }
-            
+            std::string remapped_file;
+            if (module_sp->RemapSourceFile(file_spec.GetCString(), remapped_file))
+                file_spec.SetFile(remapped_file, false);
+            support_files.Append(file_spec);
         }
     }
 
     if (offset != end_prologue_offset)
     {
         Host::SystemLog (Host::eSystemLogError, 
                          "warning: parsing line table prologue at 0x%8.8x should have ended at 0x%8.8x but it ended at 0x%8.8" PRIx64 "\n",
                          stmt_list, 

lldb/trunk/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp

if (dwarf_cu)
             else
             {
                 ModuleSP module_sp (m_obj_file->GetModule());
                 if (module_sp)
                 {
                     const DWARFDebugInfoEntry * cu_die = dwarf_cu->GetCompileUnitDIEOnly ();
                     if (cu_die)
                     {
-                        const char * cu_die_name = cu_die->GetName(this, dwarf_cu);
-                        const char * cu_comp_dir = cu_die->GetAttributeValueAsString(this, dwarf_cu, DW_AT_comp_dir, NULL);
-                        LanguageType cu_language = (LanguageType)cu_die->GetAttributeValueAsUnsigned(this, dwarf_cu, DW_AT_language, 0);
-                        if (cu_die_name)
-                        {
-                            std::string ramapped_file;
-                            FileSpec cu_file_spec;
-
-                            if (cu_die_name[0] == '/' || cu_comp_dir == NULL || cu_comp_dir[0] == '\0')
-                            {
-                                // If we have a full path to the compile unit, we don't need to resolve
-                                // the file.  This can be expensive e.g. when the source files are NFS mounted.
-                                if (module_sp->RemapSourceFile(cu_die_name, ramapped_file))
-                                    cu_file_spec.SetFile (ramapped_file.c_str(), false);
-                                else
-                                    cu_file_spec.SetFile (cu_die_name, false);
-                            }
-                            else
+                        FileSpec cu_file_spec{cu_die->GetName(this, dwarf_cu), false};
+                        if (cu_file_spec)
+                            if (cu_file_spec.IsRelativeToCurrentWorkingDirectory())
                             {
                                 // DWARF2/3 suggests the form hostname:pathname for compilation directory.
                                 // Remove the host part if present.
-                                cu_comp_dir = removeHostnameFromPathname(cu_comp_dir);
-                                std::string fullpath(cu_comp_dir);
-
-                                if (*fullpath.rbegin() != '/')
-                                    fullpath += '/';
-                                fullpath += cu_die_name;
-                                if (module_sp->RemapSourceFile (fullpath.c_str(), ramapped_file))
-                                    cu_file_spec.SetFile (ramapped_file.c_str(), false);
-                                else
-                                    cu_file_spec.SetFile (fullpath.c_str(), false);
+                                const char *cu_comp_dir{cu_die->GetAttributeValueAsString(this, dwarf_cu, DW_AT_comp_dir, nullptr)};
+                                cu_file_spec.PrependPathComponent(removeHostnameFromPathname(cu_comp_dir));
                             }
 
+                            std::string remapped_file;
+                            if (module_sp->RemapSourceFile(cu_file_spec.GetCString(), remapped_file))
+                                cu_file_spec.SetFile(remapped_file, false);
+
+                            LanguageType cu_language = (LanguageType)cu_die->GetAttributeValueAsUnsigned(this, dwarf_cu, DW_AT_language, 0);
+
                             cu_sp.reset(new CompileUnit (module_sp,
                                                          dwarf_cu,
                                                          cu_file_spec, 
                                                          MakeUserID(dwarf_cu->GetOffset()),
                                                          cu_language));
                             if (cu_sp)
                             {
                                 dwarf_cu->SetUserData(cu_sp.get());