Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

gh-104683: Argument clinic: remove the LandMine class #107541

Merged
merged 1 commit into from
Aug 1, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
3 changes: 1 addition & 2 deletions Lib/test/test_clinic.py
Original file line number Diff line number Diff line change
Expand Up @@ -331,8 +331,7 @@ def converter_init(self):
[clinic start generated code]*/
""")
msg = (
"Stepped on a land mine, trying to access attribute 'noaccess':\n"
"Don't access members of self.function inside converter_init!"
"accessing self.function inside converter_init is disallowed!"
)
self.assertIn(msg, out)

Expand Down
41 changes: 20 additions & 21 deletions Tools/clinic/clinic.py
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@
)
from types import FunctionType, NoneType
from typing import (
TYPE_CHECKING,
Any,
Final,
Literal,
Expand Down Expand Up @@ -2638,18 +2639,6 @@ def get_displayname(self, i: int) -> str:
return f'"argument {i}"'


@dc.dataclass
class LandMine:
# try to access any
__message__: str

def __getattribute__(self, name: str) -> Any:
if name in ('__repr__', '__message__'):
return super().__getattribute__(name)
# raise RuntimeError(repr(name))
fail("Stepped on a land mine, trying to access attribute " + repr(name) + ":\n" + self.__message__)


CConverterClassT = TypeVar("CConverterClassT", bound=type["CConverter"])

def add_c_converter(
Expand Down Expand Up @@ -2844,16 +2833,28 @@ def __init__(self,
if annotation is not unspecified:
fail("The 'annotation' parameter is not currently permitted.")

# this is deliberate, to prevent you from caching information
# about the function in the init.
# (that breaks if we get cloned.)
# so after this change we will noisily fail.
self.function: Function | LandMine = LandMine(
"Don't access members of self.function inside converter_init!"
)
# Make sure not to set self.function until after converter_init() has been called.
# This prevents you from caching information
# about the function in converter_init().
# (That breaks if we get cloned.)
self.converter_init(**kwargs)
self.function = function

# Add a custom __getattr__ method to improve the error message
# if somebody tries to access self.function in converter_init().
#
# mypy will assume arbitrary access is okay for a class with a __getattr__ method,
# and that's not what we want,
# so put it inside an `if not TYPE_CHECKING` block
if not TYPE_CHECKING:
def __getattr__(self, attr):
if attr == "function":
fail(
f"{self.__class__.__name__!r} object has no attribute 'function'.\n"
f"Note: accessing self.function inside converter_init is disallowed!"
)
return super().__getattr__(attr)

def converter_init(self) -> None:
pass

Expand Down Expand Up @@ -4005,7 +4006,6 @@ def converter_init(self, *, type: str | None = None) -> None:

def pre_render(self) -> None:
f = self.function
assert isinstance(f, Function)
default_type, default_name = correct_name_for_self(f)
self.signature_name = default_name
self.type = self.specified_type or self.type or default_type
Expand Down Expand Up @@ -4056,7 +4056,6 @@ def pre_render(self) -> None:
@property
def parser_type(self) -> str:
assert self.type is not None
assert isinstance(self.function, Function)
return required_type_for_self_for_parser(self.function) or self.type

def render(self, parameter: Parameter, data: CRenderData) -> None:
Expand Down