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

DSLParser should not store function as state #7

Open
AlexWaygood opened this issue Aug 1, 2023 · 0 comments
Open

DSLParser should not store function as state #7

AlexWaygood opened this issue Aug 1, 2023 · 0 comments

Comments

@AlexWaygood
Copy link

In most methods of the DSLParser, self.function can be safely assumed to be an instance of Function. However, it cannot be assumed to be an instance of Function in all methods, because it is set to None in self.reset() before starting to parse a block:

cpython/Tools/clinic/clinic.py

Lines 4457 to 4458 in 49f238e

def reset(self) -> None:
self.function = None

This is a pretty fragile design, and means we have to have a bunch of assertions scattered about the DSLParser class in order to keep mypy satisfied:

assert self.function is not None

assert isinstance(self.function, Function)

assert self.function is not None

assert self.function and self.function.parameters

assert self.function is not None

This is a pretty tell-tale sign that function shouldn't really be stored as state at all. Instead, it should be passed into these methods as parameters, and returned from these methods after having been modified. Unfortunately, I think getting there would be a pretty significant refactor...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant