Skip to content

Commit

Permalink
gh-104050: Improve some typing around defaults and sentinel values (#…
Browse files Browse the repository at this point in the history
…104626)

- 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 <[email protected]>
  • Loading branch information
AlexWaygood and erlend-aasland authored May 18, 2023
1 parent 61027c0 commit 1c55e8d
Showing 1 changed file with 23 additions and 18 deletions.
41 changes: 23 additions & 18 deletions Tools/clinic/clinic.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
import contextlib
import copy
import cpp
import enum
import functools
import hashlib
import inspect
Expand All @@ -28,7 +29,7 @@

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:
#
Expand Down Expand Up @@ -58,25 +59,26 @@
"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 = '--'

Expand Down Expand Up @@ -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.)
Expand Down Expand Up @@ -2686,11 +2688,11 @@ def __init__(self,
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
):
Expand All @@ -2699,7 +2701,10 @@ def __init__(self,
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:
Expand All @@ -2713,7 +2718,7 @@ def __init__(self,
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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -4767,7 +4772,7 @@ def bad_node(self, node):
# 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
Expand Down

0 comments on commit 1c55e8d

Please sign in to comment.