[Lldb-commits] [lldb] Add AddressRange to SB API (PR #92014)

[Alex Langford via lldb-commits]

================
@@ -0,0 +1,58 @@
+//===-- SBAddressRangeList.h ------------------------------------*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLDB_API_SBADDRESSRANGELIST_H
+#define LLDB_API_SBADDRESSRANGELIST_H
+
+#include <memory>
+
+#include "lldb/API/SBDefines.h"
+
+class AddressRangeListImpl;
+
+namespace lldb {
+
+class LLDB_API SBAddressRangeList {
+public:
+  SBAddressRangeList();
+
+  SBAddressRangeList(const lldb::SBAddressRangeList &rhs);
+
+  ~SBAddressRangeList();
+
+  const lldb::SBAddressRangeList &
+  operator=(const lldb::SBAddressRangeList &rhs);
+
+  uint32_t GetSize() const;
+
+  void Clear();
+
+  bool GetAddressRangeAtIndex(uint64_t idx, SBAddressRange &addr_range);
+
+  void Append(const lldb::SBAddressRange &addr_range);
+
+  void Append(const lldb::SBAddressRangeList &addr_range_list);
+
+protected:
+  const AddressRangeListImpl *operator->() const;
+
+  const AddressRangeListImpl &operator*() const;
+
+private:
+  friend class SBProcess;
+
+  lldb_private::AddressRanges &ref();
+
+  const lldb_private::AddressRanges &ref() const;
----------------
bulbazord wrote:

To be clear, I'm objecting to these methods existing at all. I re-read the code and my comment and realized my argument didn't actually make a ton of sense, so allow me to clarify:
I don't think we should be exposing `ref` at all here. I can understand having private methods to access the underlying `AddressRangeListImpl` (so that other classes can access it and mess with it, we do this all the time). What I don't think is a good idea is having a method to access implementation details of `AddressRangeListImpl` (namely, `AddressRanges`).

Instead, I'm suggesting that we shuffle some code around to avoid exposing these at all. The problem that these solve is that other classes may want to privately modify the underlying `AddressRangeListImpl`, but don't have access to the class. All they have is a forward declaration (because they include this header and the implementation is in the cpp file). My suggestion would be to create a private header `AddressRangeListImpl.h` in `source/API`. Then any class that wants to modify the underlying `AddressRangeListImpl` can do so without needing to know that `AddressRangeListImpl` is implemented with `AddressRanges`.

Normally I try not to push back against things here, but you cannot change or remove things from the SBAPI ever. Not even private methods like these (as I found out when I broke the ABI on Windows last year).

https://github.com/llvm/llvm-project/pull/92014