[Lldb-commits] [PATCH] D72597: [lldb][DWARF] Added support for new forms in DWARFv5 macro.

[Sourabh Singh Tomar via Phabricator via lldb-commits]

SouraVX marked 4 inline comments as done.
SouraVX added inline comments.


================
Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugMacro.cpp:62
+    const DWARFDataExtractor &debug_str_offset_data,
     const DWARFDataExtractor &debug_str_data, const bool offset_is_64_bit,
     lldb::offset_t *offset, SymbolFileDWARF *sym_file_dwarf,
----------------
clayborg wrote:
> We could also remove "offset_is_64_bit" if we fix DataExtractor to handle it as suggested below where "offset_is_64_bit" is used.
Agreed, however I think this enhancement has to be done separately.


================
Comment at: lldb/test/Shell/Commands/dwarf5-macro.test:1
+# REQUIRES: x86
+# This test checks lldb macro expansion when macro section
----------------
aprantl wrote:
> shafik wrote:
> > Is there a reason why we would only want to test this feature on x86?
> Presumably because the input is X86 assembly. That said, would it be possible to test the feature without constructing a process, so we *can* test it everywhere?
I tried, other ways also. Seems like this is bare minimum, We need to run the program to be able to expand macros. analogous behavior with GDB also.
If you've some better way in mind to test this. or where to put this. Please share.


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

https://reviews.llvm.org/D72597