]> git.ipfire.org Git - thirdparty/Python/cpython.git/commitdiff
gh-104050: Improve some typing around `default`s and sentinel values (#104626)
authorAlex Waygood <Alex.Waygood@Gmail.com>
Thu, 18 May 2023 21:58:42 +0000 (22:58 +0100)
committerGitHub <noreply@github.com>
Thu, 18 May 2023 21:58:42 +0000 (21:58 +0000)
- Convert `unspecified` and `unknown` to be members of a `Sentinels` enum, rather than instances of bespoke classes.
  - An enum feels more idiomatic here, and works better with type checkers.
  - Convert some `==` and `!=` checks for these values to identity checks, which are more idiomatic with sentinels.
  - _Don't_ do the same for `Null`, as this needs to be a distinct type due to its usage in `clinic.py`.
- Use `object` as the annotation for `default` across `clinic.py`. `default` can be literally any object, so `object` is the correct annotation here.

---

Co-authored-by: Erlend E. Aasland <erlend.aasland@protonmail.com>
Tools/clinic/clinic.py

index ebee9782939499d4ad47c3a3571abe22a2998d92..42aac7eb17f2ff013b3497d61e6ba33f5bf6395d 100755 (executable)
@@ -12,6 +12,7 @@ import collections
 import contextlib
 import copy
 import cpp
+import enum
 import functools
 import hashlib
 import inspect
@@ -28,7 +29,7 @@ import traceback
 
 from collections.abc import Callable
 from types import FunctionType, NoneType
-from typing import Any, NamedTuple, NoReturn, Literal, overload
+from typing import Any, Final, NamedTuple, NoReturn, Literal, overload
 
 # TODO:
 #
@@ -58,25 +59,26 @@ CLINIC_PREFIXED_ARGS = {
     "return_value",
 }
 
-class Unspecified:
+
+class Sentinels(enum.Enum):
+    unspecified = "unspecified"
+    unknown = "unknown"
+
     def __repr__(self) -> str:
-        return '<Unspecified>'
+        return f"<{self.value.capitalize()}>"
+
 
-unspecified = Unspecified()
+unspecified: Final = Sentinels.unspecified
+unknown: Final = Sentinels.unknown
 
 
+# This one needs to be a distinct class, unlike the other two
 class Null:
     def __repr__(self) -> str:
         return '<Null>'
 
-NULL = Null()
-
 
-class Unknown:
-    def __repr__(self) -> str:
-        return '<Unknown>'
-
-unknown = Unknown()
+NULL = Null()
 
 sig_end_marker = '--'
 
@@ -2600,7 +2602,7 @@ class CConverter(metaclass=CConverterAutoRegister):
     # Or the magic value "unknown" if this value is a cannot be evaluated
     # at Argument-Clinic-preprocessing time (but is presumed to be valid
     # at runtime).
-    default: bool | Unspecified = unspecified
+    default: object = unspecified
 
     # If not None, default must be isinstance() of this type.
     # (You can also specify a tuple of types.)
@@ -2686,11 +2688,11 @@ class CConverter(metaclass=CConverterAutoRegister):
              name: str,
              py_name: str,
              function,
-             default=unspecified,
+             default: object = unspecified,
              *,  # Keyword only args:
              c_default: str | None = None,
              py_default: str | None = None,
-             annotation: str | Unspecified = unspecified,
+             annotation: str | Literal[Sentinels.unspecified] = unspecified,
              unused: bool = False,
              **kwargs
     ):
@@ -2699,7 +2701,10 @@ class CConverter(metaclass=CConverterAutoRegister):
         self.unused = unused
 
         if default is not unspecified:
-            if self.default_type and not isinstance(default, (self.default_type, Unknown)):
+            if (self.default_type
+                and default is not unknown
+                and not isinstance(default, self.default_type)
+            ):
                 if isinstance(self.default_type, type):
                     types_str = self.default_type.__name__
                 else:
@@ -2713,7 +2718,7 @@ class CConverter(metaclass=CConverterAutoRegister):
         if py_default:
             self.py_default = py_default
 
-        if annotation != unspecified:
+        if annotation is not unspecified:
             fail("The 'annotation' parameter is not currently permitted.")
 
         # this is deliberate, to prevent you from caching information
@@ -3967,7 +3972,7 @@ class CReturnConverter(metaclass=CReturnConverterAutoRegister):
 
     # The Python default value for this parameter, as a Python value.
     # Or the magic value "unspecified" if there is no default.
-    default = None
+    default: object = None
 
     def __init__(self, *, py_default=None, **kwargs):
         self.py_default = py_default
@@ -4767,7 +4772,7 @@ class DSLParser:
                     # but at least make an attempt at ensuring it's a valid expression.
                     try:
                         value = eval(default)
-                        if value == unspecified:
+                        if value is unspecified:
                             fail("'unspecified' is not a legal default value!")
                     except NameError:
                         pass # probably a named constant