gh-85294: Handle missing arguments to @singledispatchmethod gracefully

[ammaraskar]

This prevents a confusing error message when you fail to pass an argument. The code mirrors the check already present in singledispatch.

Before

>>> import functools
>>> class A:
...   @functools.singledispatchmethod
...   def t(self):
...     pass
...   @t.register
...   def _(self, arg: int): return "int"
...
>>> A().t()
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "cpython\lib\functools.py", line 914, in _method
    method = self.dispatcher.dispatch(args[0].__class__)
IndexError: tuple index out of range

After

>>> import functools
>>> class A:
...   @functools.singledispatchmethod
...   def t(self):
...     pass
...   @t.register
...   def _(self, arg: int):
...     return "int"
...
>>> A().t()
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "cpython\lib\functools.py", line 916, in _method
    raise TypeError(f'{self.dispatcher._funcname} requires at least '
TypeError: t requires at least 1 positional argument

https://bugs.python.org/issue41122

• Issue: functools.singledispatchfunction has confusing error message if no positional arguments are passed in #85294

[nagarajan]

I am not the assigned reviewer, but was just looking through open PRs (so my review has little weight)

Your changes look good!! 👍

Just added one minor comment for changing the test method name.

[nagarajan]

Looks good!

[changeling]

Any word on this? I've adapted to recognizing the error pattern, but this fix would have saved me some time, and I imagine it's costing time for others.

[mental32]

I accidentally opened a duplicate PR for this since I ran into it today and it wasn't pleasant to figure out, +1.

[rsdenijs]

It seems this PR has been forgotten. I ran into this recently and also took me some time to figure out. It would be nice to merge it.

[AlexWaygood]

I haven't done benchmarks, but is there a risk that this could add unnecessary overhead to the happy path? What if we did something like this @serhiy-storchaka?

Suggested change

	if not args:
	raise TypeError(f'{funcname} requires at least '
	'1 positional argument')
	return dispatch(args[0].__class__).__get__(obj, cls)(*args, **kwargs)
	try:
	key = args[0].__class__
	except IndexError:
	raise TypeError(f'{funcname} requires at least '
	'1 positional argument') from None
	return dispatch(key).__get__(obj, cls)(*args, **kwargs)

[serhiy-storchaka]

It looks reasonable, but could you please benchmark this? Your code is not zero cost either, it adds STORE_FAST+LOAD_FAST instead of LOAD_FAST+TO_BOOL+POP_JUMP_IF_TRUE.

If there is a difference, we perhaps should apply the same optimization in singledispatch().

[AlexWaygood]

Yeah, I'll benchmark it.

[serhiy-storchaka]

For now, I reverted this change because it complicates the code. And the solution for other issues may change it anyway. It is better to defer it until the code will be stabilized.

[AlexWaygood]

I measure a small, but significant, speedup for the following patch (relative to your PR branch) when @singledispatchmethod is used on slotted classes. Note that funcname is only calculated in the except IndexError block:

--- a/Lib/functools.py
+++ b/Lib/functools.py
@@ -967,12 +967,14 @@ def __get__(self, obj, cls=None):
                 return _method
 
         dispatch = self.dispatcher.dispatch
-        funcname = getattr(self.func, '__name__', 'singledispatchmethod method')
         def _method(*args, **kwargs):
-            if not args:
+            try:
+                key = args[0].__class__
+            except IndexError:
+                funcname = getattr(self.func, '__name__', 'singledispatchmethod method')
                 raise TypeError(f'{funcname} requires at least '
-                                '1 positional argument')
-            return dispatch(args[0].__class__).__get__(obj, cls)(*args, **kwargs)
+                                '1 positional argument') from None
+            return dispatch(key).__get__(obj, cls)(*args, **kwargs)

Benchmark script:

import pyperf

runner = pyperf.Runner()
runner.timeit(name="bench singledispatchmethod",
              stmt="""
_ = t.go(1, 1)
""",
setup="""
from functools import singledispatch, singledispatchmethod

class Test:
    __slots__ = ()

    @singledispatchmethod
    def go(self, item, arg):
        pass

    @go.register
    def _(self, item: int, arg):
        return item + arg

t = Test()
""")

I could not measure any performance difference from my original suggestion at #21471 (comment).

Numbers from that benchmark:

• With this PR currently:

  bench singledispatchmethod: Mean +- std dev: 824 ns +- 8 ns
  

• With my proposed patch added to this PR:

  bench singledispatchmethod: Mean +- std dev: 810 ns +- 20 ns
  

I'm fine with it if you want to defer optimising this area of code for now; I agree there are lots of open bug reports regarding @singledispatchmethod, and that those should take priority.