bpo-33144: Fix choosing random.Random._randbelow implementation. (GH-6563)

random() takes precedence over getrandbits() if defined later
in the class tree.

Lib/random.py

@@ -102,18 +102,16 @@ class Random(_random.Random):
         ranges.
         """
 
-        if (cls.random is _random.Random.random) or (
+        for c in cls.__mro__:
-            cls.getrandbits is not _random.Random.getrandbits):
+            if '_randbelow' in c.__dict__:
-            # The original random() builtin method has not been overridden
+                # just inherit it
-            # or a new getrandbits() was supplied.
+                break
-            # The subclass can use the getrandbits-dependent implementation
+            if 'getrandbits' in c.__dict__:
-            # of _randbelow().
+                cls._randbelow = cls._randbelow_with_getrandbits
-            cls._randbelow = cls._randbelow_with_getrandbits
+                break
-        else:
+            if 'random' in c.__dict__:
-            # There's an overridden random() method but no new getrandbits(),
+                cls._randbelow = cls._randbelow_without_getrandbits
-            # so the subclass can only use the getrandbits-independent
+                break
-            # implementation of _randbelow().
-            cls._randbelow = cls._randbelow_without_getrandbits
 
     def seed(self, a=None, version=2):
         """Initialize internal state from hashable object.

Lib/test/test_random.py

@@ -5,7 +5,6 @@ import os
 import time
 import pickle
 import warnings
-import logging
 from functools import partial
 from math import log, exp, pi, fsum, sin, factorial
 from test import support
@@ -940,6 +939,7 @@ class TestDistributions(unittest.TestCase):
         gammavariate_mock.return_value = 0.0
         self.assertEqual(0.0, random.betavariate(2.71828, 3.14159))
 
+
 class TestRandomSubclassing(unittest.TestCase):
     def test_random_subclass_with_kwargs(self):
         # SF bug #1486663 -- this used to erroneously raise a TypeError
@@ -958,30 +958,80 @@ class TestRandomSubclassing(unittest.TestCase):
         # randrange
         class SubClass1(random.Random):
             def random(self):
-                return super().random()
+                called.add('SubClass1.random')
+                return random.Random.random(self)
 
             def getrandbits(self, n):
-                logging.getLogger('getrandbits').info('used getrandbits')
+                called.add('SubClass1.getrandbits')
-                return super().getrandbits(n)
+                return random.Random.getrandbits(self, n)
-        with self.assertLogs('getrandbits'):
+        called = set()
-            SubClass1().randrange(42)
+        SubClass1().randrange(42)
+        self.assertEqual(called, {'SubClass1.getrandbits'})
 
         # subclass providing only random => can only use random for randrange
         class SubClass2(random.Random):
             def random(self):
-                logging.getLogger('random').info('used random')
+                called.add('SubClass2.random')
-                return super().random()
+                return random.Random.random(self)
-        with self.assertLogs('random'):
+        called = set()
-            SubClass2().randrange(42)
+        SubClass2().randrange(42)
+        self.assertEqual(called, {'SubClass2.random'})
 
         # subclass defining getrandbits to complement its inherited random
         # => can now rely on getrandbits for randrange again
         class SubClass3(SubClass2):
             def getrandbits(self, n):
-                logging.getLogger('getrandbits').info('used getrandbits')
+                called.add('SubClass3.getrandbits')
-                return super().getrandbits(n)
+                return random.Random.getrandbits(self, n)
-        with self.assertLogs('getrandbits'):
+        called = set()
-            SubClass3().randrange(42)
+        SubClass3().randrange(42)
+        self.assertEqual(called, {'SubClass3.getrandbits'})
+
+        # subclass providing only random and inherited getrandbits
+        # => random takes precedence
+        class SubClass4(SubClass3):
+            def random(self):
+                called.add('SubClass4.random')
+                return random.Random.random(self)
+        called = set()
+        SubClass4().randrange(42)
+        self.assertEqual(called, {'SubClass4.random'})
+
+        # Following subclasses don't define random or getrandbits directly,
+        # but inherit them from classes which are not subclasses of Random
+        class Mixin1:
+            def random(self):
+                called.add('Mixin1.random')
+                return random.Random.random(self)
+        class Mixin2:
+            def getrandbits(self, n):
+                called.add('Mixin2.getrandbits')
+                return random.Random.getrandbits(self, n)
+
+        class SubClass5(Mixin1, random.Random):
+            pass
+        called = set()
+        SubClass5().randrange(42)
+        self.assertEqual(called, {'Mixin1.random'})
+
+        class SubClass6(Mixin2, random.Random):
+            pass
+        called = set()
+        SubClass6().randrange(42)
+        self.assertEqual(called, {'Mixin2.getrandbits'})
+
+        class SubClass7(Mixin1, Mixin2, random.Random):
+            pass
+        called = set()
+        SubClass7().randrange(42)
+        self.assertEqual(called, {'Mixin1.random'})
+
+        class SubClass8(Mixin2, Mixin1, random.Random):
+            pass
+        called = set()
+        SubClass8().randrange(42)
+        self.assertEqual(called, {'Mixin2.getrandbits'})
+
 
 class TestModule(unittest.TestCase):
     def testMagicConstants(self):