[lldb] Renaissance LineTable sequences (llvm#127800)

LineSeqeunce is basically a vector, except that users aren't supposed to
know that. This implements the same concept in a slightly simpler
fashion.

Author: labath

lldb/include/lldb/Symbol/LineTable.h

@@ -20,25 +20,11 @@
 
 namespace lldb_private {
 
-/// \class LineSequence LineTable.h "lldb/Symbol/LineTable.h" An abstract base
-/// class used during symbol table creation.
-class LineSequence {
-public:
-  LineSequence();
-
-  virtual ~LineSequence() = default;
-
-  virtual void Clear() = 0;
-
-private:
-  LineSequence(const LineSequence &) = delete;
-  const LineSequence &operator=(const LineSequence &) = delete;
-};
-
 /// \class LineTable LineTable.h "lldb/Symbol/LineTable.h"
 /// A line table class.
 class LineTable {
 public:
+  class Sequence;
   /// Construct with compile unit.
   ///
   /// \param[in] comp_unit
@@ -49,8 +35,7 @@ class LineTable {
   ///
   /// \param[in] sequences
   ///     Unsorted list of line sequences.
-  LineTable(CompileUnit *comp_unit,
-            std::vector<std::unique_ptr<LineSequence>> &&sequences);
+  LineTable(CompileUnit *comp_unit, std::vector<Sequence> &&sequences);
 
   /// Destructor.
   ~LineTable();
@@ -73,20 +58,17 @@ class LineTable {
                        bool is_start_of_basic_block, bool is_prologue_end,
                        bool is_epilogue_begin, bool is_terminal_entry);
 
-  // Used to instantiate the LineSequence helper class
-  static std::unique_ptr<LineSequence> CreateLineSequenceContainer();
-
   // Append an entry to a caller-provided collection that will later be
   // inserted in this line table.
-  static void AppendLineEntryToSequence(LineSequence *sequence, lldb::addr_t file_addr,
-                                 uint32_t line, uint16_t column,
-                                 uint16_t file_idx, bool is_start_of_statement,
-                                 bool is_start_of_basic_block,
-                                 bool is_prologue_end, bool is_epilogue_begin,
-                                 bool is_terminal_entry);
+  static void
+  AppendLineEntryToSequence(Sequence &sequence, lldb::addr_t file_addr,
+                            uint32_t line, uint16_t column, uint16_t file_idx,
+                            bool is_start_of_statement,
+                            bool is_start_of_basic_block, bool is_prologue_end,
+                            bool is_epilogue_begin, bool is_terminal_entry);
 
   // Insert a sequence of entries into this line table.
-  void InsertSequence(LineSequence *sequence);
+  void InsertSequence(Sequence sequence);
 
   /// Dump all line entries in this line table to the stream \a s.
   ///
@@ -274,17 +256,6 @@ class LineTable {
       return 0;
     }
 
-    class LessThanBinaryPredicate {
-    public:
-      LessThanBinaryPredicate(LineTable *line_table);
-      bool operator()(const LineTable::Entry &, const LineTable::Entry &) const;
-      bool operator()(const std::unique_ptr<LineSequence> &,
-                      const std::unique_ptr<LineSequence> &) const;
-
-    protected:
-      LineTable *m_line_table;
-    };
-
     static bool EntryAddressLessThan(const Entry &lhs, const Entry &rhs) {
       return lhs.file_addr < rhs.file_addr;
     }
@@ -316,6 +287,35 @@ class LineTable {
     uint16_t file_idx = 0;
   };
 
+  class Sequence {
+  public:
+    Sequence() = default;
+    // Moving clears moved-from object so it can be used anew. Copying is
+    // generally an error. C++ doesn't guarantee that a moved-from vector is
+    // empty(), so we clear it explicitly.
+    Sequence(Sequence &&rhs) : m_entries(std::exchange(rhs.m_entries, {})) {}
+    Sequence &operator=(Sequence &&rhs) {
+      m_entries = std::exchange(rhs.m_entries, {});
+      return *this;
+    }
+    Sequence(const Sequence &) = delete;
+    Sequence &operator=(const Sequence &) = delete;
+
+  private:
+    std::vector<Entry> m_entries;
+    friend class LineTable;
+  };
+
+  class LessThanBinaryPredicate {
+  public:
+    LessThanBinaryPredicate(LineTable *line_table) : m_line_table(line_table) {}
+    bool operator()(const LineTable::Entry &, const LineTable::Entry &) const;
+    bool operator()(const Sequence &, const Sequence &) const;
+
+  protected:
+    LineTable *m_line_table;
+  };
+
 protected:
   struct EntrySearchInfo {
     LineTable *line_table;
@@ -334,19 +334,6 @@ class LineTable {
   entry_collection
       m_entries; ///< The collection of line entries in this line table.
 
-  // Helper class
-  class LineSequenceImpl : public LineSequence {
-  public:
-    LineSequenceImpl() = default;
-
-    ~LineSequenceImpl() override = default;
-
-    void Clear() override;
-
-    entry_collection
-        m_entries; ///< The collection of line entries in this sequence.
-  };
-
   bool ConvertEntryAtIndexToLineEntry(uint32_t idx, LineEntry &line_entry);
 
 private:

lldb/source/Plugins/SymbolFile/Breakpad/SymbolFileBreakpad.cpp

@@ -837,18 +837,16 @@ void SymbolFileBreakpad::ParseLineTableAndSupportFiles(CompileUnit &cu,
          "How did we create compile units without a base address?");
 
   SupportFileMap map;
-  std::vector<std::unique_ptr<LineSequence>> sequences;
-  std::unique_ptr<LineSequence> line_seq_up =
-      LineTable::CreateLineSequenceContainer();
+  std::vector<LineTable::Sequence> sequences;
+  LineTable::Sequence sequence;
   std::optional<addr_t> next_addr;
   auto finish_sequence = [&]() {
     LineTable::AppendLineEntryToSequence(
-        line_seq_up.get(), *next_addr, /*line=*/0, /*column=*/0,
+        sequence, *next_addr, /*line=*/0, /*column=*/0,
         /*file_idx=*/0, /*is_start_of_statement=*/false,
         /*is_start_of_basic_block=*/false, /*is_prologue_end=*/false,
         /*is_epilogue_begin=*/false, /*is_terminal_entry=*/true);
-    sequences.push_back(std::move(line_seq_up));
-    line_seq_up = LineTable::CreateLineSequenceContainer();
+    sequences.push_back(std::move(sequence));
   };
 
   LineIterator It(*m_objfile_sp, Record::Func, data.bookmark),
@@ -870,7 +868,7 @@ void SymbolFileBreakpad::ParseLineTableAndSupportFiles(CompileUnit &cu,
       finish_sequence();
     }
     LineTable::AppendLineEntryToSequence(
-        line_seq_up.get(), record->Address, record->LineNum, /*column=*/0,
+        sequence, record->Address, record->LineNum, /*column=*/0,
         map[record->FileNum], /*is_start_of_statement=*/true,
         /*is_start_of_basic_block=*/false, /*is_prologue_end=*/false,
         /*is_epilogue_begin=*/false, /*is_terminal_entry=*/false);

lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp

@@ -1232,7 +1232,7 @@ bool SymbolFileDWARF::ParseLineTable(CompileUnit &comp_unit) {
   // FIXME: Rather than parsing the whole line table and then copying it over
   // into LLDB, we should explore using a callback to populate the line table
   // while we parse to reduce memory usage.
-  std::vector<std::unique_ptr<LineSequence>> sequences;
+  std::vector<LineTable::Sequence> sequences;
   // The Sequences view contains only valid line sequences. Don't iterate over
   // the Rows directly.
   for (const llvm::DWARFDebugLine::Sequence &seq : line_table->Sequences) {
@@ -1242,12 +1242,11 @@ bool SymbolFileDWARF::ParseLineTable(CompileUnit &comp_unit) {
     // m_first_code_address declaration for more details on this.
     if (seq.LowPC < m_first_code_address)
       continue;
-    std::unique_ptr<LineSequence> sequence =
-        LineTable::CreateLineSequenceContainer();
+    LineTable::Sequence sequence;
     for (unsigned idx = seq.FirstRowIndex; idx < seq.LastRowIndex; ++idx) {
       const llvm::DWARFDebugLine::Row &row = line_table->Rows[idx];
       LineTable::AppendLineEntryToSequence(
-          sequence.get(), row.Address.Address, row.Line, row.Column, row.File,
+          sequence, row.Address.Address, row.Line, row.Column, row.File,
           row.IsStmt, row.BasicBlock, row.PrologueEnd, row.EpilogueBegin,
           row.EndSequence);
     }

lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp

@@ -1310,18 +1310,17 @@ bool SymbolFileNativePDB::ParseLineTable(CompileUnit &comp_unit) {
   cii->m_global_line_table.Clear();
 
   // Add line entries in line_set to line_table.
-  auto line_table = std::make_unique<LineTable>(&comp_unit);
-  std::unique_ptr<LineSequence> sequence(
-      line_table->CreateLineSequenceContainer());
+  std::vector<LineTable::Sequence> sequence(1);
   for (const auto &line_entry : line_set) {
-    line_table->AppendLineEntryToSequence(
-        sequence.get(), line_entry.file_addr, line_entry.line,
+    LineTable::AppendLineEntryToSequence(
+        sequence.back(), line_entry.file_addr, line_entry.line,
         line_entry.column, line_entry.file_idx,
         line_entry.is_start_of_statement, line_entry.is_start_of_basic_block,
         line_entry.is_prologue_end, line_entry.is_epilogue_begin,
         line_entry.is_terminal_entry);
   }
-  line_table->InsertSequence(sequence.get());
+  auto line_table =
+      std::make_unique<LineTable>(&comp_unit, std::move(sequence));
 
   if (line_table->GetSize() == 0)
     return false;

lldb/source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp

@@ -1761,11 +1761,10 @@ bool SymbolFilePDB::ParseCompileUnitLineTable(CompileUnit &comp_unit,
   if (!files)
     return false;
 
-  // For each source and header file, create a LineSequence for contributions
-  // to the compiland from that file, and add the sequence.
+  // For each source and header file, create a LineTable::Sequence for
+  // contributions to the compiland from that file, and add the sequence.
   while (auto file = files->getNext()) {
-    std::unique_ptr<LineSequence> sequence(
-        line_table->CreateLineSequenceContainer());
+    LineTable::Sequence sequence;
     auto lines = m_session_up->findLineNumbers(*compiland_up, *file);
     if (!lines)
       continue;
@@ -1794,12 +1793,11 @@ bool SymbolFilePDB::ParseCompileUnitLineTable(CompileUnit &comp_unit,
       // of the previous entry's address range if the current entry resulted in
       // a gap from the previous entry.
       if (is_gap && ShouldAddLine(match_line, prev_line, prev_length)) {
-        line_table->AppendLineEntryToSequence(
-            sequence.get(), prev_addr + prev_length, prev_line, 0,
-            prev_source_idx, false, false, false, false, true);
+        line_table->AppendLineEntryToSequence(sequence, prev_addr + prev_length,
+                                              prev_line, 0, prev_source_idx,
+                                              false, false, false, false, true);
 
-        line_table->InsertSequence(sequence.get());
-        sequence = line_table->CreateLineSequenceContainer();
+        line_table->InsertSequence(std::move(sequence));
       }
 
       if (ShouldAddLine(match_line, lno, length)) {
@@ -1818,7 +1816,7 @@ bool SymbolFilePDB::ParseCompileUnitLineTable(CompileUnit &comp_unit,
             is_epilogue = (addr == epilogue->getVirtualAddress());
         }
 
-        line_table->AppendLineEntryToSequence(sequence.get(), addr, lno, col,
+        line_table->AppendLineEntryToSequence(sequence, addr, lno, col,
                                               source_idx, is_statement, false,
                                               is_prologue, is_epilogue, false);
       }
@@ -1831,12 +1829,12 @@ bool SymbolFilePDB::ParseCompileUnitLineTable(CompileUnit &comp_unit,
 
     if (entry_count > 0 && ShouldAddLine(match_line, prev_line, prev_length)) {
       // The end is always a terminal entry, so insert it regardless.
-      line_table->AppendLineEntryToSequence(
-          sequence.get(), prev_addr + prev_length, prev_line, 0,
-          prev_source_idx, false, false, false, false, true);
+      line_table->AppendLineEntryToSequence(sequence, prev_addr + prev_length,
+                                            prev_line, 0, prev_source_idx,
+                                            false, false, false, false, true);
     }
 
-    line_table->InsertSequence(sequence.get());
+    line_table->InsertSequence(std::move(sequence));
   }
 
   if (line_table->GetSize()) {