labath marked 11 inline comments as done.
labath added inline comments.
================
Comment at: source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp:309
+ decl.SetFile(
+ die.GetDWARF()->GetFile(*die.GetCU(), form_value.Unsigned()));
break;
----------------
clayborg wrote:
> At the very least we should be asking the unit for the file? That is why I
> wanted to be able to ask the DWARFDIE for the file because it contains the
> right unit. If we just put the API on the unit, then we have the chance
> someone will use the wrong unit for the file. Also, as the DWARF gets fancier
> as time goes on (DWO, DWZ, etc), the unit might refer to another unit. But at
> the very least here I would feel better if we ask the DWARFUnit for the file.
> The GetDWARF() will ask the DWARFUnit in the DIE for the is DWARF file, then
> we will call the DWARF file class (SymbolFileDWARF or DWARFContext to get a
> file from the unit by passing a reference? Seems convoluted. Fine not adding
> the API as a FileSpec if we are trying to keep the API the same as LLVM.
I think asking the unit for the file is fine. I'm not sure why llvm does not do
that (it does the "get the context, pass the unit to get the line table, fetch
the file" dance), but it sounds like a utility function doing this could be
added there. Eventually our DWARFUnit will start using DWARFContext instead of
SymbolFileDWARF for the other operations, which will make the two approaches
pretty similar.
I've kept the function as returning FileSpecs. Returning any other type would
be a pessimization, as the values are already parsed as FileSpecs.
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
