clayborg added inline comments.
================
Comment at: source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp:769-774
+FileSpec::Style DWARFUnit::GetPathStyle() {
+ if (!m_comp_dir)
+ ComputeCompDirAndGuessPathStyle();
+ return m_comp_dir->GetPathStyle();
+}
+
----------------
Remove?
================
Comment at: source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp:777
+ if (!m_comp_dir)
+ ComputeCompDirAndGuessPathStyle();
+ return *m_comp_dir;
----------------
Just move contents of ComputeCompDirAndGuessPathStyle() to this function now
that we don't have GetPathStyle?
================
Comment at: source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp:818
+
+void DWARFUnit::ComputeCompDirAndGuessPathStyle() {
+ m_comp_dir = FileSpec();
----------------
Move contents to DWARFUnit::GetCompilationDirectory?
================
Comment at: source/Plugins/SymbolFile/DWARF/DWARFUnit.h:173
+ const lldb_private::FileSpec &GetCompilationDirectory();
+ lldb_private::FileSpec::Style GetPathStyle();
+
----------------
Is GetPathStyle() needed? Seems like we should be able to grab the style from:
```
auto style = unit->GetCompilationDirectory().GetPathStyle();
```
================
Comment at: source/Plugins/SymbolFile/DWARF/DWARFUnit.h:256
+ void ComputeCompDirAndGuessPathStyle();
+
----------------
If we remove GetPathStyle above, should this be removed and just put into the
code for GetCompilationDirectory() now that we don't have two accessors?
================
Comment at: source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp:757
if (cu_die) {
- FileSpec cu_file_spec(cu_die.GetName());
+ FileSpec cu_file_spec(cu_die.GetName(), dwarf_cu->GetPathStyle());
if (cu_file_spec) {
----------------
Not sure how we would resolve this one. What if dwarf_cu doesn't have a
DW_AT_comp_dir? Maybe we add a FileSpec constructor that takes a string and a
FileSpec &relative_dir so this would would just become:
```
FileSpec cu_fs(cu_die.GetName(), dwarf_cu->GetCompilationDirectory());
```
The proto would be something like:
```
explicit FileSpec::FileSpec(llvm::StringRef path, const FileSpec
&relative_root);
```
Then the if statement below goes away.
================
Comment at: source/Utility/FileSpec.cpp:560
+void FileSpec::MakeAbsolute(const FileSpec &dir) {
+ if (IsRelative())
----------------
Should this be "bool FileSpec::ResolveRelativePath(const FileSpec &dir)"?
Returns true if this path was relative and it was resolved, and false otherwise?
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D56543/new/
https://reviews.llvm.org/D56543
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
