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

Cleanup PerMachine classes, especially nullability #14019

Merged

Conversation

dcbaker
Copy link
Member

@dcbaker dcbaker commented Dec 17, 2024

The main thing I wanted to fix is that currently calling PerMachineDefaultable[T].default_missing() returns a PerMachine[T | None], according to it's annotations and is possible according to its implementation but we universally expect that it should return PerMachine[T]. I've added asserts to ensure that this is the case, but we could write this as a return of PerMachine[T] | None, and then have the callers do a check to ensure that they're not getting a None. But I didn't like that.

Anyway, I've also ported these over to be a dataclass, as that simplifies the initializer logic and allows us to remove custom __repr__ methods, leading a net win on LOC and copying ourselves.

@dcbaker dcbaker added refactoring No behavior changes typing labels Dec 17, 2024
@dcbaker dcbaker requested a review from jpakkane as a code owner December 17, 2024 20:09
A Defaultable PerMachine has a type of `None | T`, in other words,
they have a base type of `PerMachine[None | T]`, but the purpose of
`PerMachine.default_missing()` is to get a `PerMachine[T]` from that
`PerMachine[None | T]`, therefore we should ensure that and annotate
that.
@dcbaker dcbaker force-pushed the submit/cleanup-permachine-nullability branch from 7d41e4d to 789e830 Compare January 30, 2025 17:36
This allows us to simplify the initializers, as well as remove our
custom repr methods.
@dcbaker dcbaker force-pushed the submit/cleanup-permachine-nullability branch from 789e830 to 4558679 Compare January 30, 2025 19:55
@dcbaker dcbaker merged commit cb4ac15 into mesonbuild:master Jan 31, 2025
30 of 31 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
refactoring No behavior changes typing
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants