[Lldb-commits] [PATCH] D15437: Read macro info from .debug_macro section and use it for expression evaluation.

[Tamas Berghammer via lldb-commits]

tberghammer added inline comments.

================
Comment at: source/Plugins/SymbolFile/DWARF/DWARFDebugMacro.cpp:38-41
@@ +37,6 @@
+
+    if (flags & OPCODE_OPERANDS_TABLE_MASK)
+    {
+        OperandTable::ReadOperandTable(debug_macro_data, offset, &header.m_operand_table);
+    }
+
----------------
If we can get rid of all OperandTable related data then this can become a call to the SkipOperandTable function. (What is the purpose of the operand table if you can process everything without using it?).

================
Comment at: source/Plugins/SymbolFile/DWARF/DWARFDebugMacro.h:49
@@ +48,3 @@
+        uint8_t m_entry_count;
+        std::map<uint8_t, OperandTableEntry> m_entry_map; // Mapping from opcode number to its entry.
+
----------------
Constructing it is quite expensive as we insert a new entry for each operand into a std::map what will involve a binary search and most likely a heap allocation. As we don't use it for anything I would prefer to remove it because we don't want to pay the computation cost required to create it for nothing. We can add it back when we have a use case for it.

Going forward in this route I don't see any code reading DWARFDebugMacroHeader::m_operand_table either. I might miss something but if nobody is reading it then we can remove DWARFDebugMacroHeader::m_operand_table, DWARFDebugMacroHeader::OperandTable, DWARFDebugMacroHeader::OperandTableEntry and the call to DWARFDebugMacroHeader::OperandTable::ReadOperandTable what would simplify the code and also improve the efficiency as we don't do a lot of useless work.

================
Comment at: source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h:563
@@ +562,3 @@
+    typedef std::map<lldb::offset_t, lldb_private::DebugMacrosSP> DebugMacrosMap;
+    DebugMacrosMap m_debug_macros_map;
+
----------------
Should we use std::map or std::unordered_map here?


http://reviews.llvm.org/D15437