labath added a comment.

Thank you for the review. I'll have the updated diff shortly. In the mean time, 
here are my responses.



================
Comment at: source/Plugins/SymbolFile/DWARF/DWARFIndex.cpp:40
+
+std::unique_ptr<AppleIndex> AppleIndex::Create(
+    Module &module, DWARFDataExtractor apple_names,
----------------
clayborg wrote:
> clayborg wrote:
> > Move all AppleIndex stuff to a dedicated .cpp file?
> Do we want all DWARFIndex::Create(...) signatures to take a SymbolFileDWARF 
> only? Then module can be extracted from that and all sections can be fetched 
> as well?
SymbolFileDWARF does not provide public accessors for individual sections. I 
would have to make `LoadSectionData` or some other get-me-a-section api 
available. I like how this is explicit about what kind of data a particular 
accelerator table depends on.


================
Comment at: source/Plugins/SymbolFile/DWARF/DWARFIndex.h:71
+
+class AppleIndex : public DWARFIndex {
+public:
----------------
clayborg wrote:
> Rename to AppleDWARFIndex and move to AppleDWARFIndex.h?
The classes seemed small enough to keep in one file, but that works for me too. 
A more canonical name here would be DWARFAppleIndex.h, but i hate how 
everything in this folder begins with DWARF, so this is a place I'll happily 
diverge from the norm. :P


https://reviews.llvm.org/D46889



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

Reply via email to