Bug 794180 - Rich comparison operators instead of __cmp__; r=gps
authorBenedict Singer <singerb.dev@gmail.com>
Fri, 30 Nov 2012 15:36:27 -0500
changeset 114656 6b997e5ca5ee304ad70d0c0c88ec3b982bbb88d8
parent 114655 6def19c0c235da1d9f695fdedcba6d48c1056179
child 114657 2880c2eca430349d80113e0c48c6c56cd577cc8d
push id23926
push userryanvm@gmail.com
push dateSat, 01 Dec 2012 15:27:30 +0000
treeherdermozilla-central@ecdf0e332f17 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersgps
bugs794180
milestone20.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 794180 - Rich comparison operators instead of __cmp__; r=gps DONTBUILD (NPOTB)
python/mozbuild/mozbuild/compilation/warnings.py
python/mozbuild/mozbuild/test/compilation/test_warnings.py
--- a/python/mozbuild/mozbuild/compilation/warnings.py
+++ b/python/mozbuild/mozbuild/compilation/warnings.py
@@ -51,43 +51,51 @@ class CompilerWarning(dict):
         dict.__init__(self)
 
         self['filename'] = None
         self['line'] = None
         self['column'] = None
         self['message'] = None
         self['flag'] = None
 
-    def __eq__(self, other):
-        if not isinstance(other, CompilerWarning):
-            return False
+    # Since we inherit from dict, functools.total_ordering gets confused.
+    # Thus, we define a key function, a generic comparison, and then
+    # implement all the rich operators with those; approach is from:
+    # http://regebro.wordpress.com/2010/12/13/python-implementing-rich-comparison-the-correct-way/
+    def _cmpkey(self):
+        return (self['filename'], self['line'], self['column'])
 
-        return self['filename'] == other['filename'] \
-            and self['line'] == other['line'] \
-            and self['column'] == other['column']
+    def _compare(self, other, func):
+        if not isinstance(other, CompilerWarning):
+            return NotImplemented
+
+        return func(self._cmpkey(), other._cmpkey())
+
+    def __eq__(self, other):
+        return self._compare(other, lambda s,o: s == o)
 
     def __neq__(self, other):
-        return not self.__eq__(other)
+        return self._compare(other, lambda s,o: s != o)
+
+    def __lt__(self, other):
+        return self._compare(other, lambda s,o: s < o)
+
+    def __le__(self, other):
+        return self._compare(other, lambda s,o: s <= o)
+
+    def __gt__(self, other):
+        return self._compare(other, lambda s,o: s > o)
+
+    def __ge__(self, other):
+        return self._compare(other, lambda s,o: s >= o)
 
     def __hash__(self):
         """Define so this can exist inside a set, etc."""
         return hash(tuple(sorted(self.items())))
 
-    def __cmp__(self, other):
-        if not isinstance(other, CompilerWarning):
-            return -1
-
-        for key in ('filename', 'line', 'column'):
-            x = cmp(self[key], other[key])
-
-            if x != 0:
-                return x
-
-        return 0
-
 
 class WarningsDatabase(object):
     """Holds a collection of warnings.
 
     The warnings database is a semi-intelligent container that holds warnings
     encountered during builds.
 
     The warnings database is backed by a JSON file. But, that is transparent
--- a/python/mozbuild/mozbuild/test/compilation/test_warnings.py
+++ b/python/mozbuild/mozbuild/test/compilation/test_warnings.py
@@ -71,16 +71,57 @@ class TestCompilerWarning(unittest.TestC
         w2['line'] = 5
 
         w2['filename'] = '/foo.c'
         w1['column'] = 3
         w2['column'] = 3
 
         self.assertEqual(w1, w2)
 
+    def test_comparison(self):
+        w1 = CompilerWarning()
+        w2 = CompilerWarning()
+
+        w1['filename'] = '/aaa.c'
+        w1['line'] = 5
+        w1['column'] = 5
+
+        w2['filename'] = '/bbb.c'
+        w2['line'] = 5
+        w2['column'] = 5
+
+        self.assertLess(w1, w2)
+        self.assertGreater(w2, w1)
+        self.assertGreaterEqual(w2, w1)
+
+        w2['filename'] = '/aaa.c'
+        w2['line'] = 4
+        w2['column'] = 6
+
+        self.assertLess(w2, w1)
+        self.assertGreater(w1, w2)
+        self.assertGreaterEqual(w1, w2)
+
+        w2['filename'] = '/aaa.c'
+        w2['line'] = 5
+        w2['column'] = 10
+
+        self.assertLess(w1, w2)
+        self.assertGreater(w2, w1)
+        self.assertGreaterEqual(w2, w1)
+
+        w2['filename'] = '/aaa.c'
+        w2['line'] = 5
+        w2['column'] = 5
+
+        self.assertLessEqual(w1, w2)
+        self.assertLessEqual(w2, w1)
+        self.assertGreaterEqual(w2, w1)
+        self.assertGreaterEqual(w1, w2)
+
 class TestWarningsParsing(unittest.TestCase):
     def test_clang_parsing(self):
         for source, filename, line, column, message, flag in CLANG_TESTS:
             collector = WarningsCollector(resolve_files=False)
             warning = collector.process_line(source)
 
             self.assertIsNotNone(warning)