Make operator==s consistent between c++ and python APIs

[labath]

Summary

modify-python-lldb.py had code to insert python equality operators to
some classes. Some of those classes already had c++ equality operators,
and some didn't.

This makes the situation more consistent, by removing all equality
handilng from modify-python-lldb. Instead, I add c++ operators to
classes where they were missing, and expose them in the swig interface
files so that they are available to python too.

The only tricky case was the SBAddress class, which had an operator==
defined as a free function, which is not handled by swig. This function
cannot be removed without breaking ABI, and we cannot add an extra
operator== member, as that would make equality comparisons ambiguous.
For this class, I define a python eq function by hand and have it
delegate to the operator!=, which I have defined as a member function.

This isn't fully NFC, as the semantics of some equality functions in
python changes slightly, but I believe it changes for the better (e.g.,
previously SBBreakpoint.eq would consider two breakpoints with the
same ID as equal, even if they belonged to different targets; now they
are only equal if they belong to the same target).

[labath]

include/lldb/API/SBAddress.h

34–37 ↗

(On Diff #192271)

A different solution to this could be to still keep the free operator== defined, but do not expose it in the header. Then we could define a member operator==, without things being ambiguous. The people who build against old headers will still find the free operator== in the shared library, and people who use the new headers will start using the member operator==.

source/API/SBAddress.cpp

66–70 ↗

(On Diff #192271)

@JDevlieghere: you might be interested to know that this function is not intercepted by the api recorder. (This could be another argument for the "private free operator==" idea I mention above, as it would mean we don't need to add special code to intercept free functions.)

lldb/trunk/include/lldb/API/SBAddress.h

public:
   SBAddress(lldb::addr_t load_addr, lldb::SBTarget &target);
 
   ~SBAddress();
 
   const lldb::SBAddress &operator=(const lldb::SBAddress &rhs);
 
   explicit operator bool() const;
 
+  // operator== is a free function
+
+  bool operator!=(const SBAddress &rhs) const;
+
   bool IsValid() const;
 
   void Clear();
 
   addr_t GetFileAddress() const;
 
   addr_t GetLoadAddress(const lldb::SBTarget &target) const;
 

lldb/trunk/include/lldb/API/SBFileSpec.h

public:
   SBFileSpec(const char *path, bool resolve);
 
   ~SBFileSpec();
 
   const SBFileSpec &operator=(const lldb::SBFileSpec &rhs);
 
   explicit operator bool() const;
 
+  bool operator==(const SBFileSpec &rhs) const;
+
+  bool operator!=(const SBFileSpec &rhs) const;
+
   bool IsValid() const;
 
   bool Exists() const;
 
   bool ResolveExecutableLocation();
 
   const char *GetFilename() const;
 

lldb/trunk/include/lldb/API/SBWatchpoint.h

public:
   SBWatchpoint(const lldb::WatchpointSP &wp_sp);
 
   ~SBWatchpoint();
 
   const lldb::SBWatchpoint &operator=(const lldb::SBWatchpoint &rhs);
 
   explicit operator bool() const;
 
+  bool operator==(const SBWatchpoint &rhs) const;
+
+  bool operator!=(const SBWatchpoint &rhs) const;
+
   bool IsValid() const;
 
   SBError GetError();
 
   watch_id_t GetID();
 
   /// With -1 representing an invalid hardware index.
   int32_t GetHardwareIndex();

lldb/trunk/scripts/Python/modify-python-lldb.py

 watchpoint_iter = "    def watchpoint_iter(self): return lldb_iter(self, '%s', '%s')"
 section_iter = "    def section_iter(self): return lldb_iter(self, '%s', '%s')"
 compile_unit_iter = "    def compile_unit_iter(self): return lldb_iter(self, '%s', '%s')"
 
 # Called to implement the built-in function len().
 # Eligible objects are those containers with unambiguous iteration support.
 len_def = "    def __len__(self): return self.%s()"
 
-# This supports the rich comparison methods of __eq__ and __ne__.
-eq_def = "    def __eq__(self, other): return isinstance(other, %s) and %s"
-ne_def = "    def __ne__(self, other): return not self.__eq__(other)"
-
 # A convenience iterator for SBSymbol!
 symbol_in_section_iter_def = '''
     def symbol_in_section_iter(self, section):
         """Given a module and its contained section, returns an iterator on the
         symbols within the section."""
         for sym in self:
             if in_range(sym, section):
                 yield sym
d = {'SBBreakpoint': ('GetNumLocations', 'GetLocationAtIndex'),
      # SBModule has an additional section_iter(), see below.
      'SBModule-section': ('GetNumSections', 'GetSectionAtIndex'),
      # And compile_unit_iter().
      'SBModule-compile-unit': ('GetNumCompileUnits', 'GetCompileUnitAtIndex'),
      # As well as symbol_in_section_iter().
      'SBModule-symbol-in-section': symbol_in_section_iter_def
      }
 
-#
-# This dictionary defines a mapping from classname to equality method name(s).
-#
-e = {'SBAddress': ['GetFileAddress', 'GetModule'],
-     'SBBreakpoint': ['GetID'],
-     'SBWatchpoint': ['GetID'],
-     'SBFileSpec': ['GetFilename', 'GetDirectory'],
-     'SBModule': ['GetFileSpec', 'GetUUIDString'],
-     }
-
-
 def list_to_frag(list):
     """Transform a list to equality program fragment.
 
     For example, ['GetID'] is transformed to 'self.GetID() == other.GetID()',
     and ['GetFilename', 'GetDirectory'] to 'self.GetFilename() == other.GetFilename()
     and self.GetDirectory() == other.GetDirectory()'.
     """
     if not list:

 
 # The new content will have the iteration protocol defined for our lldb
 # objects.
 new_content = NewContent()
 
 with open(output_name, 'r') as f_in:
     content = f_in.read()
 
-# The pattern for recognizing the SWIG Version string
-version_pattern = re.compile("^# Version:? (.*)$")
-
 # The pattern for recognizing the beginning of an SB class definition.
 class_pattern = re.compile("^class (SB.*)\(_object\):$")
 
 # The pattern for recognizing the beginning of the __init__ method definition.
 init_pattern = re.compile("^    def __init__\(self.*\):")
 
-# The pattern for recognizing the beginning of the IsValid method definition.
-isvalid_pattern = re.compile("^    def IsValid\(")
-
 # These define the states of our finite state machine.
 NORMAL = 1
 DEFINING_ITERATOR = 2
-DEFINING_EQUALITY = 4
 CLEANUP_DOCSTRING = 8
 
 # Our FSM begins its life in the NORMAL state, and transitions to the
-# DEFINING_ITERATOR and/or DEFINING_EQUALITY state whenever it encounters the
-# beginning of certain class definitions, see dictionaries 'd' and 'e' above.
+# DEFINING_ITERATOR state whenever it encounters the beginning of certain class
+# definitions, see dictionary 'd' above.
 #
-# Note that the two states DEFINING_ITERATOR and DEFINING_EQUALITY are
-# orthogonal in that our FSM can be in one, the other, or both states at the
-# same time.  During such time, the FSM is eagerly searching for the __init__
+# In the DEFINING_ITERATOR state, the FSM is eagerly searching for the __init__
 # method definition in order to insert the appropriate method(s) into the lldb
 # module.
 #
 # The state CLEANUP_DOCSTRING can be entered from either the NORMAL or the
-# DEFINING_ITERATOR/EQUALITY states.  While in this state, the FSM is fixing/
-# cleaning the Python docstrings generated by the swig docstring features.
-#
-# The FSM, in all possible states, also checks the current input for IsValid()
-# definition, and inserts a __nonzero__() method definition to implement truth
-# value testing and the built-in operation bool().
+# DEFINING_ITERATOR state.  While in this state, the FSM is fixing/cleaning the
+# Python docstrings generated by the swig docstring features.
 state = NORMAL
 
 for line in content.splitlines():
     # Handle the state transition into CLEANUP_DOCSTRING state as it is possible
     # to enter this state from either NORMAL or DEFINING_ITERATOR/EQUALITY.
     #
     # If '        """' is the sole line, prepare to transition to the
     # CLEANUP_DOCSTRING state or out of it.
 
     if line == toggle_docstring_cleanup_line:
         if state & CLEANUP_DOCSTRING:
             # Special handling of the trailing blank line right before the '"""'
             # end docstring marker.
             new_content.del_blank_line()
             state ^= CLEANUP_DOCSTRING
         else:
             state |= CLEANUP_DOCSTRING
 
     if state == NORMAL:
         match = class_pattern.search(line)
         # If we are at the beginning of the class definitions, prepare to
-        # transition to the DEFINING_ITERATOR/DEFINING_EQUALITY state for the
-        # right class names.
+        # transition to the DEFINING_ITERATOR state for the right class names.
         if match:
             cls = match.group(1)
             if cls in d:
                 # Adding support for iteration for the matched SB class.
                 state |= DEFINING_ITERATOR
-            if cls in e:
-                # Adding support for eq and ne for the matched SB class.
-                state |= DEFINING_EQUALITY
 
-    if (state & DEFINING_ITERATOR) or (state & DEFINING_EQUALITY):
+    if state & DEFINING_ITERATOR:
         match = init_pattern.search(line)
         if match:
             # We found the beginning of the __init__ method definition.
-            # This is a good spot to insert the iter and/or eq-ne support.
+            # This is a good spot to insert the iter support.
             #
             # But note that SBTarget has three types of iterations.
             if cls == "SBTarget":
                 new_content.add_line(module_iter % (d[cls]['module']))
                 new_content.add_line(breakpoint_iter % (d[cls]['breakpoint']))
                 new_content.add_line(watchpoint_iter % (d[cls]['watchpoint']))
             else:
                 if (state & DEFINING_ITERATOR):
                     new_content.add_line(iter_def % d[cls])
                     new_content.add_line(len_def % d[cls][0])
-                if (state & DEFINING_EQUALITY):
-                    new_content.add_line(eq_def % (cls, list_to_frag(e[cls])))
-                    new_content.add_line(ne_def)
 
             # SBModule has extra SBSection, SBCompileUnit iterators and
             # symbol_in_section_iter()!
             if cls == "SBModule":
                 new_content.add_line(section_iter % d[cls + '-section'])
                 new_content.add_line(compile_unit_iter %
                                      d[cls + '-compile-unit'])
                 new_content.add_line(d[cls + '-symbol-in-section'])

lldb/trunk/scripts/interface/SBAddress.i

public:
 
     ~SBAddress ();
 
     bool
     IsValid () const;
 
     explicit operator bool() const;
 
+    // operator== is a free function, which swig does not handle, so we inject
+    // our own equality operator here
+    %pythoncode%{
+    def __eq__(self, other):
+      return not self.__ne__(other)
+    %}
+
+    bool operator!=(const SBAddress &rhs) const;
+
     void
     Clear ();
 
     addr_t
     GetFileAddress () const;
 
     addr_t
     GetLoadAddress (const lldb::SBTarget &target) const;

lldb/trunk/scripts/interface/SBBreakpoint.i

 public:
 
     SBBreakpoint ();
 
     SBBreakpoint (const lldb::SBBreakpoint& rhs);
 
     ~SBBreakpoint();
 
+    bool operator==(const lldb::SBBreakpoint &rhs);
+
+    bool operator!=(const lldb::SBBreakpoint &rhs);
+
     break_id_t
     GetID () const;
 
     bool
     IsValid() const;
 
     explicit operator bool() const;
 

lldb/trunk/scripts/interface/SBFileSpec.i

public:
     SBFileSpec (const lldb::SBFileSpec &rhs);
 
     SBFileSpec (const char *path);// Deprecated, use SBFileSpec (const char *path, bool resolve)
 
     SBFileSpec (const char *path, bool resolve);
 
     ~SBFileSpec ();
 
+    bool operator==(const SBFileSpec &rhs) const;
+
+    bool operator!=(const SBFileSpec &rhs) const;
+
     bool
     IsValid() const;
 
     explicit operator bool() const;
 
     bool
     Exists () const;
 

lldb/trunk/scripts/interface/SBModule.i

public:
     bool
     SetRemoteInstallFileSpec (lldb::SBFileSpec &file);
 
     %feature("docstring", "Returns the UUID of the module as a Python string."
     ) GetUUIDString;
     const char *
     GetUUIDString () const;
 
+    bool operator==(const lldb::SBModule &rhs) const;
+
+    bool operator!=(const lldb::SBModule &rhs) const;
+
     lldb::SBSection
     FindSection (const char *sect_name);
 
     lldb::SBAddress
     ResolveFileAddress (lldb::addr_t vm_addr);
 
     lldb::SBSymbolContext
     ResolveSymbolContextForAddress (const lldb::SBAddress& addr, 

lldb/trunk/scripts/interface/SBWatchpoint.i

public:
 
     ~SBWatchpoint ();
 
     bool
     IsValid();
 
     explicit operator bool() const;
 
+    bool operator==(const SBWatchpoint &rhs) const;
+
+    bool operator!=(const SBWatchpoint &rhs) const;
+
     SBError
     GetError();
 
     watch_id_t
     GetID ();
 
     %feature("docstring", "
     //------------------------------------------------------------------

lldb/trunk/scripts/lldb.swig

def lldb_iter(obj, getsize, getelem):
     size = getattr(obj, getsize)
     elem = getattr(obj, getelem)
     for i in range(size()):
         yield elem(i)
 
 # ==============================================================================
 # The modify-python-lldb.py script is responsible for post-processing this SWIG-
 # generated lldb.py module.  It is responsible for adding support for: iteration
-# protocol: __iter__, rich comparison methods: __eq__ and __ne__, and built-in
-# function len(): __len__.
+# protocol: __iter__, and built-in function len(): __len__.
 # ==============================================================================
 %}
 
 %include "./Python/python-typemaps.swig"
 
 /* C++ headers to be included. */
 %{
 #include <algorithm>

lldb/trunk/source/API/SBAddress.cpp

 }
 
 bool lldb::operator==(const SBAddress &lhs, const SBAddress &rhs) {
   if (lhs.IsValid() && rhs.IsValid())
     return lhs.ref() == rhs.ref();
   return false;
 }
 
+bool SBAddress::operator!=(const SBAddress &rhs) const {
+  LLDB_RECORD_METHOD_CONST(bool, SBAddress, operator!=,(const SBAddress &),
+                           &rhs);
+
+  return !(*this == rhs);
+}
+
 bool SBAddress::IsValid() const {
   LLDB_RECORD_METHOD_CONST_NO_ARGS(bool, SBAddress, IsValid);
   return this->operator bool();
 }
 SBAddress::operator bool() const {
   LLDB_RECORD_METHOD_CONST_NO_ARGS(bool, SBAddress, operator bool);
 
   return m_opaque_up != NULL && m_opaque_up->IsValid();

 template <>
 void RegisterMethods<SBAddress>(Registry &R) {
   LLDB_REGISTER_CONSTRUCTOR(SBAddress, ());
   LLDB_REGISTER_CONSTRUCTOR(SBAddress, (const lldb::SBAddress &));
   LLDB_REGISTER_CONSTRUCTOR(SBAddress, (lldb::SBSection, lldb::addr_t));
   LLDB_REGISTER_CONSTRUCTOR(SBAddress, (lldb::addr_t, lldb::SBTarget &));
   LLDB_REGISTER_METHOD(const lldb::SBAddress &,
                        SBAddress, operator=,(const lldb::SBAddress &));
+  LLDB_REGISTER_METHOD_CONST(bool,
+                             SBAddress, operator!=,(const lldb::SBAddress &));
   LLDB_REGISTER_METHOD_CONST(bool, SBAddress, IsValid, ());
   LLDB_REGISTER_METHOD_CONST(bool, SBAddress, operator bool, ());
   LLDB_REGISTER_METHOD(void, SBAddress, Clear, ());
   LLDB_REGISTER_METHOD(void, SBAddress, SetAddress,
                        (lldb::SBSection, lldb::addr_t));
   LLDB_REGISTER_METHOD_CONST(lldb::addr_t, SBAddress, GetFileAddress, ());
   LLDB_REGISTER_METHOD_CONST(lldb::addr_t, SBAddress, GetLoadAddress,
                              (const lldb::SBTarget &));

lldb/trunk/source/API/SBFileSpec.cpp

const SBFileSpec &SBFileSpec::operator=(const SBFileSpec &rhs) {
   LLDB_RECORD_METHOD(const lldb::SBFileSpec &,
                      SBFileSpec, operator=,(const lldb::SBFileSpec &), rhs);
 
   if (this != &rhs)
     m_opaque_up = clone(rhs.m_opaque_up);
   return *this;
 }
 
+bool SBFileSpec::operator==(const SBFileSpec &rhs) const {
+  LLDB_RECORD_METHOD_CONST(bool, SBFileSpec, operator==,(const SBFileSpec &rhs),
+                           rhs);
+
+  return ref() == rhs.ref();
+}
+
+bool SBFileSpec::operator!=(const SBFileSpec &rhs) const {
+  LLDB_RECORD_METHOD_CONST(bool, SBFileSpec, operator!=,(const SBFileSpec &rhs),
+                           rhs);
+
+  return !(*this == rhs);
+}
+
 bool SBFileSpec::IsValid() const {
   LLDB_RECORD_METHOD_CONST_NO_ARGS(bool, SBFileSpec, IsValid);
   return this->operator bool();
 }
 SBFileSpec::operator bool() const {
   LLDB_RECORD_METHOD_CONST_NO_ARGS(bool, SBFileSpec, operator bool);
 
   return m_opaque_up->operator bool();

 template <>
 void RegisterMethods<SBFileSpec>(Registry &R) {
   LLDB_REGISTER_CONSTRUCTOR(SBFileSpec, ());
   LLDB_REGISTER_CONSTRUCTOR(SBFileSpec, (const lldb::SBFileSpec &));
   LLDB_REGISTER_CONSTRUCTOR(SBFileSpec, (const char *));
   LLDB_REGISTER_CONSTRUCTOR(SBFileSpec, (const char *, bool));
   LLDB_REGISTER_METHOD(const lldb::SBFileSpec &,
                        SBFileSpec, operator=,(const lldb::SBFileSpec &));
+  LLDB_REGISTER_METHOD_CONST(bool,
+                             SBFileSpec, operator==,(const lldb::SBFileSpec &));
+  LLDB_REGISTER_METHOD_CONST(bool,
+                             SBFileSpec, operator!=,(const lldb::SBFileSpec &));
   LLDB_REGISTER_METHOD_CONST(bool, SBFileSpec, IsValid, ());
   LLDB_REGISTER_METHOD_CONST(bool, SBFileSpec, operator bool, ());
   LLDB_REGISTER_METHOD_CONST(bool, SBFileSpec, Exists, ());
   LLDB_REGISTER_METHOD(bool, SBFileSpec, ResolveExecutableLocation, ());
   LLDB_REGISTER_STATIC_METHOD(int, SBFileSpec, ResolvePath,
                               (const char *, char *, size_t));
   LLDB_REGISTER_METHOD_CONST(const char *, SBFileSpec, GetFilename, ());
   LLDB_REGISTER_METHOD_CONST(const char *, SBFileSpec, GetDirectory, ());

lldb/trunk/source/API/SBWatchpoint.cpp

bool SBWatchpoint::IsValid() const {
   return this->operator bool();
 }
 SBWatchpoint::operator bool() const {
   LLDB_RECORD_METHOD_CONST_NO_ARGS(bool, SBWatchpoint, operator bool);
 
   return bool(m_opaque_wp.lock());
 }
 
+bool SBWatchpoint::operator==(const SBWatchpoint &rhs) const {
+  LLDB_RECORD_METHOD_CONST(
+      bool, SBWatchpoint, operator==,(const SBWatchpoint &), rhs);
+
+  return GetSP() == rhs.GetSP();
+}
+
+bool SBWatchpoint::operator!=(const SBWatchpoint &rhs) const {
+  LLDB_RECORD_METHOD_CONST(
+      bool, SBWatchpoint, operator!=,(const SBWatchpoint &), rhs);
+
+  return !(*this == rhs);
+}
+
 SBError SBWatchpoint::GetError() {
   LLDB_RECORD_METHOD_NO_ARGS(lldb::SBError, SBWatchpoint, GetError);
 
   SBError sb_error;
   lldb::WatchpointSP watchpoint_sp(GetSP());
   if (watchpoint_sp) {
     sb_error.SetError(watchpoint_sp->GetError());
   }
void RegisterMethods<SBWatchpoint>(Registry &R) {
   LLDB_REGISTER_CONSTRUCTOR(SBWatchpoint, ());
   LLDB_REGISTER_CONSTRUCTOR(SBWatchpoint, (const lldb::WatchpointSP &));
   LLDB_REGISTER_CONSTRUCTOR(SBWatchpoint, (const lldb::SBWatchpoint &));
   LLDB_REGISTER_METHOD(const lldb::SBWatchpoint &,
                        SBWatchpoint, operator=,(const lldb::SBWatchpoint &));
   LLDB_REGISTER_METHOD(lldb::watch_id_t, SBWatchpoint, GetID, ());
   LLDB_REGISTER_METHOD_CONST(bool, SBWatchpoint, IsValid, ());
   LLDB_REGISTER_METHOD_CONST(bool, SBWatchpoint, operator bool, ());
+  LLDB_REGISTER_METHOD_CONST(
+      bool, SBWatchpoint, operator==,(const lldb::SBWatchpoint &));
+  LLDB_REGISTER_METHOD_CONST(
+      bool, SBWatchpoint, operator!=,(const lldb::SBWatchpoint &));
   LLDB_REGISTER_METHOD(lldb::SBError, SBWatchpoint, GetError, ());
   LLDB_REGISTER_METHOD(int32_t, SBWatchpoint, GetHardwareIndex, ());
   LLDB_REGISTER_METHOD(lldb::addr_t, SBWatchpoint, GetWatchAddress, ());
   LLDB_REGISTER_METHOD(size_t, SBWatchpoint, GetWatchSize, ());
   LLDB_REGISTER_METHOD(void, SBWatchpoint, SetEnabled, (bool));
   LLDB_REGISTER_METHOD(bool, SBWatchpoint, IsEnabled, ());
   LLDB_REGISTER_METHOD(uint32_t, SBWatchpoint, GetHitCount, ());
   LLDB_REGISTER_METHOD(uint32_t, SBWatchpoint, GetIgnoreCount, ());