[lldb] [DynamicRegisterInfo] Remove AddRegister() and make Finalize() protected

[mgorny]

Summary

(note that this assumes parent diffs are merged)

Now that AddRegister() is no longer used, remove it. While at it,
we can also make Finalize() protected as all supported API methods
call it internally.

[labath]

LGTM (assuming parent diffs are merged) :P

lldb/include/lldb/Target/DynamicRegisterInfo.h

public:
   DynamicRegisterInfo &operator=(DynamicRegisterInfo &&info);
 
   size_t SetRegisterInfo(const lldb_private::StructuredData::Dictionary &dict,
                          const lldb_private::ArchSpec &arch);
 
   size_t SetRegisterInfo(std::vector<Register> &&regs,
                          const lldb_private::ArchSpec &arch);
 
-  void AddRegister(lldb_private::RegisterInfo reg_info,
-                   lldb_private::ConstString &set_name);
-
-  void Finalize(const lldb_private::ArchSpec &arch);
-
   size_t GetNumRegisters() const;
 
   size_t GetNumRegisterSets() const;
 
   size_t GetRegisterDataByteSize() const;
 
   const lldb_private::RegisterInfo *GetRegisterInfoAtIndex(uint32_t i) const;
 
llvm::Expected<uint32_t> ByteOffsetFromComposite(
       uint32_t index, lldb_private::StructuredData::Array &composite_reg_list,
       lldb::ByteOrder byte_order);
   llvm::Expected<uint32_t> ByteOffsetFromRegInfoDict(
       uint32_t index, lldb_private::StructuredData::Dictionary &reg_info_dict,
       lldb::ByteOrder byte_order);
 
   void MoveFrom(DynamicRegisterInfo &&info);
 
+  void Finalize(const lldb_private::ArchSpec &arch);
+
   void ConfigureOffsets();
 
   reg_collection m_regs;
   set_collection m_sets;
   set_reg_num_collection m_set_reg_nums;
   name_collection m_set_names;
   reg_to_regs_map m_value_regs_map;
   reg_to_regs_map m_invalidate_regs_map;

lldb/source/Target/DynamicRegisterInfo.cpp

for (auto it : llvm::enumerate(regs)) {
     assert(set < m_set_names.size());
     m_set_reg_nums[set].push_back(local_regnum);
   };
 
   Finalize(arch);
   return m_regs.size();
 }
 
