labath marked 5 inline comments as done.
labath added inline comments.

================
Comment at: source/Plugins/SymbolFile/DWARF/DWARFUnit.h:200
   const lldb_private::FileSpec &GetCompilationDirectory();
+  const lldb_private::FileSpec &GetFileSpec();
   lldb_private::FileSpec::Style GetPathStyle();
----------------
JDevlieghere wrote:
> Given the implementation, how about `GetAbsolutePath`? I think the "FileSpec" 
> of a DWARF unit is a little weird, but feel free to ignore this. 
GetAbsolutePath sounds good.


================
Comment at: source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp:821
+  if (iter_bool.second) {
+    list.Append(FileSpec());
+    DWARFDebugLine::ParseSupportFiles(GetObjectFile()->GetModule(),
----------------
JDevlieghere wrote:
> Not your responsibility but we should really get rid of this thing as it 
> doesn't make sense anymore in > DWARF5.
I'm waiting on your llvm debug_lines patch. :)


================
Comment at: source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h:303
 
+  lldb_private::FileSpec GetSupportFile(DWARFUnit &unit, size_t file_idx);
+
----------------
JDevlieghere wrote:
> clayborg wrote:
> > Can probably just be named GetFile
> Should this return an optional?
I don't know. On one hand, both the level above this function and the level 
below use an empty FileSpec do denote failure, so I'd have to convert it 
explicitly in both places. OTOH, Optional is the direction we want to move in.

I can change it if you want..


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D62894/new/

https://reviews.llvm.org/D62894



_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to