-void DynamicRegisterInfo::AddRegister(RegisterInfo reg_info,
-                                      ConstString &set_name) {
-  assert(!m_finalized);
-  const uint32_t reg_num = m_regs.size();
-  assert(reg_info.name);
-  uint32_t i;
-  if (reg_info.value_regs) {
-    for (i = 0; reg_info.value_regs[i] != LLDB_INVALID_REGNUM; ++i)
-      m_value_regs_map[reg_num].push_back(reg_info.value_regs[i]);
-
-    // invalidate until Finalize() is called
-    reg_info.value_regs = nullptr;
-  }
-  if (reg_info.invalidate_regs) {
-    for (i = 0; reg_info.invalidate_regs[i] != LLDB_INVALID_REGNUM; ++i)
-      m_invalidate_regs_map[reg_num].push_back(reg_info.invalidate_regs[i]);
-
-    // invalidate until Finalize() is called
-    reg_info.invalidate_regs = nullptr;
-  }
-
-  m_regs.push_back(reg_info);
-  uint32_t set = GetRegisterSetIndexByName(set_name, true);
-  assert(set < m_sets.size());
-  assert(set < m_set_reg_nums.size());
-  assert(set < m_set_names.size());
-  m_set_reg_nums[set].push_back(reg_num);
-}
-
 void DynamicRegisterInfo::Finalize(const ArchSpec &arch) {
   if (m_finalized)
     return;
 
   m_finalized = true;
   const size_t num_sets = m_sets.size();
   for (size_t set = 0; set < num_sets; ++set) {
     assert(m_sets.size() == m_set_reg_nums.size());

lldb/unittests/Target/DynamicRegisterInfoTest.cpp

static std::vector<uint32_t> regs_to_vector(uint32_t *regs) {
   std::vector<uint32_t> ret;
   if (regs) {
     while (*regs != LLDB_INVALID_REGNUM)
       ret.push_back(*regs++);
   }
   return ret;
 }
 
-class DynamicRegisterInfoTest : public ::testing::Test {
-protected:
-  DynamicRegisterInfo info;
-  uint32_t next_regnum = 0;
-  ConstString group{"group"};
-
-  uint32_t AddTestRegister(const char *name, uint32_t byte_size,
-                           std::vector<uint32_t> value_regs = {},
-                           std::vector<uint32_t> invalidate_regs = {}) {
-    struct RegisterInfo new_reg {
-      name, nullptr, byte_size, LLDB_INVALID_INDEX32, lldb::eEncodingUint,
-          lldb::eFormatUnsigned,
-          {LLDB_INVALID_REGNUM, LLDB_INVALID_REGNUM, LLDB_INVALID_REGNUM,
-           next_regnum, next_regnum},
-          nullptr, nullptr
-    };
-
-    if (!value_regs.empty()) {
-      value_regs.push_back(LLDB_INVALID_REGNUM);
-      new_reg.value_regs = value_regs.data();
-    }
-    if (!invalidate_regs.empty()) {
-      invalidate_regs.push_back(LLDB_INVALID_REGNUM);
-      new_reg.invalidate_regs = invalidate_regs.data();
-    }
-
-    info.AddRegister(new_reg, group);
-    return next_regnum++;
-  }
-
-  void AssertRegisterInfo(uint32_t reg_num, const char *reg_name,
-                          uint32_t byte_offset,
-                          std::vector<uint32_t> value_regs = {},
-                          std::vector<uint32_t> invalidate_regs = {}) {
-    const RegisterInfo *reg = info.GetRegisterInfoAtIndex(reg_num);
-    EXPECT_NE(reg, nullptr);
-    if (!reg)
-      return;
-
-    EXPECT_STREQ(reg->name, reg_name);
-    EXPECT_EQ(reg->byte_offset, byte_offset);
-    EXPECT_THAT(regs_to_vector(reg->value_regs), value_regs);
-    EXPECT_THAT(regs_to_vector(reg->invalidate_regs), invalidate_regs);
-  }
-};
-
-#define ASSERT_REG(reg, ...)                                                   \
-  {                                                                            \
-    SCOPED_TRACE("at register " #reg);                                         \
-    AssertRegisterInfo(reg, #reg, __VA_ARGS__);                                \
-  }
-
-TEST_F(DynamicRegisterInfoTest, finalize_regs) {
-  // Add regular registers
-  uint32_t b1 = AddTestRegister("b1", 8);
-  uint32_t b2 = AddTestRegister("b2", 8);
-
-  // Add a few sub-registers
-  uint32_t s1 = AddTestRegister("s1", 4, {b1});
-  uint32_t s2 = AddTestRegister("s2", 4, {b2});
-
-  // Add a register with invalidate_regs
-  uint32_t i1 = AddTestRegister("i1", 8, {}, {b1});
-
-  // Add a register with indirect invalidate regs to be expanded
-  // TODO: why is it done conditionally to value_regs?
-  uint32_t i2 = AddTestRegister("i2", 4, {b2}, {i1});
-
-  info.Finalize(lldb_private::ArchSpec());
-
-  ASSERT_REG(b1, 0);
-  ASSERT_REG(b2, 8);
-  ASSERT_REG(s1, 0, {b1});
-  ASSERT_REG(s2, 8, {b2});
-  ASSERT_REG(i1, 16, {}, {b1});
-  ASSERT_REG(i2, 8, {b2}, {b1, i1});
-}
-
-TEST_F(DynamicRegisterInfoTest, no_finalize_regs) {
-  // Add regular registers
-  uint32_t b1 = AddTestRegister("b1", 8);
-  uint32_t b2 = AddTestRegister("b2", 8);
-
-  // Add a few sub-registers
-  uint32_t s1 = AddTestRegister("s1", 4, {b1});
-  uint32_t s2 = AddTestRegister("s2", 4, {b2});
-
-  // Add a register with invalidate_regs
-  uint32_t i1 = AddTestRegister("i1", 8, {}, {b1});
-
-  // Add a register with indirect invalidate regs to be expanded
-  // TODO: why is it done conditionally to value_regs?
-  uint32_t i2 = AddTestRegister("i2", 4, {b2}, {i1});
-
-  ASSERT_REG(b1, LLDB_INVALID_INDEX32);
-  ASSERT_REG(b2, LLDB_INVALID_INDEX32);
-  ASSERT_REG(s1, LLDB_INVALID_INDEX32);
-  ASSERT_REG(s2, LLDB_INVALID_INDEX32);
-  ASSERT_REG(i1, LLDB_INVALID_INDEX32);
-  ASSERT_REG(i2, LLDB_INVALID_INDEX32);
-}
-
 class DynamicRegisterInfoRegisterTest : public ::testing::Test {
 protected:
   std::vector<DynamicRegisterInfo::Register> m_regs;
   DynamicRegisterInfo m_dyninfo;
 
   uint32_t AddTestRegister(
       const char *name, const char *group, uint32_t byte_size,
       std::function<void(const DynamicRegisterInfo::Register &)> adder,